parser: modify parser to support partial index (#63448)#68830
parser: modify parser to support partial index (#63448)#68830ti-chi-bot wants to merge 1 commit into
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
do-not-merge/hold
|
@YangKeao This PR has conflicts, I have hold it. |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThis PR extends TiDB's SQL parser to support partial indexes with WHERE conditions. The changes add AST fields to represent partial-index clauses, extend parser grammar to recognize the new syntax, and include test coverage for three index creation forms: inline table indexes, standalone CREATE INDEX, and ALTER TABLE ADD INDEX. ChangesPartial Index Parser Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/parser/ast/ddl.go`:
- Around line 731-752: Remove the leftover merge-conflict markers (<<<<<<<,
=======, >>>>>>>) in the struct definition around KeyBlockSize/Tp/... and
resolve the duplicated fields by keeping the intended branch state (the updated
struct that includes KeyBlockSize, Tp (IndexType), Comment, ParserName (CIStr),
Visibility, PrimaryKeyTp, Global plus the new fields SplitOpt *SplitOption,
SecondaryEngineAttr, AddColumnarReplicaOnDemand and Condition ExprNode) so the
struct is consistent and compiles; do the same cleanup for the other conflict
regions referenced (around the other occurrences at the noted ranges) ensuring
no conflict markers remain and field types/names match the updated Index-related
types (IndexType, CIStr, PrimaryKeyType, SplitOption, ExprNode).
In `@pkg/parser/parser.y`:
- Around line 6638-6646: Remove the leftover merge conflict markers (<<<<<<<,
=======, >>>>>>>) and keep a single coherent else-if chain that merges opt2 into
opt1 (the intended actions assigning opt1.SplitOpt = opt2.SplitOpt,
opt1.SecondaryEngineAttr = opt2.SecondaryEngineAttr, opt1.Condition =
opt2.Condition as appropriate). Locate the conflicting block around the
opt1/opt2 handling (references: opt1, opt2, SplitOpt, SecondaryEngineAttr,
Condition) and delete the conflict markers and duplicate branches so the
parser.y grammar contains only the correct sequence; also apply the same
removal/fix for the later conflicting region (the other block around lines
6703-6729) to ensure the file is valid for parser generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 510be38c-d1e8-40f9-9592-ae68d71aefa9
📒 Files selected for processing (4)
pkg/parser/ast/ddl.gopkg/parser/parser.gopkg/parser/parser.ypkg/parser/parser_test.go
| <<<<<<< HEAD | ||
| KeyBlockSize uint64 | ||
| Tp model.IndexType | ||
| Comment string | ||
| ParserName model.CIStr | ||
| Visibility IndexVisibility | ||
| PrimaryKeyTp model.PrimaryKeyType | ||
| Global bool | ||
| ======= | ||
| KeyBlockSize uint64 | ||
| Tp IndexType | ||
| Comment string | ||
| ParserName CIStr | ||
| Visibility IndexVisibility | ||
| PrimaryKeyTp PrimaryKeyType | ||
| Global bool | ||
| SplitOpt *SplitOption `json:"-"` // SplitOption contains expr nodes, which cannot marshal for DDL job arguments. | ||
| SecondaryEngineAttr string | ||
| AddColumnarReplicaOnDemand int | ||
| Condition ExprNode `json:"-"` // Condition contains expr nodes, which cannot marshal for DDL job arguments. It's used for partial index. | ||
| >>>>>>> 7b93bcb4150 (parser: modify parser to support partial index (#63448)) | ||
| } |
There was a problem hiding this comment.
Resolve leftover merge-conflict markers before merge.
Lines in this file still contain <<<<<<<, =======, >>>>>>>, which makes the parser package fail to compile and blocks validation of the partial-index changes. Please resolve the conflict and keep only the intended branch state.
Suggested cleanup (illustrative)
-<<<<<<< HEAD
-...
-=======
-...
->>>>>>> 7b93bcb4150 (parser: modify parser to support partial index (`#63448`))
+// keep only the resolved final implementationAlso applies to: 763-770, 844-892
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/parser/ast/ddl.go` around lines 731 - 752, Remove the leftover
merge-conflict markers (<<<<<<<, =======, >>>>>>>) in the struct definition
around KeyBlockSize/Tp/... and resolve the duplicated fields by keeping the
intended branch state (the updated struct that includes KeyBlockSize, Tp
(IndexType), Comment, ParserName (CIStr), Visibility, PrimaryKeyTp, Global plus
the new fields SplitOpt *SplitOption, SecondaryEngineAttr,
AddColumnarReplicaOnDemand and Condition ExprNode) so the struct is consistent
and compiles; do the same cleanup for the other conflict regions referenced
(around the other occurrences at the noted ranges) ensuring no conflict markers
remain and field types/names match the updated Index-related types (IndexType,
CIStr, PrimaryKeyType, SplitOption, ExprNode).
| <<<<<<< HEAD | ||
| ======= | ||
| } else if opt2.SplitOpt != nil { | ||
| opt1.SplitOpt = opt2.SplitOpt | ||
| } else if len(opt2.SecondaryEngineAttr) > 0 { | ||
| opt1.SecondaryEngineAttr = opt2.SecondaryEngineAttr | ||
| } else if opt2.Condition != nil { | ||
| opt1.Condition = opt2.Condition | ||
| >>>>>>> 7b93bcb4150 (parser: modify parser to support partial index (#63448)) |
There was a problem hiding this comment.
Resolve unresolved merge-conflict markers in grammar
<<<<<<<, =======, >>>>>>> are still present (Line 6638 and Line 6703). This makes parser.y invalid and blocks parser generation/build. Please resolve the conflict and keep only the intended grammar/actions.
Also applies to: 6703-6729
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/parser/parser.y` around lines 6638 - 6646, Remove the leftover merge
conflict markers (<<<<<<<, =======, >>>>>>>) and keep a single coherent else-if
chain that merges opt2 into opt1 (the intended actions assigning opt1.SplitOpt =
opt2.SplitOpt, opt1.SecondaryEngineAttr = opt2.SecondaryEngineAttr,
opt1.Condition = opt2.Condition as appropriate). Locate the conflicting block
around the opt1/opt2 handling (references: opt1, opt2, SplitOpt,
SecondaryEngineAttr, Condition) and delete the conflict markers and duplicate
branches so the parser.y grammar contains only the correct sequence; also apply
the same removal/fix for the later conflicting region (the other block around
lines 6703-6729) to ensure the file is valid for parser generation.
|
@ti-chi-bot: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@ti-chi-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
ti-chi-bot
Bot
added
cherry-pick-approved
2 participants
This is an automated cherry-pick of #63448
What problem does this PR solve?
Issue Number: close #63447
-> #63448
#62759
#62762
Problem Summary:
What changed and how does it work?
Modify the parser to support partial index grammar
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit