Skip to content

fix: harden prepare long data parsing#23980

Merged
mergify[bot] merged 5 commits intomatrixorigin:mainfrom
XuPeng-SH:fix-prepare-long-data-and-bounds
Mar 28, 2026
Merged

fix: harden prepare long data parsing#23980
mergify[bot] merged 5 commits intomatrixorigin:mainfrom
XuPeng-SH:fix-prepare-long-data-and-bounds

Conversation

@XuPeng-SH
Copy link
Copy Markdown
Contributor

@XuPeng-SH XuPeng-SH commented Mar 27, 2026

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #23979

What this PR does / why we need it:

This PR fixes three categories of issues:

1. Harden prepared-statement binary protocol edges

  • Repeated COM_STMT_SEND_LONG_DATA chunks for the same parameter now append instead of overwriting, matching MySQL protocol semantics.
  • Truncated COM_STMT_EXECUTE packets now return a malformed-packet error instead of risking a panic while reading the new parameter bound flag.

2. Fix data race on shared TableDef.TblId (detected by -race in TestDeleteAndSelect)

  • PreInsert.refreshAutoIncrementTableID() wrote directly to the shared *plan.TableDef.TblId during Prepare().
  • MultiUpdate.Prepare() read the same TblId concurrently in a different ants worker pool goroutine.
  • Fix: Store the refreshed TblId in a local container field (ctr.tblId) instead of mutating the shared plan object.

3. Harden partition operators against shared plan object mutations

  • PartitionInsert and PartitionDelete temporarily mutated the shared ObjRef.ObjName during partition iteration (save/restore via defer) -- replaced with stack-local copies.
  • PartitionMultiUpdate.writeTable() directly mutated shared MultiUpdateCtx entries -- now uses clone() to match the writeS3 code path.
  • Also fixed a pre-existing copy-paste bug in MultiUpdateCtx.clone() where PartitionCols was incorrectly copying DeleteCols.

@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Mar 27, 2026
@mergify mergify Bot added the kind/bug Something isn't working label Mar 27, 2026
XuPeng-SH and others added 5 commits March 28, 2026 14:32
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PreInsert.refreshAutoIncrementTableID wrote directly to the shared
*plan.TableDef.TblId, which is the same pointer held by MultiUpdate.
When both operators run Prepare() concurrently in the ants worker pool,
the race detector fires.

Store the refreshed TblId in a local container field (ctr.tblId) instead
of mutating the shared plan object.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PartitionInsert and PartitionDelete temporarily mutated the shared
ObjRef.ObjName during their partition iteration loop (save/restore via
defer).  PartitionMultiUpdate.writeTable directly mutated shared
MultiUpdateCtx entries unlike the writeS3 path which already cloned.

These mutations are safe today (merge scope is single-threaded), but are
fragile: any future concurrent reader of the same ObjRef/TableDef would
race.  Use stack copies (Insert/Delete) or clone() (MultiUpdate) to
keep shared plan objects immutable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 28, 2026

Merge Queue Status

  • Entered queue2026-03-28 13:59 UTC · Rule: main
  • Checks skipped · PR is already up-to-date
  • Merged2026-03-28 13:59 UTC · at a7c444b64d6dab40993c24bf41e0eaf184ebbe18

This pull request spent 7 seconds in the queue, with no time running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage

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/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants