planner: fix prepared plan cache after table rename - #71042
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe schema-change path resets and rebuilds prepared statement metadata. A shared helper collects current table information. A test covers rename, recreate, and drop operations. The test target shard count increases from 8 to 9. ChangesPrepared plan cache schema refresh
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The schema-refresh change includes coverage for the rename, replacement, and old-table drop flow. No unresolved merge-blocking issue is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. A rabbit checks the table trail Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #71042 +/- ##
================================================
- Coverage 76.3176% 72.4458% -3.8718%
================================================
Files 2041 2102 +61
Lines 556081 596713 +40632
================================================
+ Hits 424388 432294 +7906
- Misses 130793 163131 +32338
- Partials 900 1288 +388
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
7a0b49d to
6cc78f4
Compare
098ea1c to
5a00df4
Compare
5a00df4 to
083097d
Compare
|
/test check-dev2 |
1 similar comment
|
/test check-dev2 |
What problem does this PR solve?
Issue Number: close #71041
Problem Summary:
A prepared plan cache entry can retain stale table metadata after a table is renamed. If a new table is created with the original name and the renamed table is later dropped, executing the prepared statement can incorrectly return error 8113 for the dropped table.
This is a regression introduced by the metadata-lock support for plan cache:
70a825397f3b84e2926684What changed and how does it work?
After a schema change, re-preprocess the prepared AST with a fresh resolve context and rebuild the schema-dependent metadata.
The metadata-lock/cache-key table set is rebuilt from the complete
StmtCtx.RelatedTableIDsdependency set after a fresh plan-builder dependency-discovery pass. This preserves dependencies expanded from views and other resolved plan inputs; the AST-onlystmt.tableslist is retained only for AST-derived plan-cache information. The table metadata conversion is shared with initial plan-cache construction.This fixes the rename/drop/create sequence while preserving the original metadata-lock behavior introduced by #51897. The additional discovery build runs only on the schema-change reprepare path.
Regression and release impact
The introducing change is present through equivalent commits in the following code lines, so the rename regression should be considered affected unless this fix is backported:
84e2926684is present from v7.5.2 onward in the inspected tags.e39969afee49/ *: add metadata lock when using the plan cache (#51897) #52955 is present.70a825397f3bis in the release lineage; the pre-fix reprepare path is present in inspected tags.84eobject was cherry-picked there.25.10tag or branch exists in the repository snapshot used for this analysis, so its impact cannot be confirmed here.No inspected release ref contains this fix yet.
Check List
Tests
go test -tags=intest ./pkg/executor/test/plancache -run '^TestPreparedPlanCacheRenameCreateDropOldName$' -count=1go test -tags=intest ./pkg/executor/test/plancache -count=1Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
Bug Fixes
Tests