Skip to content

planner: warn view-style hints on CTE references#68426

Open
hawkingrei wants to merge 1 commit into
pingcap:masterfrom
hawkingrei:issue-64570-qb-name-cte-warning
Open

planner: warn view-style hints on CTE references#68426
hawkingrei wants to merge 1 commit into
pingcap:masterfrom
hawkingrei:issue-64570-qb-name-cte-warning

Conversation

@hawkingrei
Copy link
Copy Markdown
Member

@hawkingrei hawkingrei commented May 16, 2026

What problem does this PR solve?

Issue Number: close #64570

Problem Summary:

View-style optimizer hints that target a CTE reference can be collected before name resolution, but the CTE builder path does not consume the forwarded view hints. As a result, some hints are silently dropped and only the qb_name hint may be reported as unused.

What changed and how does it work?

This PR detects view-style QB_NAME hints when a table name resolves to a CTE reference. If the first hinted table matches the CTE reference in the current query block, the forwarded table hints are reported as ignored due to an unknown query block name.

The hint is still not applied to the CTE body. This keeps the existing plan behavior unchanged while making the warning surface consistent.

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.

Fix an issue that view-style optimizer hints targeting CTE references could be silently ignored instead of reporting an unknown query block warning.

Summary by CodeRabbit

  • Tests

    • Added test cases validating optimizer hint behavior with Common Table Expressions.
  • Bug Fixes

    • Enhanced warning messages to clarify when optimizer hints cannot be applied to Common Table Expressions due to unknown query block references.

Review Change Stack

@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/needs-triage-completed sig/planner SIG: Planner size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 16, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented May 16, 2026

[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 terry1purcell 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

@hawkingrei hawkingrei added the AI-Correction Bugfix by AI label May 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

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: 2f0301cd-94e1-4264-9830-d73e00b5a99f

📥 Commits

Reviewing files that changed from the base of the PR and between dd8253c and f21fa72.

📒 Files selected for processing (4)
  • pkg/planner/core/casetest/hint/testdata/integration_suite_in.json
  • pkg/planner/core/casetest/hint/testdata/integration_suite_out.json
  • pkg/planner/core/casetest/hint/testdata/integration_suite_xut.json
  • pkg/planner/core/logical_plan_builder.go

📝 Walkthrough

Walkthrough

This PR adds warning logic to handle view-style QB_NAME hints that target CTE references but cannot be consumed. A new helper function detects mismatched query block names and records appropriate warnings during CTE resolution. Integration test cases validate the warning messages across three test data files.

Changes

CTE QB_NAME Hint Warning

Layer / File(s) Summary
CTE hint warning implementation
pkg/planner/core/logical_plan_builder.go
tryBuildCTE now calls warnViewStyleHintsForCTE when resolving CTE references. New helper function inspects view-style QB_NAME hint mappings, derives the effective CTE reference name, and records warnings when hints are ignored due to unknown query block names.
CTE QB_NAME hint test cases
pkg/planner/core/casetest/hint/testdata/integration_suite_in.json, integration_suite_out.json, integration_suite_xut.json
Two new TestAllViewHintType test cases added covering qb_name(...) + ignore_index(...) hints applied to CTE-derived table references with different qb identifiers, asserting IndexLookUp plan shapes and expected warning messages for unknown/unused query block names.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • pingcap/tidb#66677: Modifies QB_NAME hint state tracking and unused qb_name warning generation in hintProcessor mappings, closely related to the new CTE-specific warning helper in this PR.

Suggested labels

sig/planner, size/L, ok-to-test

Suggested reviewers

  • qw4990
  • winoros

Poem

🐰 A hint by any other name would warn the same,
CTE aliases now caught within their frame,
QB_NAME seeks its blocks with honest care,
Unknown queries met with warnings fair! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'planner: warn view-style hints on CTE references' clearly summarizes the main change to warn when view-style hints target CTEs.
Description check ✅ Passed The PR description includes the required issue number, problem summary, explanation of changes, test selection, and release note, following the template structure.
Linked Issues check ✅ Passed The PR implements the objective from issue #64570 to make warning behavior consistent when qb_name targets CTEs by reporting forwarded table hints as ignored.
Out of Scope Changes check ✅ Passed All changes (test additions and the warnViewStyleHintsForCTE helper in logical_plan_builder.go) are directly scoped to addressing the issue of inconsistent hint warnings on CTEs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

❌ Patch coverage is 43.47826% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.4999%. Comparing base (dd8253c) to head (f21fa72).

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #68426        +/-   ##
================================================
- Coverage   77.2764%   76.4999%   -0.7766%     
================================================
  Files          2010       1992        -18     
  Lines        555481     557555      +2074     
================================================
- Hits         429256     426529      -2727     
- Misses       125305     130982      +5677     
+ Partials        920         44       -876     
Flag Coverage Δ
integration 41.5394% <43.4782%> (+1.7453%) ⬆️

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

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

@hawkingrei
Copy link
Copy Markdown
Member Author

/retest

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

Labels

AI-Correction Bugfix by AI do-not-merge/needs-triage-completed release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/planner SIG: Planner size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Odd warnings when QB_NAME tries to address a CTE

1 participant