fix(torii-grpc): subscribe indexer #2563
Conversation
WalkthroughOhayo! This pull request modifies the SQL query in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- crates/torii/grpc/src/server/subscriptions/indexer.rs (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
crates/torii/grpc/src/server/subscriptions/indexer.rs (1)
Line range hint
1-190: Ohayo again, sensei! The rest of the file looks good.The change to the SQL query is the only modification in this file. The overall structure and logic of the
IndexerManagerandServiceimplementations remain intact. Keep up the good work!
| @@ -47,7 +47,7 @@ impl IndexerManager { | |||
| let (sender, receiver) = channel(1); | |||
|
|
|||
| let mut statement = | |||
| "SELECT head, tps, last_block_timestamp, contract_address FROM contracts".to_string(); | |||
| "SELECT * FROM contracts".to_string(); | |||
There was a problem hiding this comment.
💡 Codebase verification
Ohayo, sensei! It's best to specify the needed columns instead of using SELECT *:
- Performance: Retrieving all columns can slow down the query, especially if some columns are large or not needed.
- Maintainability: Explicitly listing columns makes the code clearer and less prone to breakage if the table structure changes in the future.
🔗 Analysis chain
Ohayo, sensei! Consider the implications of selecting all columns.
The change from selecting specific columns to SELECT * simplifies the query, but it might have some drawbacks:
- Performance: Retrieving all columns could impact performance, especially if the
contractstable has many columns or large data types. - Future-proofing: If the table structure changes in the future, this query might retrieve unexpected columns.
To assess the impact, let's check the contracts table structure:
Consider reverting to selecting specific columns or using a view if you need flexibility in the future.
Ohayo, sensei! Let's delve deeper into the contracts table structure to evaluate the use of SELECT *.
To fully understand the implications of selecting all columns, we need to know which columns exist in the contracts table.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure of the contracts table
# Expected result: List of columns in the contracts table
# Search for the contracts table definition
rg -i 'create\s+table\s+contracts'
Length of output: 129
Script:
#!/bin/bash
# Description: Extract the full CREATE TABLE statement for contracts
# Expected result: Full table definition including all columns
rg -A 20 -i 'CREATE\s+TABLE\s+contracts\s*\(' crates/torii/migrations/20240829114436_merge_worlds_and_indexer.sql
Length of output: 692
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2563 +/- ##
==========================================
+ Coverage 69.77% 69.80% +0.03%
==========================================
Files 400 400
Lines 51171 51182 +11
==========================================
+ Hits 35704 35729 +25
+ Misses 15467 15453 -14 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes