fix insert ignore by daviszhen · Pull Request #26607 · matrixorigin/matrixone

daviszhen · 2026-08-03T04:14:39Z

What type of PR is this?

Which issue(s) this PR fixes:

issue #25367

What this PR does / why we need it:

ENUM 在数值比较、算术和纯数字 IN 中按成员索引计算。
SET 在数值、位运算和纯数字比较中按位图计算。
字符串函数、COALESCE、字符串比较及混合字符串 IN 保持 ENUM/SET 的显示值语义。
扩展 ENUM/SET 的 planner 回归测试，覆盖数值运算、数值比较、纯数字/混合 IN、字符串函数与 COALESCE。
更新 dtype/enum、dtype/set BVT 场景及对应结果快照。

# Conflicts: # pkg/sql/plan/base_binder.go # pkg/sql/plan/types.go

qodo-code-review · 2026-08-03T04:14:45Z

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 →

…/matrixone into 0731-fix-insert-ignore

XuPeng-SH

Deep-reviewed exact head 7532437. The previous ENUM/SET numeric-context blocker and the local/VALUES INSERT IGNORE cases are addressed, and the focused planner/function/util regressions pass. One distributed correctness blocker remains.

P1 — INSERT IGNORE adjustment state is lost at the remote-CN process boundary

The new execution-time YEAR/BIT/ENUM/SET adjustment branches all depend on statementIgnore(proc), which reads proc.GetStmtProfile().GetStatementIgnore(). The frontend sets that flag on the coordinator, but BuildProcessInfo and pipeline.ProcessInfo do not serialize the statement runtime profile, and remoterunServer.go reconstructs a fresh StmtProfile from only txnId/stmtId. Its ignore bit is therefore false.

This is reachable, not just a codec concern: INSERT IGNORE ... SELECT adds the assignment casts in a PROJECT above the source; compilePlanScope keeps PROJECT on each table-scan scope, and table-scan scopes are Remote over the generated CN nodes. If an invalid YEAR/BIT/ENUM/SET value is evaluated on a remote source scope, the same cast that adjusts locally takes its strict error branch and aborts the statement. The added BVT covers INSERT ... SELECT but does not force that projection onto a different CN, so it does not close this execution shape.

A temporary boundary regression on this head set statementRuntimeIgnore=true, built ProcessInfo, reconstructed the remote StmtProfile exactly as remoterunServer does, and asserted the remote flag. It fails: expected true, got false.

Please carry the required statement runtime semantics through ProcessInfo and restore them before remote pipeline execution, with rolling-upgrade-safe behavior, and add a forced remote/multi-CN regression alongside the local control. A minimal public witness is strict mode plus INSERT IGNORE INTO dst(bit(4)) SELECT 31 FROM a source placed on another CN: expected stored 15 without statement failure; the remote projection currently sees ignore=false and raises out-of-range.

Unhappy-path audit: Q1 ownership/transfer fails at coordinator StmtProfile -> ProcessInfo -> remote StmtProfile; Q2 aborts before the write path can apply ignore semantics; Q3 adds no growth risk, but the state-transfer contract is incomplete. diff --check is clean; focused planner, function, and util tests pass; CI is green.

daviszhen · 2026-08-04T10:02:33Z

Deep-reviewed exact head 7532437. The previous ENUM/SET numeric-context blocker and the local/VALUES INSERT IGNORE cases are addressed, and the focused planner/function/util regressions pass. One distributed correctness blocker remains.

P1 — INSERT IGNORE adjustment state is lost at the remote-CN process boundary

The new execution-time YEAR/BIT/ENUM/SET adjustment branches all depend on statementIgnore(proc), which reads proc.GetStmtProfile().GetStatementIgnore(). The frontend sets that flag on the coordinator, but BuildProcessInfo and pipeline.ProcessInfo do not serialize the statement runtime profile, and remoterunServer.go reconstructs a fresh StmtProfile from only txnId/stmtId. Its ignore bit is therefore false.

This is reachable, not just a codec concern: INSERT IGNORE ... SELECT adds the assignment casts in a PROJECT above the source; compilePlanScope keeps PROJECT on each table-scan scope, and table-scan scopes are Remote over the generated CN nodes. If an invalid YEAR/BIT/ENUM/SET value is evaluated on a remote source scope, the same cast that adjusts locally takes its strict error branch and aborts the statement. The added BVT covers INSERT ... SELECT but does not force that projection onto a different CN, so it does not close this execution shape.

A temporary boundary regression on this head set statementRuntimeIgnore=true, built ProcessInfo, reconstructed the remote StmtProfile exactly as remoterunServer does, and asserted the remote flag. It fails: expected true, got false.

Please carry the required statement runtime semantics through ProcessInfo and restore them before remote pipeline execution, with rolling-upgrade-safe behavior, and add a forced remote/multi-CN regression alongside the local control. A minimal public witness is strict mode plus INSERT IGNORE INTO dst(bit(4)) SELECT 31 FROM a source placed on another CN: expected stored 15 without statement failure; the remote projection currently sees ignore=false and raises out-of-range.

Unhappy-path audit: Q1 ownership/transfer fails at coordinator StmtProfile -> ProcessInfo -> remote StmtProfile; Q2 aborts before the write path can apply ignore semantics; Q3 adds no growth risk, but the state-transfer contract is incomplete. diff --check is clean; focused planner, function, and util tests pass; CI is green.

修复 ENUM/SET 数值上下文：不再只依赖有限 AST 形式；CAST AS SIGNED、ABS、数值列比较、BETWEEN、非字面量 IN、一元数值比较均使用内部数值，而字符串上下文仍使用显示值。
完成 INSERT IGNORE 的 YEAR/BIT/ENUM/SET 调整语义：严格模式下非法值不再直接中止，按 MySQL 兼容规则调整并写入。
修复远端 CN 丢失 IGNORE 状态：将该语义写入 pipeline.ProcessInfo，远端解码并重建 StmtProfile，使 INSERT IGNORE … SELECT 的远端投影也走调整逻辑。
增加 UT、强制多 CN 集成测试及 BVT：验证远端 BIT(4) 溢出值可成功写入 15；补充 ENUM/SET 数值与字符串上下文对照。

XuPeng-SH

Re-review of c716491: the previous blocker is resolved.\n\nThe statement-level IGNORE flag is now carried through ProcessInfo, restored by both codecService.Decode and the remote-run server, and re-serialized on further hops. Missing field data from an older sender decodes to false, so mixed-version execution fails closed instead of silently applying IGNORE semantics.\n\nI also verified the forced multi-CN regression actually sets ForceScanOnMultiCN and exercises an assignment cast on a remote scan scope.\n\nFocused local validation passed:\n- pkg/vm/process: ProcessInfo build/codec round-trip + legacy absent-field case\n- pkg/sql/compile: remote helper/new compile restoration\n- pkg/tests/dml: TestInsertIgnoreSpecialTypeOnRemoteCN\n\nNo blocking correctness issue found.

mergify · 2026-08-04T10:59:32Z

Merge Queue Status

✅ Entered queue — 2026-08-04 10:59 UTC · Rule: main · triggered by rule Automatic queue on approval for main
🟠 Checks running · in-place
🚫 Left the queue — 2026-08-04 11:33 UTC · at 50f55a76b12151a2d241ba45c7cd73e0086f23f6

This pull request spent 34 minutes 18 seconds in the queue, with no time running CI.

Waiting for

All conditions

Reason

Pull request #26607 has been dequeued

Pull request from fork cannot be queued. This pull request comes from a fork, and Mergify needs the author's permission to update its branch.

The author needs to enable "Allow edits from maintainers" on this pull request.

Failing checks:

Hint

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

Requeue this pull request

XuPeng-SH

Re-review of new head 50f55a7 after the main-branch merge.\n\nc71649112e365edd6831725d6ba46a4962f866dd is the first parent. The merge adds only already-merged main changes and does not modify this PR's protobuf, ProcessInfo codec, remote-run restoration, or INSERT IGNORE regression files; no conflict resolution altered the reviewed fix.\n\nExact-head focused validation passed again:\n- ProcessInfo build/codec round-trip and legacy absent-field behavior\n- remote helper/new compile statement state restoration\n- forced multi-CN INSERT IGNORE special-type integration test\n\nThe prior approval remains valid on the new head; no blocking issue found.

daviszhen added 6 commits July 31, 2026 18:39

update

b64794c

update

68206db

update

d390235

update

3ec76e5

update

6f919cd

Merge branch 'main' into 0731-fix-insert-ignore

