Create interface for valid tables #14
Conversation
WalkthroughThe pull request refactors the row pruning mechanism in the informative indexer. It introduces a Changes
Sequence DiagramsequenceDiagram
participant Prunner
participant ValidTablesMap
participant Database
Prunner->>ValidTablesMap: Check table type
ValidTablesMap-->>Prunner: Return table interface
Prunner->>Database: Fetch rows to prune
Database-->>Prunner: Return database rows
Prunner->>ValidTablesMap: Unmarshal rows using table interface
ValidTablesMap-->>Prunner: Return unmarshalled data
Poem
Finishing Touches
🪧 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
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (4)
informative-indexer/db/valid_tables.go (3)
9-12: Consider using constants for table names.Table names are used as string literals in multiple places. Consider defining them as constants to prevent typos and enable compile-time checks.
+const ( + TableTransactionEvents = "transaction_events" + TableFinalizeBlockEvents = "finalize_block_events" + TableMoveEvents = "move_events" +) var ValidTablesMap = map[string]ValidTable{ - "transaction_events": &TransactionEvent{}, - "finalize_block_events": &FinalizeBlockEvent{}, - "move_events": &MoveEvent{}, + TableTransactionEvents: &TransactionEvent{}, + TableFinalizeBlockEvents: &FinalizeBlockEvent{}, + TableMoveEvents: &MoveEvent{}, }
Line range hint
16-21: Optimize table name validation.The current implementation iterates through the map keys. Consider using direct map lookup for better performance.
func isValidTableName(tableName string) bool { - for validTable, _ := range ValidTablesMap { - if tableName == validTable { - return true - } - } - return false + _, ok := ValidTablesMap[tableName] + return ok }
25-29: Optimize slice allocation in GetValidTableNames.Preallocate the slice with the known capacity to avoid reallocation during append.
func GetValidTableNames() []string { - var keys []string + keys := make([]string, 0, len(ValidTablesMap)) for key := range ValidTablesMap { keys = append(keys, key) } return keys }informative-indexer/prunner/prunner.go (1)
130-137: Consider batch processing for better performance.Processing rows one at a time might be inefficient for large datasets. Consider implementing batch processing if supported by the underlying implementations.
Additionally, preallocate the result slice if the number of rows can be determined:
- var result []interface{} + result := make([]interface{}, 0, estimatedSize) // if rows.CommandTag().RowsAffected() is available
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (3)
informative-indexer/db/db.go(1 hunks)informative-indexer/db/valid_tables.go(2 hunks)informative-indexer/prunner/prunner.go(1 hunks)
🔇 Additional comments (2)
informative-indexer/db/valid_tables.go (1)
5-7: LGTM! Well-designed interface.The ValidTable interface is well-defined with a single responsibility of unmarshalling rows.
informative-indexer/prunner/prunner.go (1)
134-137: LGTM! Proper error handling.The implementation correctly checks for errors during unmarshalling and provides clear error messages.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
informative-indexer/db/valid_tables.go (3)
5-7: Add documentation and consider type safety improvements.The
ValidTableinterface would benefit from:
- GoDoc comments explaining its purpose and usage
- Documentation about expected map keys and values
- Consider using a more type-safe approach instead of
map[string]interface{}+// ValidTable defines the interface for database tables that can unmarshal their rows +// into a structured format. Implementers should document the expected keys and values +// in the returned map. type ValidTable interface { Unmarshal(pgx.Rows) (map[string]interface{}, error) }
15-18: Add documentation for public functions.The
isValidTableNamefunction should have GoDoc comments explaining its purpose and parameters.+// isValidTableName checks if the given table name exists in the ValidTablesMap. +// It returns true if the table name is valid, false otherwise. func isValidTableName(tableName string) bool { _, ok := ValidTablesMap[tableName] return ok }
20-25: Consider caching the table names slice.Since the table names are static, consider caching the slice to avoid allocations on every call to
GetValidTableNames().+var validTableNamesCache []string + +func init() { + validTableNamesCache = make([]string, 0, len(ValidTablesMap)) + for key := range ValidTablesMap { + validTableNamesCache = append(validTableNamesCache, key) + } +} + +// GetValidTableNames returns a slice of all valid table names. func GetValidTableNames() []string { - var keys []string - for key := range ValidTablesMap { - keys = append(keys, key) - } - return keys + return append([]string{}, validTableNamesCache...) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
informative-indexer/db/db.go(1 hunks)informative-indexer/db/valid_tables.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- informative-indexer/db/db.go
🔇 Additional comments (1)
informative-indexer/db/valid_tables.go (1)
1-25: Overall implementation looks good!The new design successfully achieves the PR objectives by:
- Centralizing table definitions
- Providing a clean interface for table operations
- Eliminating the need for multiple if-else statements
The suggested improvements are mostly for optimization and documentation, but the core implementation is solid.
Instead of keep adding if-else for each table we add. We introduce the ValidTablesMap to keep track of valid table names and their corresponding struct. Therefore we do not have to repeat adding table names in multiple files and occasions, also making it easier to add and unmarshalling new tables.
Summary by CodeRabbit
Refactor
ValidTableinterface to improve type mapping and table handling.Bug Fixes