planner: prove null-reject for deferred expressions (#67789)

[ti-chi-bot]

This is an automated cherry-pick of #67789

What problem does this PR solve?

Issue Number: close #67788, close #65583

Problem Summary:

Null-reject proof skipped Constant expressions with DeferredExpr to avoid treating plan-cache-sensitive runtime values as static constants. That was conservative, but it also missed safe symbolic proof opportunities and left a now-obsolete plan-cache compatibility parameter on planner/util.IsNullRejected.

What changed and how does it work?

This PR lets null-reject proof inspect DeferredExpr symbolically without evaluating or nullify-folding its runtime value.

• Add an internal allowNullifiedFold mode to proveNullRejected. Normal proof still uses the nullify-then-fold bridge. When descending into Constant.DeferredExpr, the proof keeps using structural null-reject rules but disables nullified folding.
• Keep ParamMarker and DeferredExpr out of static constant folding candidates.
• Remove the obsolete skipPlanCacheCheck argument from IsNullRejected and update its callers.
• Add regression cases for symbolic deferred proof, placeholder NULL classification, and ensuring deferred expressions skip nullified folding.

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.

Unit tests:

go test -run TestIsNullRejectedProofModes -tags=intest,deadlock
./tools/check/failpoint-go-test.sh pkg/planner/core/operator/logicalop -run '^$'
make lint

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

• Refactor

  • Simplified and unified null-rejection logic in the query planner, changing how null-rejection is determined and propagated during join, projection, and functional-dependency extraction.

• Tests

  • Updated tests to match the revised null-rejection behavior and ensure verification remains accurate.

[ti-chi-bot]

This cherry pick PR is for a release branch and has not yet been approved by triage owners.
Adding the do-not-merge/cherry-pick-not-approved label.

To merge this cherry pick:

1. It must be LGTMed and approved by the reviewers firstly.
2. For pull requests to TiDB-x branches, it must have no failed tests.
3. AFTER it has lgtm and approved labels, please wait for the cherry-pick merging approval from triage owners.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ti-chi-bot]

@winoros This PR has conflicts, I have hold it.
Please resolve them or ask others to resolve them, then comment /unhold to remove the hold label.

[ti-chi-bot]

@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[coderabbitai]

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: 81ee6607-8227-434b-930f-4e73e464dc12

📥 Commits

Reviewing files that changed from the base of the PR and between 5e46cee and d554da6.

📒 Files selected for processing (6)

• pkg/planner/core/operator/logicalop/logical_aggregation.go
• pkg/planner/core/operator/logicalop/logical_join.go
• pkg/planner/core/operator/logicalop/logical_projection.go
• pkg/planner/util/funcdep_misc.go
• pkg/planner/util/null_misc.go
• pkg/planner/util/null_misc_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/planner/core/operator/logicalop/logical_aggregation.go
• pkg/planner/util/null_misc.go
• pkg/planner/core/operator/logicalop/logical_join.go

---

📝 Walkthrough

Walkthrough

IsNullRejected's API was simplified by removing the skipPlanCacheCheck parameter; implementation comments were clarified. All planner call sites (logical join, aggregation, projection, funcdep extraction) and a test were updated to the new 3-argument signature.

Changes

IsNullRejected API signature simplification

Layer / File(s)	Summary
IsNullRejected contract and implementation pkg/planner/util/null_misc.go	Function signature changed by removing skipPlanCacheCheck; comments clarified about ParamMarker/DeferredExpr and allConstants behavior.
Logical join null-rejection derivation and simplification pkg/planner/core/operator/logicalop/logical_join.go	Updated IsNullRejected call sites used by simplifyOuterJoin, ConvertOuterToInnerJoin, ExtractOnCondition, and deriveNotNullExpr to the new signature.
Functional dependency extraction and aggregation/projection pkg/planner/core/operator/logicalop/logical_aggregation.go, pkg/planner/core/operator/logicalop/logical_projection.go, pkg/planner/util/funcdep_misc.go	Updated ExtractFD and ExtractNotNullFromConds to call IsNullRejected with the revised arity when computing not-null columns for FD construction.
Test updates pkg/planner/util/null_misc_test.go	Adjusted test assertion(s) to invoke IsNullRejected with three arguments instead of four.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• pingcap/tidb#67789: Related work updating null-rejection proof behavior and removing the extra IsNullRejected boolean parameter at call sites.
• pingcap/tidb#68744: Related changes to planner null-reject proof and outer-join logic that touch similar call sites.

Suggested labels

size/L, cherry-pick-approved, ok-to-test, approved, lgtm

Suggested reviewers

• winoros
• Reminiscent
• terry1purcell

Poem

