feat: validate partition rule on create table #4213
Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
WalkthroughThe changes across the files focus on enhancing error handling and partitioning logic within the system. Key modifications include adding a new Changes
Sequence Diagram(s)(Omitted; the changes don't significantly alter control flow functionality.) Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/operator/src/error.rs (2 hunks)
- src/operator/src/statement/ddl.rs (4 hunks)
- src/partition/src/multi_dim.rs (2 hunks)
- tests/cases/standalone/common/partition.result (1 hunks)
- tests/cases/standalone/common/partition.sql (1 hunks)
Additional comments not posted (6)
tests/cases/standalone/common/partition.sql (1)
73-83: Review the test for incorrect partition rule.The SQL statement for creating a table with an incorrect partition rule is added for testing purposes. Ensure that this test case is included in the test suite and that it is expected to fail as intended.
tests/cases/standalone/common/partition.result (1)
170-183: Validate the expected result for incorrect partition rule.The expected error message for the incorrect partition rule is correctly added. This ensures that the system properly handles and reports partition rule errors.
src/partition/src/multi_dim.rs (2)
192-192: Review changes inRuleChecker::checkmethod.The refactoring of the
checkmethod inRuleCheckershould be carefully examined to ensure it does not introduce any regressions or overlook potential edge cases in partition rule validation.
69-69: Ensure proper error handling intry_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.Verification successful
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Ensure that this validation is comprehensive and covers all necessary cases to prevent invalid partition rules.
Ensure proper error handling in
try_newmethod.The
try_newmethod ofMultiDimPartitionRuleuses thecheckmethod ofRuleCheckerto validate the rule. Thecheckmethod, along with thewalk_exprandcheck_axismethods, provides comprehensive validation by ensuring that expressions are well-formed and all split points in the axis are properly closed. This validation covers all necessary cases to prevent invalid partition rules.
checkmethod: Iterates over expressions and callswalk_expr.walk_exprmethod: Recursively validates expressions.check_axismethod: Ensures all split points in the axis are properly closed.These methods collectively ensure that the validation is thorough and robust.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all necessary cases are covered in the `check` method. # Test: Search for the `check` method usage. Expect: Comprehensive validation logic. rg --type rust $'check'Length of output: 139608
Script:
#!/bin/bash # Description: Retrieve the implementation of the `check` method and related methods within the `multi_dim.rs` file to ensure comprehensive validation. ast-grep --lang rust --pattern $'impl RuleChecker { $_ pub fn check(&mut self) -> Result<()> { $_ } $_ }' --debugLength of output: 344
Script:
#!/bin/bash # Description: Retrieve the implementation of the `check` method and related methods within the `multi_dim.rs` file to ensure comprehensive validation. ast-grep --lang rust --pattern $'impl RuleChecker { $_ pub fn check(&mut self) -> Result<()> { $_ } $_ }' --debug-queryLength of output: 302
Script:
#!/bin/bash # Description: Retrieve the implementation of the `check` method and related methods within the `multi_dim.rs` file to ensure comprehensive validation. rg 'impl RuleChecker' -A 20 src/partition/src/multi_dim.rsLength of output: 60
Script:
#!/bin/bash # Description: Retrieve the implementation of the `check` method and related methods within the `multi_dim.rs` file to ensure comprehensive validation. rg 'impl RuleChecker' -A 50 src/partition/src/multi_dim.rsLength of output: 60
Script:
#!/bin/bash # Description: Retrieve the implementation of the `check` method directly within the `multi_dim.rs` file to ensure comprehensive validation. rg 'pub fn check' -A 20 src/partition/src/multi_dim.rsLength of output: 785
Script:
#!/bin/bash # Description: Retrieve the full implementation of the `walk_expr` and `check_axis` methods within the `multi_dim.rs` file to ensure comprehensive validation. rg 'fn walk_expr' -A 30 src/partition/src/multi_dim.rs rg 'fn check_axis' -A 30 src/partition/src/multi_dim.rsLength of output: 2128
src/operator/src/statement/ddl.rs (2)
43-43: Updated import statement for partitioning logic.The import statement for
MultiDimPartitionRulehas been added to support the new partitioning validation logic. This is correctly placed in the import section and is necessary for the changes in partition rule validation.
73-75: Error variant renaming and addition.The error variant
InvalidPartitionRuleSnafuhas been renamed toInvalidPartitionSnafu, which is a clearer name reflecting that it deals with invalid partition errors in general, not just rules. This change improves readability and maintainability of the error handling code.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4213 +/- ##
==========================================
- Coverage 84.92% 84.61% -0.31%
==========================================
Files 1023 1039 +16
Lines 179298 182766 +3468
==========================================
+ Hits 152272 154653 +2381
- Misses 27026 28113 +1087 |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Validate partition rule on
CREATE TABLEChecklist
Summary by CodeRabbit
New Features
InvalidPartitionerror variant, providing more specific error messages for invalid partition rules.Bug Fixes
Refactor
Tests