Skip to content

planner: fix the global binding is not working when using Prepared Statement with "select ... as col ... group by col"#69766

Open
qw4990 wants to merge 1 commit into
pingcap:masterfrom
qw4990:fix-57992
Open

planner: fix the global binding is not working when using Prepared Statement with "select ... as col ... group by col"#69766
qw4990 wants to merge 1 commit into
pingcap:masterfrom
qw4990:fix-57992

Conversation

@qw4990

@qw4990 qw4990 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #57992

Problem Summary: planner: fix the global binding is not working when using Prepared Statement with "select ... as col ... group by col"

What changed and how does it work?

planner: fix the global binding is not working when using Prepared Statement with "select ... as col ... group by col"

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

  • Bug Fixes
    • Improved prepared-statement plan caching by preserving binding information for more reliable subsequent executions.
    • Avoided redundant SQL binding checks when a valid cached match is already available.

@ti-chi-bot ti-chi-bot Bot added the release-note-none Denotes a PR that doesn't merit a release note. label Jul 10, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign 0xpoe for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/planner SIG: Planner labels Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b2862213-57bc-48cb-b7a9-77c095715a89

📥 Commits

Reviewing files that changed from the base of the PR and between 4894ac0 and 999b280.

📒 Files selected for processing (2)
  • pkg/bindinfo/binding.go
  • pkg/planner/core/plan_cache_utils.go

📝 Walkthrough

Walkthrough

Prepared statement plan cache entries now retain binding-matching context. Test-mode binding matching reuses an existing per-statement cache entry when no override is provided.

Changes

Prepared statement binding

Layer / File(s) Summary
Prepared plan binding context
pkg/planner/core/plan_cache_utils.go
Prepared statements compute a normalized binding digest and referenced table names, then store them in PlanCacheStmt.BindingInfo.
Cached binding match reuse
pkg/bindinfo/binding.go
The test-mode path returns cached binding results when a cache entry exists and no override is supplied.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit watched the bindings hop,
Through prepared plans from start to stop.
Cached results now spring ahead,
With tables and digests neatly fed.
“No second match!” the rabbit said.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing global binding for prepared statements with alias-based GROUP BY queries.
Description check ✅ Passed The PR description follows the template, includes the issue number, problem summary, changes, and a test checklist item.
Linked Issues check ✅ Passed The code changes address the reported prepared-statement binding gap and align with the issue's expected behavior.
Out of Scope Changes check ✅ Passed The modified files are directly related to binding matching and plan cache handling, with no clear unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: context deadline exceeded"
level=error msg="Timeout exceeded: try increasing it by passing --timeout option"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.2886%. Comparing base (4894ac0) to head (999b280).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #69766        +/-   ##
================================================
- Coverage   76.3192%   74.2886%   -2.0307%     
================================================
  Files          2041       2051        +10     
  Lines        560139     575388     +15249     
================================================
- Hits         427494     427448        -46     
- Misses       131744     147692     +15948     
+ Partials        901        248       -653     
Flag Coverage Δ
integration 40.8643% <58.3333%> (+1.1590%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 60.4471% <ø> (ø)
parser ∅ <ø> (∅)
br 47.4060% <ø> (-15.3154%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

planner: the global binding is not working when using Prepared Statement with "select ... as col ... group by col"

1 participant