🐰 I pruned a boolean from the proof-tree vine,
Deferred checks stay symbolic, neat and fine.
Callers shortened, signatures now lean,
Null-rejects clarified—proof stays clean.
Hoppity hops, the planner's tidy and green.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'planner: prove null-reject for deferred expressions (#67789)' clearly and specifically describes the main change: enhancing null-rejection proof for deferred expressions in the planner.
Description check	✅ Passed	The PR description follows the required template with all essential sections completed: issue references (close #67788, close #65583), problem summary, explanation of changes, test checklist with unit test checked, and release note marked as None.
Linked Issues check	✅ Passed	The code changes directly address both linked issues: #67788 (symbolic null-reject proof for deferred expressions) and #65583 (fixing plan cache skipping for parameter-based null-reject inference). The refactoring of IsNullRejected signature and updates across call sites implement the symbolic proof mechanism.
Out of Scope Changes check	✅ Passed	All changes are in-scope: modifying IsNullRejected signature, updating call sites in logical operations, adjusting functional dependency extraction, and test updates all directly support the null-reject proof enhancement objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

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

✨ 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)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

pkg/planner/util/null_misc_test.go (2)

194-228: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Unresolved merge conflict blocks compilation.

The file contains unresolved merge conflict markers that will cause a syntax error. The conflict is between the release-8.5 branch (which includes week/yearweek nullable-mode test expressions from a separate PR) and the cherry-picked commit.

To resolve: decide whether to keep the week/yearweek test expressions (from HEAD) or remove them (from the cherry-picked commit), then remove all conflict markers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/planner/util/null_misc_test.go` around lines 194 - 228, The file has
unresolved merge conflict markers around test variables weekWithNullableMode,
yearWeekWithNullableMode, weekWithNullableDateAndOuterMode, and
yearWeekWithNullableDateAndOuterMode; remove the conflict markers and either
keep the block that constructs those newNullRejectFunc tests (using
newNullRejectStringConst("2024-01-08"), innerDate, outerC as in HEAD) or drop
that block to match the cherry-picked commit, ensuring no <<<<<<<, =======,
>>>>>>> remain and the file compiles; verify
newNullRejectFunc/newNullRejectStringConst, innerDate and outerC references are
consistent after resolution.

---

391-441: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Second unresolved merge conflict in test cases array.

This conflict corresponds to the expression-setup conflict above. Ensure both conflicts are resolved consistently—if the week/yearweek expressions are retained, these test cases must also be kept; if removed, these entries must be deleted along with the conflict markers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/planner/util/null_misc_test.go` around lines 391 - 441, Resolve the merge
conflict by removing Git conflict markers and making the test cases array
consistent with the resolved expression definitions: either keep the four test
entries (names "week_nullable_mode_uses_default_mode_zero",
"yearweek_nullable_mode_uses_default_mode_zero",
"week_nullable_date_rejects_null_even_with_outer_mode",
"yearweek_nullable_date_rejects_null_even_with_outer_mode") and ensure the
referenced symbols weekWithNullableMode, yearWeekWithNullableMode,
weekWithNullableDateAndOuterMode, yearWeekWithNullableDateAndOuterMode and the
helper newNullRejectFunc/exprCtx are present, or delete those four entries
entirely if the corresponding week/yearweek expressions were removed; in all
cases remove the <<<<<<</=======/>>>>>>> markers and keep the file
compile/test-clean.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/planner/util/funcdep_misc.go`:
- Around line 45-49: Remove the unresolved merge conflict markers (<<<<<<<,
=======, >>>>>>>) and keep the updated call to IsNullRejected with three
arguments: replace the conflicted block so it calls IsNullRejected(p.SCtx(),
p.Schema(), condition, true); ensure no leftover conflict markers remain and
that the surrounding code compiles with the IsNullRejected function signature.

---

Outside diff comments:
In `@pkg/planner/util/null_misc_test.go`:
- Around line 194-228: The file has unresolved merge conflict markers around
test variables weekWithNullableMode, yearWeekWithNullableMode,
weekWithNullableDateAndOuterMode, and yearWeekWithNullableDateAndOuterMode;
remove the conflict markers and either keep the block that constructs those
newNullRejectFunc tests (using newNullRejectStringConst("2024-01-08"),
innerDate, outerC as in HEAD) or drop that block to match the cherry-picked
commit, ensuring no <<<<<<<, =======, >>>>>>> remain and the file compiles;
verify newNullRejectFunc/newNullRejectStringConst, innerDate and outerC
references are consistent after resolution.
- Around line 391-441: Resolve the merge conflict by removing Git conflict
markers and making the test cases array consistent with the resolved expression
definitions: either keep the four test entries (names
"week_nullable_mode_uses_default_mode_zero",
"yearweek_nullable_mode_uses_default_mode_zero",
"week_nullable_date_rejects_null_even_with_outer_mode",
"yearweek_nullable_date_rejects_null_even_with_outer_mode") and ensure the
referenced symbols weekWithNullableMode, yearWeekWithNullableMode,
weekWithNullableDateAndOuterMode, yearWeekWithNullableDateAndOuterMode and the
helper newNullRejectFunc/exprCtx are present, or delete those four entries
entirely if the corresponding week/yearweek expressions were removed; in all
cases remove the <<<<<<</=======/>>>>>>> markers and keep the file
compile/test-clean.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cc472080-1043-418b-bfcf-90201f387ee3

📥 Commits

Reviewing files that changed from the base of the PR and between 4f11f32 and 5e46cee.

📒 Files selected for processing (6)

• pkg/planner/core/operator/logicalop/logical_aggregation.go
• pkg/planner/core/operator/logicalop/logical_join.go
• pkg/planner/core/operator/logicalop/logical_projection.go
• pkg/planner/util/funcdep_misc.go
• pkg/planner/util/null_misc.go
• pkg/planner/util/null_misc_test.go

[ti-chi-bot]

[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 ask for approval from winoros. 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:

• pkg/planner/OWNERS

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

[winoros]

/unhold

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release-8.5@4f11f32). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff                @@
##             release-8.5     #68850   +/-   ##
================================================
  Coverage               ?   55.1038%           
================================================
  Files                  ?       1824           
  Lines                  ?     656334           
  Branches               ?          0           
================================================
  Hits                   ?     361665           
  Misses                 ?     267761           
  Partials               ?      26908           

Flag	Coverage Δ
integration	38.1745% <88.8888%> (?)
unit	65.1311% <100.0000%> (?)

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

Components	Coverage Δ
dumpling	55.3108% <0.0000%> (?)
parser	∅ <0.0000%> (?)
br	54.6336% <0.0000%> (?)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[winoros]

/retest