Conversation
WalkthroughThis change introduces two new SQL actions for querying taxonomy data by block height and by specific streams, along with comprehensive Go tests and supporting utilities. The migration adds the actions, while new tests validate their correctness, and utility functions and types facilitate test execution and input structuring. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Utils as Test Utilities
participant SQL as SQL Engine
Test->>Utils: Prepare input structs (e.g., ListTaxonomiesByHeightInput)
Utils->>SQL: Call list_taxonomies_by_height or get_taxonomies_for_streams with parameters
SQL-->>Utils: Return taxonomy query results
Utils-->>Test: Return processed results for assertions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Time Submission Status
|
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/migrations/016-taxonomy-query-actions.sql (1)
72-76: Consider making the lookback window configurableThe hardcoded lookback window of 1000 blocks (line 75) may not be sufficient for all use cases. Consider adding a configuration parameter or environment variable to make this value adjustable based on network requirements.
tests/streams/taxonomy_query_actions_test.go (1)
354-357: Strengthen pagination test assertionThe test should verify the exact number of results on the second page rather than just checking it's not empty.
-// Should return remaining results -if len(result2) == 0 { - return errors.New("expected at least 1 result on page 2") -} +// Should return remaining result (1 taxonomy) +if len(result2) != 1 { + return errors.Errorf("expected 1 result on page 2, got %d", len(result2)) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/migrations/016-taxonomy-query-actions.sql(1 hunks)tests/streams/taxonomy_query_actions_test.go(1 hunks)tests/streams/utils/procedure/execute.go(1 hunks)tests/streams/utils/procedure/types.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (11)
tests/streams/utils/procedure/execute.go (2)
765-805: LGTM!The
ListTaxonomiesByHeightfunction correctly implements the invocation of thelist_taxonomies_by_heightSQL action, following the established patterns in this file for context creation, error handling, and result processing.
807-845: LGTM!The
GetTaxonomiesForStreamsfunction correctly implements the invocation of theget_taxonomies_for_streamsSQL action with proper context creation and error handling.tests/streams/utils/procedure/types.go (2)
145-153: LGTM!The
ListTaxonomiesByHeightInputstruct correctly defines all necessary fields for thelist_taxonomies_by_heightaction, with appropriate use of pointers for optional parameters.
155-161: LGTM!The
GetTaxonomiesForStreamsInputstruct correctly defines all necessary fields for theget_taxonomies_for_streamsaction, supporting batch queries with array inputs.internal/migrations/016-taxonomy-query-actions.sql (3)
38-139: Well-structured taxonomy query actionThe
list_taxonomies_by_heightaction is well-implemented with proper input validation, efficient CTE usage for latest-only filtering, and consistent handling of active taxonomies. The pagination support and flexible height range querying make it suitable for explorer synchronization.
235-237: Consider duplicate handling in resultsWhen the same stream appears multiple times in the input arrays with
latest_only=true, the current implementation will return duplicate rows. Consider whether this is the intended behavior or if you want to return only unique results.
156-277: Efficient batch query implementationThe
get_taxonomies_for_streamsaction effectively handles batch queries using recursive CTEs for array processing. The implementation correctly validates inputs and provides consistent filtering for active taxonomies.tests/streams/taxonomy_query_actions_test.go (4)
35-156: Well-structured test for height-based queryingThe test effectively validates the height range filtering functionality, correctly expecting only taxonomies within the specified range.
159-257: Comprehensive test for latest_only functionalityThe test properly validates that the latest_only flag returns only the most recent taxonomy per stream, correctly identifying it by group_sequence.
364-457: Effective test for batch stream queryingThe test properly validates the batch query functionality for multiple streams.
460-554: Thorough test for batch query with latest_onlyThe test effectively validates that batch queries with latest_only return the correct taxonomy, using weight value verification to ensure accuracy.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
internal/migrations/016-taxonomy-query-actions.sql (3)
73-76: Consider making the lookback window configurable.The hardcoded lookback window of 1000 blocks may not be suitable for all use cases. Different networks or scenarios might require different default ranges.
Consider adding a configuration parameter or using a more adaptive approach:
- -- Use a reasonable lookback window to find recent taxonomies - $effective_from := $current_block - 1000; + -- Use a configurable lookback window (could be stored in a system parameter table) + $lookback_window INT8 := 1000; -- Default value, could be made configurable + $effective_from := $current_block - $lookback_window;
129-130: Simplify the ORDER BY clause for better performance.The ORDER BY clause with 5 columns could impact query performance, especially with large datasets. Since
created_atshould be unique enough for consistent pagination, the additional columns may be unnecessary.- ORDER BY t.created_at ASC, t.data_provider ASC, t.stream_id ASC, t.child_data_provider ASC, t.child_stream_id ASC + ORDER BY t.created_at ASC, t.data_provider ASC, t.stream_id ASC
291-294: Simplify redundant JOIN pattern.The query joins
all_pairswithunique_pairsand then with taxonomies, butunique_pairsis already derived fromall_pairs. This creates unnecessary complexity.- FROM all_pairs ap - JOIN unique_pairs up ON ap.data_provider = up.data_provider AND ap.stream_id = up.stream_id - JOIN taxonomies t ON up.data_provider = t.data_provider AND up.stream_id = t.stream_id + FROM unique_pairs up + JOIN taxonomies t ON up.data_provider = t.data_provider AND up.stream_id = t.stream_idNote: If you need to preserve duplicates from the input arrays, consider using
all_pairsdirectly without theunique_pairsCTE.tests/streams/taxonomy_query_actions_test.go (1)
170-212: Extract common stream setup logic to reduce duplication.The stream setup pattern is repeated across multiple test functions. Consider extracting this into a helper function.
type streamSetup struct { ComposedStreamId util.StreamId ChildStreamIds []util.StreamId Deployer util.EthereumAddress } func setupStreamsForTest(ctx context.Context, platform *kwilTesting.Platform, setup streamSetup, height int64) error { // Setup composed stream if err := setupComposedStream(ctx, SetupComposedStreamInput{ Platform: platform, StreamId: setup.ComposedStreamId, Height: height, }); err != nil { return errors.Wrap(err, "error setting up composed stream") } // Setup child streams for _, childId := range setup.ChildStreamIds { // ... setup child stream } return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/migrations/016-taxonomy-query-actions.sql(1 hunks)tests/streams/taxonomy_query_actions_test.go(1 hunks)tests/streams/utils/procedure/execute.go(1 hunks)tests/streams/utils/procedure/types.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/streams/utils/procedure/types.go
- tests/streams/utils/procedure/execute.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/streams/taxonomy_query_actions_test.go (2)
20-30: Good implementation of column constants.This addresses the previous review comment about avoiding hardcoded indices. The constants improve code maintainability and readability.
32-51: Comprehensive test coverage including error conditions.Good job addressing the previous review comment by adding error condition tests. The test suite now covers invalid height ranges, invalid pagination, and mismatched arrays.
🧹 Nitpick comments (2)
tests/streams/taxonomy_query_actions_test.go (2)
737-770: Consider using more flexible error validation.While the error condition test is good, hardcoding the exact error message makes the test brittle. Consider checking for the error type or a more general pattern.
- // Verify the error message contains expected text - expectedError := "Invalid height range" - if !strings.Contains(err.Error(), expectedError) { - return errors.Errorf("expected error message to contain '%s', got: %s", expectedError, err.Error()) - } + // Verify we got an error about invalid range + errMsg := strings.ToLower(err.Error()) + if !strings.Contains(errMsg, "invalid") || !strings.Contains(errMsg, "range") { + return errors.Errorf("expected error about invalid range, got: %s", err.Error()) + }
825-861: Consider more flexible error validation for maintainability.Similar to the previous error test, consider using a more flexible error validation approach.
- // Verify the error message contains expected text - expectedError := "must have the same length" - if !strings.Contains(err.Error(), expectedError) { - return errors.Errorf("expected error message to contain '%s', got: %s", expectedError, err.Error()) - } + // Verify we got an error about mismatched lengths + errMsg := strings.ToLower(err.Error()) + if !strings.Contains(errMsg, "length") || !strings.Contains(errMsg, "same") { + return errors.Errorf("expected error about mismatched lengths, got: %s", err.Error()) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/migrations/016-taxonomy-query-actions.sql(1 hunks)tests/streams/taxonomy_query_actions_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/migrations/016-taxonomy-query-actions.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (6)
tests/streams/taxonomy_query_actions_test.go (6)
1-18: LGTM!The imports are well-organized and appropriate for the test file.
53-175: Well-structured test for height-based querying.The test properly sets up test data and validates the filtering by height range.
177-298: Proper use of column constants and thorough testing.The test correctly uses the defined column constants and thoroughly validates the latest_only functionality.
300-402: Good pagination test coverage.The test properly validates both limit and offset functionality across multiple pages.
597-735: Excellent comprehensive column validation.The test thoroughly validates all 8 columns with descriptive error messages and proper use of column constants.
772-823: Good validation of pagination parameter handling.The test properly validates that negative pagination parameters are handled gracefully.
* feat: view list taxonomies by height * chore: apply @coderrabit suggestion
* chore: add data providers table * feat: add data_providers table * chore: add unique index * chore: create index if not exists * chore: normalize streams table (#1046) * chore: normalize metadata table (#1047) * chore: normalize taxonomies table (#1053) * chore: normalize primitive events table (#1054) * chore: adjust insert actions to normalized tables * chore: revise error message * feat: create action for data provider and test * chore: adjust insert metadata * feat: adjust insert primitive events * chore: adjust tests * chore: adjust tn ops tests * chore: adjust tests * chore: optimize stream ids lookup * fix: variable name * chore: use inner join * chore: remove unnecessary select * chore: use optimized action on truflation query too * chore: adjust common actions to normalized tables (#1068) * chore: adjust common actions to normalized tables * fix: join data providers * chore: debug * chore: adjust authorization actions to normalized tables (#1070) * chore: adjust authorization actions to normalized tables * chore: update actions * chore: update actions * fix: logic * chore: adjust taxonomy actions to normalized tables (#1074) * chore: adjust taxonomy actions to normalized tables * chore: avoid rate limiting on github actions * chore: adjust primitive query actions to normalized tables (#1075) * chore: adjust composed query actions to normalized tables (#1076) * chore: adjust composed query actions to normalized tables * chore: update derivates * fix: stream ref for parent * chore: adjust utilities actions to normalized tables (#1078) * chore: adjust utilities actions to normalized tables * chore: reduce database size expectation on test * chore: update truflation query * chore: use stream ref * fix: uuid to int * chore: register data provider on cache height tracking test * chore: remove unused local deployer * chore: remove unused local deployer * feat: add migration scripts for normalization process - Introduced multiple SQL migration scripts to transition the database schema from UUID to INT types for IDs. - Implemented actions to handle both old and new schemas during the migration. - Added scripts to populate `data_provider_id` and `id` columns in the `streams` table. - Created necessary indexes and constraints to optimize performance and maintain data integrity. - Included a shell script for executing migrations and logging execution times. - Documented the migration process and performance recommendations in the README. * chore: remove optimized index for gap-filling queries in primitive_events table * refactor: streamline SQL migration script for foreign key constraints - Added unique index on streams.id to ensure data integrity before creating foreign key references. - Simplified the addition of foreign key constraints for metadata and taxonomies tables. - Removed commented-out code related to partial indexes for primitive_events, as it is not currently supported. * feat: view list taxonomies by height (#1098) * feat: view list taxonomies by height * chore: apply @coderrabit suggestion * feat: register deployer as a data provider in benchmark setup - Added functionality to register the deployer as a data provider during the benchmark setup process. - Included error handling to wrap any issues encountered while creating the data provider. * chore: remove unused SQL migration scripts for weight and composed data retrieval - Deleted the SQL files `get-all-weights.sql` and `get-composed-data.sql` as they are no longer needed in the migration process. - These scripts were previously used for retrieving weights and composed data from taxonomies but have been deemed obsolete. * chore: remove obsolete SQL migration scripts for normalization process - Deleted multiple SQL migration scripts related to the normalization process, including those for structure migration, data population, and final indexing. - These scripts were previously used to transition the database schema and have been deemed unnecessary after the migration completion. - The removal helps to clean up the codebase and reduce clutter in the migration directory. * refactor: normalize SQL migration scripts and clean up legacy columns - Updated SQL migration scripts to normalize references and improve data integrity. - Removed denormalized columns from `metadata`, `taxonomies`, and `primitive_events` tables. - Adjusted primary keys and added necessary indexes to optimize performance. - Cleaned up the migration directory by removing obsolete scripts and ensuring a streamlined structure. * chore: remove obsolete test files for weight and category streams - Deleted the test files `get_all_weights_for_query_test.go` and `get_category_test.go` as they are no longer needed in the codebase. - These tests were previously used for validating weight calculations and category stream functionalities but have been deemed unnecessary after recent refactoring and updates. - The removal helps to clean up the test directory and reduce clutter in the codebase. * feat: add tests for primitive batch insert alignment and enhance complex composed tests - Introduced `TestPrimitiveBatchInsertAlignment` to ensure correct mapping of batch insertions across multiple streams, addressing a regression issue with stream alignment. - Added `testComplexComposedRecordTruflationVariant` to validate the consistency of composed records when using Truflation-prefixed actions, ensuring compatibility with frozen logic. - Updated existing tests to register data providers consistently across various test cases, improving setup reliability. --------- Co-authored-by: williamrusdyputra <williamrusdyputra@gmail.com> Co-authored-by: Michael Buntarman <michaelboentarman@gmail.com>
2 participants
Description
Related Problem
resolves: https://github.com/trufnetwork/truf-network/issues/1080
How Has This Been Tested?
Summary by CodeRabbit
New Features
Tests
Chores