Skip to content

fix(frontend): preserve params across prepared multi-SET - #26705

Draft
VioletQwQ-0 wants to merge 6 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-26685-prepared-multiset
Draft

fix(frontend): preserve params across prepared multi-SET#26705
VioletQwQ-0 wants to merge 6 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-26685-prepared-multiset

Conversation

@VioletQwQ-0

@VioletQwQ-0 VioletQwQ-0 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Important

Draft dependency: this PR is stacked on #26694 (16c22842c93e2eca446c2ddab4ec58ba18180ea4). Its diff temporarily includes that PR because the dependency branch belongs to another fork. After #26694 merges, this branch will be rebased onto the latest main and fully revalidated.

What type of PR is this?

  • BUG

Which issue(s) this PR fixes:

issue #26685

What this PR does / why we need it:

Prepared multi-assignment SET evaluated each RHS through a derived SELECT in the session's shared Process. Cleanup after the first expression freed the outer prepared-parameter vector, so the next global parameter ordinal panicked. The recovered error also left the first session variable updated.

Changes:

  • Fence the complete prepared-parameter state (vector, binary flags, and ownership) while a derived query frees its own resources, then restore the outer owner on every return or panic path.
  • Evaluate every RHS of a prepared SET before applying assignments, so expression and cleanup failures do not partially update variables. Ordinary non-prepared SET retains its sequential behavior.
  • Reject prepared multi-assignment SET containing a system variable before any RHS evaluation or mutation; prepared single-system assignment remains supported. This fail-closed boundary prevents apply-time conversion or side-effect failures from leaving partial state.
  • Add a defensive missing-parameter error instead of dereferencing a nil vector.
  • Extend binary prepared SET from one assignment to var_assignment_list.
  • Cover text and real binary protocol execution, rebinding, composite expressions, numeric/string/NULL values, invalid arity, failure atomicity, connection reuse, and final ownership cleanup.

Non-goals:

  • This PR does not add a general rollback framework for apply-time side effects of mixed session/global/system-variable assignments; prepared multi-assignment statements containing system variables are intentionally unsupported.

Validation:

  • Base behavior: FAIL on fix(frontend): allow binary prepare for single SET assignment #26694 head 16c22842c93e2eca446c2ddab4ec58ba18180ea4 — the second parameter was unavailable after derived-query cleanup, while the first target variable had already changed.
  • Runtime regression: PASS on exact head, covering text PREPARE/EXECUTE and real COM_STMT_PREPARE/EXECUTE, rebinding, NULL, RHS failure, mixed-target prevalidation, unchanged terminal state, connection reuse, and single-system compatibility.
  • Focused race: PASS, -race -count=100 for Process ownership and Frontend prepared SET coverage.
  • Owning packages: PASS on exact head, Frontend full normal and Process full race; the repair-relevant focused Frontend race also passed 100 repetitions.
  • BVT: PASS locally on the latest built mo-service, test/distributed/cases/prepare/prepare_all.sql (378 success, 1 existing ignore, 0 failed/abnormal).
  • Build/static: PASS, make build, focused go vet, and git diff --check.
  • Preflight: PASS review=PASS validation=PASS for exact head e019869e93054c40c84b99bae7799e77f959a7f9.

QA Decision

  • required: yes
  • reason: user-visible SQL compatibility, a deterministic recovered panic, and session-state failure atomicity
  • production_entrypoint: text PREPARE/EXECUTE and binary COM_STMT_PREPARE/EXECUTE
  • automated_terminal_coverage: user/system variables' final values, repeated binding, NULL, invalid arity, failing second RHS, mixed-target rejection before mutation, single-system compatibility, connection reuse, and parameter ownership cleanup
  • remaining_validation: verify text and binary drivers on a deployed build after merge, including repeated rebinding and failure-state inspection

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