parser: support MySQL 8.0.19+ INSERT row alias syntax#67920
parser: support MySQL 8.0.19+ INSERT row alias syntax#67920paultyng wants to merge 2 commits intopingcap:masterfrom
Conversation
Add parsing support for the `AS row_alias[(col_alias, ...)]` clause in INSERT ... VALUES and INSERT ... SET statements, as introduced in MySQL 8.0.19 (WL pingcap#6312) and required since VALUES() was deprecated in MySQL 8.0.20. Examples: INSERT INTO t VALUES (1,2) AS new ON DUPLICATE KEY UPDATE b = new.b; INSERT INTO t VALUES (1,2) AS new(m,n) ON DUPLICATE KEY UPDATE b = m; INSERT INTO t SET a=1,b=2 AS new ON DUPLICATE KEY UPDATE b = new.a; This is a parser-only change; planner/executor support for resolving the alias references is not included. Ref pingcap#51650, ref pingcap#29259
ti-chi-bot
bot
added
the
release-note
|
@paultyng I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details. ⏳ This process typically takes 10-30 minutes depending on the complexity of the changes. ℹ️ Learn more details on Pantheon AI. |
|
Hi @paultyng. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
ti-chi-bot
bot
added
contribution
|
Welcome @paultyng! |
ti-chi-bot
bot
added
the
first-time-contributor
|
|
|
Hi @paultyng. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds MySQL 8.0.19+ row-alias support for Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/parser/parser.y (1)
7908-7989:⚠️ Potential issue | 🟠 MajorINSERT row aliases incorrectly accepted in REPLACE syntax due to InsertValues grammar reuse.
Line 8089 reuses
InsertValuesforREPLACE, which includesInsertRowAliasOptrules. This allows syntactically invalid REPLACE statements likeREPLACE INTO t VALUES (1) AS newandREPLACE INTO t SET a = 1 AS newto parse successfully. MySQL documents row aliases forINSERTvalues-list and SET forms (since 8.0.19), but not forREPLACE. The ReplaceIntoStmt action must reject any row aliases to comply with MySQL syntax.Add a check in the ReplaceIntoStmt action to reject aliases:
ReplaceIntoStmt: "REPLACE" TableOptimizerHintsOpt PriorityOpt IntoOpt TableName PartitionNameListOpt InsertValues { x := $7.(*ast.InsertStmt) + if x.RowAlias.O != "" || len(x.ColumnAliases) > 0 { + yylex.AppendError(ErrSyntax) + return 1 + } if $2 != nil { x.TableHints = $2.([]*ast.TableOptimizerHint) }Add negative parser tests for both forms:
REPLACE INTO ... VALUES (...) AS aliasandREPLACE INTO ... SET ... AS alias.
🧹 Nitpick comments (1)
pkg/parser/parser_test.go (1)
920-929: Add a negative guard for unsupportedINSERT ... SELECT ... AS row_aliasforms.Great positive coverage here. Please also add at least one negative case to lock the “VALUES/SET-only” contract from this PR.
✅ Suggested test additions
// row alias without ON DUPLICATE KEY UPDATE {"INSERT INTO t VALUES (1,2) AS new;", true, "INSERT INTO `t` VALUES (1,2) AS `new`"}, {"INSERT INTO t VALUES (1,2) AS new(a,b);", true, "INSERT INTO `t` VALUES (1,2) AS `new`(`a`, `b`)"}, + // row alias is not supported for INSERT ... SELECT + {"INSERT INTO t SELECT 1,2 AS new;", false, ""}, + {"INSERT INTO t SELECT 1,2 AS new(a,b);", false, ""},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/parser/parser_test.go` around lines 920 - 929, Add a negative test case alongside the existing row-alias entries in the SQL test table in parser_test.go to assert that INSERT ... SELECT ... AS <row_alias> is rejected; specifically add an entry mirroring the positive VALUES/SET cases but using an INSERT ... SELECT ... AS form (e.g., an INSERT INTO t SELECT ... AS new) and mark it as failing (ok=false) so the parser test ensures AS row_alias is only accepted for VALUES/SET forms (update the test table near the existing "AS new" rows).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/parser/parser_test.go`:
- Around line 920-929: Add a negative test case alongside the existing row-alias
entries in the SQL test table in parser_test.go to assert that INSERT ... SELECT
... AS <row_alias> is rejected; specifically add an entry mirroring the positive
VALUES/SET cases but using an INSERT ... SELECT ... AS form (e.g., an INSERT
INTO t SELECT ... AS new) and mark it as failing (ok=false) so the parser test
ensures AS row_alias is only accepted for VALUES/SET forms (update the test
table near the existing "AS new" rows).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 003c0468-0502-4c47-88ed-b86fbb865229
📒 Files selected for processing (4)
pkg/parser/ast/dml.gopkg/parser/parser.gopkg/parser/parser.ypkg/parser/parser_test.go
REPLACE does not support the AS row_alias syntax in MySQL. Since ReplaceIntoStmt reuses InsertValues, add a check to reject any row alias parsed via that shared grammar rule. Ref pingcap#51650
|
[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 |
|
|
|
/ok-to-test |
ti-chi-bot
bot
added
ok-to-test
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #67920 +/- ##
================================================
- Coverage 77.8055% 77.1559% -0.6497%
================================================
Files 1983 1965 -18
Lines 549119 549122 +3
================================================
- Hits 427245 423680 -3565
- Misses 120954 125440 +4486
+ Partials 920 2 -918
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@paultyng: 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. |
2 participants
What problem does this PR solve?
Issue Number: ref #51650, ref #29259
Problem Summary:
TiDB returns a syntax error on the MySQL 8.0.19+
INSERT ... VALUES (...) AS aliassyntax. MySQL 8.0.20 deprecatedVALUES()in favor of this syntax, so TiDB can't parse modern MySQL upserts.What changed and how does it work?
Parser-only change — adds
AS row_alias[(col_alias, ...)]to INSERT ... VALUES and INSERT ... SET.RowAliasandColumnAliasesfields toInsertStmtAST nodeInsertRowAliasOptgrammar rule inside VALUES/SET alternatives ofInsertValues(not SELECT, matching MySQL spec)Restore()for round-trip fidelity, following the existingTableSourcealias patternPlanner/executor support for resolving alias references in ON DUPLICATE KEY UPDATE is not included.
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
AS row_aliasandAS row_alias(columns...), supported for both VALUES and SET forms and usable with ON DUPLICATE KEY UPDATE.