feat(parser): support MySQL REPLACE syntax variants - #25083
Merged
Conversation
Accept the MySQL-compatible REPLACE forms that MatrixOne previously rejected at the parser: - `REPLACE INTO dst TABLE src` (equivalent to `REPLACE INTO dst SELECT * FROM src`) - `REPLACE LOW_PRIORITY INTO ...` - `REPLACE DELAYED INTO ...` The priority modifiers (LOW_PRIORITY / DELAYED, plus HIGH_PRIORITY) are parsed and ignored, since MatrixOne has no corresponding scheduling behavior; the resulting statement is identical to a plain REPLACE. The TABLE source form is rewritten into a `SELECT *` over the source table, reusing the existing `REPLACE ... SELECT` execution path, so no AST or planner changes are needed. Adds parser round-trip unit tests and end-to-end BVT cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
heni02
approved these changes
Jun 23, 2026
# Conflicts: # pkg/sql/parsers/dialect/mysql/mysql_sql.go
XuPeng-SH
requested changes
Jun 29, 2026
XuPeng-SH
left a comment
XuPeng-SH
left a comment
Contributor
There was a problem hiding this comment.
I found one blocker here.
This parser only accepts the bare TABLE table_name form (with or without a target column list), but MySQL TABLE syntax also supports ORDER BY and LIMIT, and REPLACE ... TABLE ... inherits that source form. So valid MySQL statements such as REPLACE INTO dst TABLE src ORDER BY id LIMIT 1 will still fail to parse.
The current implementation rewrites directly to tree.NewSelect(makeSelectStarFromTable(...), nil, nil), so there is nowhere to preserve ORDER BY / LIMIT even if we want to. That makes the new compatibility support incomplete.
Suggested fix:
- parse a reusable
TABLEsource form with optionalORDER BY/LIMIT; - thread those clauses into the rewritten
tree.NewSelect(...); - add parser + BVT coverage for
REPLACE ... TABLE ... ORDER BY ... LIMIT ....
I re-checked the rest of the patch and did not find another substantive code issue beyond this gap.
# Conflicts: # pkg/sql/parsers/dialect/mysql/mysql_sql.go
XuPeng-SH
approved these changes
Jul 1, 2026
XuPeng-SH
left a comment
XuPeng-SH
left a comment
Contributor
There was a problem hiding this comment.
I re-reviewed the current head from MySQL compatibility, parser-generation consistency, mergeability, and unhappy-path/resource-risk angles.
The previous blockers are addressed:
REPLACE ... (cols) TABLE srcis now supported and covered.REPLACE ... TABLE src ORDER BY ... LIMIT/OFFSET ...is now preserved through the TABLE-to-SELECT rewrite and covered.REPLACE HIGH_PRIORITYis rejected via the REPLACE-specific priority rule and an invalid parser test.- The checked-in generated parser is consistent with
mysql_sql.y.
Local checks I ran:
git diff --check origin/main...HEAD
make -B -C pkg/sql/parsers/dialect/mysql mysql_sql.go
git diff --exit-code -- pkg/sql/parsers/dialect/mysql/mysql_sql.go pkg/sql/parsers/dialect/mysql/mysql_sql.y
go test ./pkg/sql/parsers/dialect/mysql -run 'TestValid|TestInvalid|TestDebug' -count=1\ngo test ./pkg/sql/parsers/dialect/mysql -count=1\ngo test ./pkg/sql/parsers -count=1\ngit merge-tree --write-tree origin/main HEAD\n```\n\nNo further code issue found. Non-blocking: the PR description still appears to contain stale wording from the earlier `HIGH_PRIORITY` iteration, so it would be clearer to update that before merge.
iamlinjunhong
approved these changes
Jul 1, 2026
Contributor
Merge Queue Status
This pull request spent 1 hour 12 minutes 42 seconds in the queue, including 1 hour 12 minutes 24 seconds running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue(s) this PR fixes:
issue #24947
What this PR does / why we need it:
Accept the MySQL-compatible
REPLACEforms that MatrixOne previously rejected at the parser:REPLACE INTO dst TABLE src— equivalent toREPLACE INTO dst SELECT * FROM src(MySQL 8.0.19+)REPLACE LOW_PRIORITY INTO ...REPLACE DELAYED INTO ...Implementation notes
LOW_PRIORITY/DELAYED, and alsoHIGH_PRIORITY) are parsed and ignored, since MatrixOne has no corresponding scheduling behavior. The resulting statement is identical to a plainREPLACE— same execution path and semantics. This matches MySQL 8.0, whereDELAYEDis downgraded to a plainREPLACE(MatrixOne simply does not emit the deprecation warning).TABLE srcsource form is rewritten in the grammar into aSELECT *over the source table, reusing the existingREPLACE ... SELECTexecution path. No AST struct or planner changes are required.mysql_sql.gois regenerated frommysql_sql.yviamake; the large generated diff is the goyacc state-table renumbering caused by the two new grammar rules (no new shift/reduce conflicts).Tests
mysql_sql_test.gofor theTABLEform and all three priority modifiers.test/distributed/cases/dml/replace/(verified 201/201, 100%).Parent issue: #24918