Skip to content

fix(plan): evaluate REPLACE ... SET RHS column references as DEFAULT(col)#25084

Open
ck89119 wants to merge 5 commits into
matrixorigin:mainfrom
ck89119:issue-24945-main
Open

fix(plan): evaluate REPLACE ... SET RHS column references as DEFAULT(col)#25084
ck89119 wants to merge 5 commits into
matrixorigin:mainfrom
ck89119:issue-24945-main

Conversation

@ck89119

@ck89119 ck89119 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • BUG

Which issue(s) this PR fixes:

issue #24945

What this PR does / why we need it:

Subtask of #24918 (section 1). For REPLACE ... SET col = col + 1, MySQL treats the RHS reference to a target-table column as DEFAULT(col), not the current conflicting row value. Missing columns also fall back to their defaults, matching normal REPLACE insert semantics.

Before this PR, MatrixOne reported invalid input: ambiguous column reference 'v' and left the old row unchanged:

create table t_set(id int primary key, v int default 5, note varchar(20) default 'd');
insert into t_set values (1,10,'old');
replace into t_set set id = 1, v = v + 1;
select * from t_set;   -- expected 1 | 6 | d

Root cause

The parser lowers REPLACE ... SET a = x, b = y to tree.Replace{Columns, Rows: ValuesClause}, i.e. the same AST shape as the VALUES form. When buildValueScan binds the value expressions it uses a DefaultBinder whose bind context is nil, so any column reference hits the b.ctx == nil branch in baseBindColRef and fails with "ambiguous column reference".

Fix

  • Add an IsSetFormat marker to tree.Replace, set only in the SET grammar branch, so the SET-only semantics can be distinguished from the VALUES form (which still rejects column references, matching MySQL).
  • Add a ReplaceValueBinder that resolves every RHS column reference to that column's DEFAULT(col) expression. SET-form value binding is routed through it via a new colRefAsDefault flag threaded from bindReplaceinitInsertReplaceStmtbuildValueScan. INSERT and the VALUES form are unchanged.

Tests

  • Unit tests: ReplaceValueBinder (default value resolution, case-insensitive lookup, unknown column error, unsupported bindings) — 100% coverage of the new file; planner test TestReplaceSetColRefAsDefault.
  • BVT: new cases in dml/replace/replace.test covering provided/omitted columns and references to other columns' defaults (set id=1,v=v+11|6|d, set id=2,v=v*22|10|d, set id=1,a=b+11|8|7); verified with mo-tester (198/198), replace_statement 50/50 with no regression.

🤖 Generated with Claude Code

…col)

For `REPLACE ... SET col = col + 1`, MySQL treats the RHS reference to a
target-table column as DEFAULT(col), not the current conflicting row value.
MatrixOne instead reported `ambiguous column reference 'v'` and left the old
row unchanged.

The parser lowers `REPLACE ... SET` to the same Columns + ValuesClause shape
as the VALUES form, so the value expressions are bound by buildValueScan with
a DefaultBinder whose bind context is nil; any column reference therefore hits
the `ctx == nil` branch in baseBindColRef and fails.

Add an IsSetFormat marker to tree.Replace (set only in the SET grammar branch)
so the SET-only semantics can be distinguished from the VALUES form, and route
SET-form value binding through a new ReplaceValueBinder that resolves every
column reference to that column's DEFAULT expression. Missing columns continue
to fall back to their defaults like a normal REPLACE insert. The VALUES form is
unchanged and still rejects column references, matching MySQL.

Adds planner/binder unit tests and BVT coverage for provided and omitted
columns and for references to other columns' defaults.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

ck89119 and others added 2 commits June 23, 2026 11:21
- fix "agregate" -> "aggregate" typo in the unsupported aggregate error
- add `var _ Binder = (*ReplaceValueBinder)(nil)` compile-time interface check
- strengthen binder tests: assert DEFAULT(v) resolves to literal 5 for both
  the case-insensitive lookup and the `v + 1` binding (structure check, since
  constant folding happens later in the optimizer)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	pkg/sql/parsers/dialect/mysql/mysql_sql.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants