Skip to content

fix: guard prepared phy scope shape mismatch - #24512

Merged
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
jiangxinmeng1:fix/24510-prepare-phy-scope-main
May 21, 2026
Merged

fix: guard prepared phy scope shape mismatch#24512
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
jiangxinmeng1:fix/24510-prepare-phy-scope-main

Conversation

@jiangxinmeng1

@jiangxinmeng1 jiangxinmeng1 commented May 20, 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) does this PR fix or relate to?

Fixes #24510

What this PR does / why we need it:

Summary

  • return false when prepared physical plan local scope/pre-scope/operator shape differs from the runtime scope
  • allow AnalyzeExecPlan to regenerate the physical plan instead of panicking on cached plan reuse
  • add regression coverage for local scope and pre-scope shape mismatches

Tests

  • CGO_CFLAGS="-I$(pwd)/thirdparties/install/include" CGO_LDFLAGS="-L$(pwd)/thirdparties/install/lib" LD_LIBRARY_PATH="$(pwd)/thirdparties/install/lib:${LD_LIBRARY_PATH}" go test ./pkg/sql/compile -count=1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@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 →

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

Review: cached physical-plan shape guard

Approve.

No blocking issues found. The added guards correctly avoid the reported cached physical-plan shape panics and allow regeneration through the existing UpdatePreparePhyPlan=false path.

@mergify mergify Bot added the queued label May 21, 2026
@mergify

mergify Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-05-21 03:39 UTC · Rule: main
  • Checks passed · in-place
  • Merged2026-05-21 04:45 UTC · at b70c1393db440e0581350aa71f8dae608ee2eaf1 · squash

This pull request spent 1 hour 5 minutes 27 seconds in the queue, including 1 hour 3 minutes 53 seconds 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]
  • github-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

@mergify
mergify Bot merged commit 61eb2e2 into matrixorigin:main May 21, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label May 21, 2026
ULookup pushed a commit to ULookup/matrixone that referenced this pull request May 26, 2026
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 kind/enhancement size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: prepared TPCC SELECT FOR UPDATE panics in UpdatePreparePhyScope when cached physical scope shape changes

4 participants