b8228aa

# Conflicts: # pkg/sql/plan/base_binder.go # pkg/sql/plan/types.go

daviszhen requested review from XuPeng-SH, aunjgr and heni02 as code owners August 3, 2026 04:14

daviszhen temporarily deployed to ci August 3, 2026 04:14 — with GitHub Actions Inactive

daviszhen temporarily deployed to ci August 3, 2026 04:15 — with GitHub Actions Inactive

mergify Bot added the kind/bug Something isn't working label Aug 3, 2026

matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Aug 3, 2026

daviszhen had a problem deploying to ci August 3, 2026 04:49 — with GitHub Actions Failure

update

55d6bcf

daviszhen had a problem deploying to ci August 3, 2026 06:36 — with GitHub Actions Error

daviszhen temporarily deployed to ci August 3, 2026 06:36 — with GitHub Actions Inactive

daviszhen had a problem deploying to ci August 3, 2026 06:36 — with GitHub Actions Error

Merge branch 'main' into 0731-fix-insert-ignore

ca48d92

daviszhen had a problem deploying to ci August 3, 2026 06:36 — with GitHub Actions Failure

daviszhen temporarily deployed to ci August 3, 2026 06:36 — with GitHub Actions Inactive

daviszhen had a problem deploying to ci August 4, 2026 04:55 — with GitHub Actions Error

daviszhen temporarily deployed to ci August 4, 2026 04:55 — with GitHub Actions Inactive

Merge branch 'main' into 0731-fix-insert-ignore

4497577

daviszhen had a problem deploying to ci August 4, 2026 04:56 — with GitHub Actions Error

daviszhen temporarily deployed to ci August 4, 2026 04:56 — with GitHub Actions Inactive

matrix-meow added size/XXL Denotes a PR that changes 2000+ lines and removed size/XL Denotes a PR that changes [1000, 1999] lines labels Aug 4, 2026

daviszhen temporarily deployed to ci August 4, 2026 04:56 — with GitHub Actions Inactive

daviszhen had a problem deploying to ci August 4, 2026 05:44 — with GitHub Actions Failure

daviszhen added 2 commits August 4, 2026 14:50

update

39cb6b7

Merge branch '0731-fix-insert-ignore' of https://github.com/daviszhen…

acaa808

…/matrixone into 0731-fix-insert-ignore

daviszhen had a problem deploying to ci August 4, 2026 06:50 — with GitHub Actions Error

daviszhen added 3 commits August 4, 2026 14:50

Merge branch 'main' into 0731-fix-insert-ignore

86ad8fe

update ut

fcf5052

Merge branch '0731-fix-insert-ignore' of https://github.com/daviszhen…

7532437

…/matrixone into 0731-fix-insert-ignore

XuPeng-SH requested changes Aug 4, 2026

View reviewed changes

daviszhen added 3 commits August 4, 2026 17:34

update

106af34

update

f1a90e7

Merge branch 'main' into 0731-fix-insert-ignore

c716491

XuPeng-SH approved these changes Aug 4, 2026

View reviewed changes

Merge branch 'main' into 0731-fix-insert-ignore

50f55a7

XuPeng-SH approved these changes Aug 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix insert ignore - #26607

fix insert ignore#26607
XuPeng-SH merged 33 commits into
matrixorigin:mainfrom
daviszhen:0731-fix-insert-ignore

daviszhen commented Aug 3, 2026

Uh oh!

qodo-code-review Bot commented Aug 3, 2026

Uh oh!

XuPeng-SH left a comment

Uh oh!

daviszhen commented Aug 4, 2026

Uh oh!

XuPeng-SH left a comment

Uh oh!

mergify Bot commented Aug 4, 2026 •

edited

Loading

Uh oh!

XuPeng-SH left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

daviszhen commented Aug 3, 2026

What type of PR is this?

Which issue(s) this PR fixes:

What this PR does / why we need it:

Uh oh!

qodo-code-review Bot commented Aug 3, 2026

Qodo reviews are paused for this user.

Uh oh!

XuPeng-SH left a comment

Choose a reason for hiding this comment

Uh oh!

daviszhen commented Aug 4, 2026

Uh oh!

XuPeng-SH left a comment

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Reason

Hint

Uh oh!

XuPeng-SH left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mergify Bot commented Aug 4, 2026 •

edited

Loading