Skip to content

fix(plan): allow correlated update target subqueries - #26605

Open
ck89119 wants to merge 6 commits into
matrixorigin:mainfrom
ck89119:issue-26548-main
Open

fix(plan): allow correlated update target subqueries#26605
ck89119 wants to merge 6 commits into
matrixorigin:mainfrom
ck89119:issue-26548-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 #26548

What this PR does / why we need it:

The MySQL 1093 compatibility check treated every direct read of an UPDATE target table inside a subquery as an uncorrelated self-read. This regressed the correlated UPDATE behavior previously verified in #24824.

This change makes the check scope-aware:

  • correlated target-table subqueries are allowed when they reference the visible outer UPDATE target name or alias;
  • update-target qualifiers are tied to the table occurrence actually modified by SET, so read-only self-join aliases cannot grant a correlation exemption;
  • target context is preserved through nested JOIN ON expressions and SELECT wrapper clauses such as ORDER BY, LIMIT, and TimeWindow;
  • JOIN ON qualifier visibility follows the current join's left and right inputs, so later FROM aliases cannot shadow outer target references prematurely;
  • CROSS APPLY table-function arguments participate in outer-target correlation analysis while left-input aliases retain lateral shadowing semantics;
  • local aliases correctly shadow outer target qualifiers across nested SELECT scopes;
  • UNION branches retain independent correlation decisions while wrapper-level correlation applies to the complete query block;
  • uncorrelated target-table reads continue to return error 1093;
  • target identity remains isolated for same-named tables from different schemas.

The check remains planner-only and adds no execution-path overhead.

Validation:

  • GOWORK=off go list -mod=readonly ./pkg/sql/plan
  • GOWORK=off go build -mod=readonly ./pkg/sql/plan
  • GOWORK=off go vet -mod=readonly ./pkg/sql/plan
  • golangci-lint run -c .golangci.yml ./pkg/sql/plan/...
  • .agents/skills/mo-dev/scripts/mo-cgo-test -count=1 -timeout=120s ./pkg/sql/plan
  • changed production statement coverage: 176/218 = 80.73%

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

Request changes: I found two uncovered scope paths that violate the intended target-correlation invariant.

  1. Blocker: self-join source aliases are promoted to update-target aliases. mysqlUpdateTargetQualifiers walks every occurrence in stmt.Tables and records every alias whose resolved physical identity matches one of the modified targets. getUpdateTableInfo has already narrowed that target list to aliases actually assigned by SET, so a second read-only occurrence of the same table must not become another target qualifier. Reproducer on this head:

    UPDATE nation AS dst
    JOIN nation AS src ON dst.n_nationkey = src.n_nationkey
    SET dst.n_name = (
    SELECT max(inner_n.n_name)
    FROM nation AS inner_n
    WHERE inner_n.n_regionkey = src.n_regionkey
    )

Only dst is modified and the subquery correlates only to the outer read-only src occurrence, not to dst. It should therefore remain ER_UPDATE_TABLE_USED, but BuildPlan returns nil because both dst and src are attached to the same mysqlDMLTarget. Please preserve update-target occurrence/alias identity when constructing the qualifier map and add this negative regression.

  1. Blocker: correlation inside CROSS APPLY table-function arguments is not inspected. mysqlTableExprReferencesOuterQualifier recurses through ApplyTableExpr, but the right branch eventually reaches tree.TableFunction and no case walks its Func arguments. This target-correlated query is consequently still rejected with 1093:

    UPDATE nation AS dst
    SET n_name = (
    SELECT max(src.n_name)
    FROM nation AS src
    CROSS APPLY generate_series(dst.n_nationkey, dst.n_nationkey) AS g
    )

The equivalent SELECT shape plans successfully, so dst is a valid visible outer reference; only the compatibility pre-check rejects the UPDATE. Please cover table-function argument expressions and add a positive regression.

Validation performed at cd9730b: full pkg/sql/plan tests pass; the focused compatibility tests pass at count 20; go list, go build, go vet, and diff check pass. Both counterexamples were also exercised through full BuildPlan.

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

Re-reviewed the exact new head. The previous two blockers are closed: target qualifiers now come only from the modified table occurrences, so a read-only self-join alias cannot grant the exemption; CROSS APPLY table-function arguments are included in correlation analysis with left-side shadowing preserved. I also checked the surrounding nested JOIN/UNION/wrapper/local-shadow cases and found no new reachable violation. Focused compatibility tests passed at count=20, the full pkg/sql/plan suite passed, and go list/build/vet plus diff-check were clean.

@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 10:37 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🟠 Preparing checks
  • ⏳ Merge · ETA: 2026-08-03 11:42 UTC 🚀

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 queued size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants