Skip to content

refactor(plan): migrate remaining legacy UPDATE special cases - #26596

Merged
XuPeng-SH merged 4 commits into
matrixorigin:mainfrom
ck89119:issue-26342-main
Aug 3, 2026
Merged

refactor(plan): migrate remaining legacy UPDATE special cases#26596
XuPeng-SH merged 4 commits into
matrixorigin:mainfrom
ck89119:issue-26342-main

Conversation

@ck89119

@ck89119 ck89119 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #26342

What this PR does / why we need it:

This PR removes the remaining legacy UPDATE planner fallbacks owned by issue #26342.

  • Route primary-key and unique-key updates on mo_pubs and mo_subs through the modern MULTI_UPDATE planner.
  • Route legal SET ... AUTO_INCREMENT updates through MULTI_UPDATE while preserving SET value conversion and PRE_INSERT handling.
  • Keep auto-increment foreign-key updates on their existing legacy route, with a precise auto_increment_foreign_key telemetry reason.
  • Add planner regressions for pub/sub keys, SET conversion, PRE_INSERT, MULTI_UPDATE, and the retained foreign-key control path.

Validation:

  • Focused UPDATE planner tests.
  • Full pkg/sql/plan tests with coverage.
  • Full dependent pkg/sql/compile tests.
  • go build and go vet for pkg/sql/plan and pkg/sql/compile.
  • git diff --check.

@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 →

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep-reviewed at cb39c93. No blocking findings.

Checked the removed legacy routes end to end:

  • mo_pubs/mo_subs PK/UK updates now build modern MULTI_UPDATE contexts, including the mo_subs hidden unique index; the integration test covers key migration, duplicate-key atomicity, ordinary/join updates, and tenant authorization.
  • SET AUTO_INCREMENT preserves SET conversion and PRE_INSERT. On an exact-head temporary service I compared explicit, 0, NULL, and DEFAULT assignments against the base legacy path; plans changed as intended and row results remained identical.
  • The remaining auto-increment + affected-FK case keeps an explicit typed legacy reason instead of becoming a catch-all.

Exact-head validation passed: full pkg/sql/plan and pkg/sql/compile tests, TestIssue26342MoSubsModernUpdate, go vet for pkg/sql/plan and pkg/tests/issues, and a full mo-service build.

@ck89119

ck89119 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio refresh

@mergify

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

refresh

✅ Pull request refreshed

@mergify mergify Bot added the queued label Aug 3, 2026
@mergify

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-08-03 07:17 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🟠 Checks running · in-place
  • 🚫 Left the queue2026-08-03 08:09 UTC · at 72a21bb86b5abb036b4477345f3821edaf2fc092

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

Reason

Pull request #26596 has been merged manually at 1878a38

Hint

You were too fast!

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

  • Requeue this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants