fix: panic while reading information_schema. KEY_COLUMN_USAGE #4318
Conversation
WalkthroughThis update introduces a refined control flow in the key column usage logic to safely skip dropped tables during iteration. Additionally, new tests demonstrate the functionality by creating, querying, and dropping tables to verify index display correctness. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SQLClient
participant Catalog
User->>+SQLClient: Create `test` table
SQLClient->>+Catalog: Execute CREATE TABLE
Catalog-->>-SQLClient: Table created
User->>+SQLClient: Show indexes from `test`
SQLClient->>+Catalog: Execute SHOW INDEX FROM test
Catalog-->>-SQLClient: Return index details
User->>+SQLClient: Drop `test` table
SQLClient->>+Catalog: Execute DROP TABLE
Catalog-->>-SQLClient: Table dropped
User->>+SQLClient: Show indexes from `system_metrics`
SQLClient->>+Catalog: Execute SHOW INDEX FROM system_metrics
Catalog-->>-SQLClient: Return index details
PoemCrafting code with careful might, 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/catalog/src/information_schema/key_column_usage.rs (1 hunks)
Additional comments not posted (1)
src/catalog/src/information_schema/key_column_usage.rs (1)
259-260: LGTM! But verify the handling of dropped tables.The change replaces
unreachable!()with a comment and acontinuestatement to handle cases where a table might be dropped during iteration. This prevents a panic and allows the iteration to continue, which is a crucial fix for stability.However, ensure that the logic correctly handles the cases where tables are dropped during iteration.
Verification successful
Handling of dropped tables during iteration is correctly implemented.
The change replaces
unreachable!()with a comment and acontinuestatement to handle cases where a table might be dropped during iteration. This prevents a panic and allows the iteration to continue, which is a crucial fix for stability. The implementation correctly skips the dropped table and proceeds with the next one.
- The
continuestatement ensures smooth iteration without panics.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of dropped tables during iteration. # Test: Search for the usage of the `make_key_column_usage` method. Expect: Proper handling of dropped tables. rg --type rust $'make_key_column_usage'Length of output: 411
Script:
#!/bin/bash # Extract the full implementation of the `make_key_column_usage` method for detailed analysis. ast-grep --lang rust --pattern $'async fn make_key_column_usage(&mut self, request: Option<ScanRequest>) -> Result<RecordBatch> { $$$ }'Length of output: 7645
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4318 +/- ##
==========================================
- Coverage 85.15% 84.89% -0.27%
==========================================
Files 1061 1061
Lines 188255 188247 -8
==========================================
- Hits 160313 159803 -510
- Misses 27942 28444 +502 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/catalog/src/information_schema/key_column_usage.rs (2 hunks)
- tests/cases/standalone/common/show/show_index.result (2 hunks)
- tests/cases/standalone/common/show/show_index.sql (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/catalog/src/information_schema/key_column_usage.rs
Additional comments not posted (8)
tests/cases/standalone/common/show/show_index.sql (4)
12-19: LGTM!The creation of the
testtable with specified columns, primary key, and time index follows the correct pattern.
23-23: LGTM!The command to show indexes from the
testtable is syntactically correct.
24-24: Duplicate command.This command already exists in the file and does not need to be added again.
35-35: LGTM!The command to drop the
testtable is syntactically correct.tests/cases/standalone/common/show/show_index.result (4)
14-24: LGTM!The result of creating the
testtable shows 0 affected rows, which is as expected.
29-37: LGTM!The result correctly displays the index details for the
testtable, including the primary key and time index.
38-38: LGTM!The result correctly displays the index details for the
system_metricstable.
75-78: LGTM!The result of dropping the
testtable shows 0 affected rows, which is as expected.
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
#4305
What's changed and what's your intention?
information_schema. KEY_COLUMN_USAGE. Panic while reading information_schema.KEY_COLUMN_USAGE #4305Seq_in_index.Checklist
Summary by CodeRabbit
New Features
testand existingsystem_metrics.testwith columns and indexes, and commands to show and drop these indexes.Bug Fixes