planner: avoid NULLIF type leak from comparison rewrite

[hawkingrei]

What problem does this PR solve?

Issue Number: close #66322

Problem Summary:

NULLIF(a, b) is rewritten as IF(a = b, NULL, a). The comparison branch may coerce or wrap the first argument for equality evaluation. Reusing the same expression object as the returned value branch can leak comparison-side type metadata into the NULLIF result, which may further affect UNION result metadata and values for nested NULLIF expressions.

What changed and how does it work?

Clone the NULLIF comparison arguments before building the equality function, so comparison-side coercion cannot mutate the returned value expression.

Build the rewritten IF return type from the rewritten first argument type with NotNull cleared. This keeps the IF false branch aligned with NULLIF semantics and avoids adding comparison-side casts to the value returned when the comparison is false.

Add integration coverage for nested NULLIF(enum, double) and a UNION ALL shape matching the issue.

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.

Manual test details:

./tools/check/failpoint-go-test.sh pkg/expression/integration_test -run TestCompareBuiltin -count=1
PASS
ok  	github.com/pingcap/tidb/pkg/expression/integration_test	3.104s

cd tests/integrationtest && ./run-tests.sh -r executor/aggregate
./t/executor/aggregate.test: ok! 891 test cases passed
Great, All tests passed
integrationtest passed!

cd tests/integrationtest && ./run-tests.sh -t executor/aggregate
./t/executor/aggregate.test: ok! 891 test cases passed
Great, All tests passed
integrationtest passed!

git -c core.fsmonitor=false diff --check
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.

Fix a type metadata leak in NULLIF expression rewriting that could produce incorrect result metadata for nested NULLIF and UNION ALL queries.

Summary by CodeRabbit

• Bug Fixes

  • NULLIF now preserves the first argument’s type/precision, preventing incorrect type/precision results and removing a spurious truncation warning.

• Tests

  • Expanded integration tests to cover NULLIF across nullable types, nested NULLIF, UNION/CTE, aggregate/EXPLAIN cases, and added assertions for numeric formatting/aggregate outputs.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

NULLIF rewrite now clones operands and makes the generated IF return type follow the first operand (nullable). Integration tests and expected results updated to cover nested NULLIF, mediumtext/enum/float/double, UNION ALL, CTE+VARIANCE, and scalar/aggregate NULLIF cases.

Changes

Cohort / File(s)	Summary
Expression rewrite pkg/planner/core/expression_rewriter.go	NULLIF rewrite clones input expressions, builds equality from clones, derives IF return FieldType from a nullable copy of the first argument, uses TypeNull for the NULL branch, and adjusts charset/collation on the value branch.
Integration tests pkg/expression/integration_test/integration_test.go	Extended TestCompareBuiltin with NULLIF scenarios: nullable mediumtext, enum, float, double, nested NULLIF, UNION ALL, CTE with VARIANCE, and an EXPLAIN assertion for nullif(u, 1) on unsigned int.
Expected results / Test data tests/integrationtest/r/executor/aggregate.result, tests/integrationtest/t/executor/aggregate.test	Added expected outputs for nullif(77.15, PI()) and avg(nullif(77.15, PI())) (77.150000), adjusted DISTINCT aggregate expectation, and removed a truncation warning expectation.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• planner: preserve IFNULL semantics when return type changes | tidb-test=pr/2715 #67362 — similar planner expression_rewriter adjustments for null-producing builtins (IFNULL-related rewrite).

Suggested labels

ok-to-test, approved, lgtm

Suggested reviewers

• AilinKid
• qw4990

Poem

🐇 I cloned the args with tiny hops,
So NULLIF keeps the type that stops,
Enums and floats in orderly rows,
CTEs and unions strike a pose,
I munch a carrot — tests pass, hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'planner: avoid NULLIF type leak from comparison rewrite' clearly describes the main change - fixing a type metadata leak in NULLIF expression rewriting by avoiding unwanted type propagation from the comparison side.
Description check	✅ Passed	The description follows the repository template with all required sections: problem statement with issue link, explanation of what changed and how, completed test checklist, manual test commands, and a clear release note.
Linked Issues check	✅ Passed	All coding requirements from issue #66322 are met: NULLIF arguments are cloned before building the equality function, IF return type is derived from the rewritten first argument, and test coverage includes nested NULLIF and UNION ALL scenarios matching the reported bug.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the NULLIF type leak issue: the planner logic changes, integration test additions, and test result updates are all within scope of the reported problem.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.0548%. Comparing base (36bfd11) to head (287d667).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #68127        +/-   ##
================================================
- Coverage   77.7559%   77.0548%   -0.7011%     
================================================
  Files          1990       1972        -18     
  Lines        551769     555027      +3258     
================================================
- Hits         429033     427675      -1358     
- Misses       121816     127185      +5369     
+ Partials        920        167       -753     

Flag	Coverage Δ
integration	41.3559% <91.6666%> (+1.5540%)	⬆️

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

Components	Coverage Δ
dumpling	60.4888% <ø> (ø)
parser	∅ <ø> (∅)
br	50.0597% <ø> (-13.0338%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hawkingrei]

Now, it is the same as MySQL.

[hawkingrei]

mysql.GetDefaultFieldLengthAndDecimal(mysql.TypeNull) will get flen=0, decimal=0.

[hawkingrei]

So it is necessary to set flen and decimal.

[hawkingrei]

/retest

[hawkingrei]

/test tidb_parser_test

[ti-chi-bot]

@hawkingrei: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build

/test check-dev

/test check-dev2

/test mysql-test

/test pull-br-integration-test

/test pull-build-next-gen

/test pull-integration-e2e-test

/test pull-integration-realcluster-test-next-gen

/test pull-lightning-integration-test

/test pull-mysql-client-test

/test pull-mysql-client-test-next-gen

/test pull-unit-test-ddlv1

/test pull-unit-test-next-gen

/test unit-test

The following commands are available to trigger optional jobs:

/test pingcap/tidb/canary_ghpr_unit_test

/test pull-br-integration-test-next-gen

/test pull-check-deps

/test pull-common-test

/test pull-e2e-test

/test pull-error-log-review

/test pull-integration-common-test

/test pull-integration-copr-test

/test pull-integration-ddl-test

/test pull-integration-ddl-test-next-gen

/test pull-integration-e2e-test-next-gen

/test pull-integration-jdbc-test

/test pull-integration-mysql-test

/test pull-integration-nodejs-test

/test pull-integration-python-orm-test

/test pull-mysql-test-next-gen

/test pull-sqllogic-test

/test pull-tiflash-integration-test

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build

pingcap/tidb/ghpr_check

pingcap/tidb/ghpr_check2

pingcap/tidb/ghpr_mysql_test

pingcap/tidb/ghpr_unit_test

pingcap/tidb/pull_build_next_gen

pingcap/tidb/pull_integration_e2e_test

pingcap/tidb/pull_integration_realcluster_test_next_gen

pingcap/tidb/pull_mysql_client_test

pingcap/tidb/pull_mysql_client_test_next_gen

pingcap/tidb/pull_unit_test_next_gen

pull-check-deps

pull-error-log-review

Details

In response to this:

/test tidb_parser_test

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.

[hawkingrei]

/test check-dev2

[tiprow]

@hawkingrei: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow

/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test check-dev2

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: guo-shaoge, qw4990

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [guo-shaoge,qw4990]
• pkg/planner/OWNERS [guo-shaoge,qw4990]

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]

[LGTM Timeline notifier]

Timeline:

• 2026-04-30 06:30:36.114816113 +0000 UTC m=+2838641.320176170: ☑️ agreed by qw4990.
• 2026-05-07 02:44:12.429949894 +0000 UTC m=+322125.303299856: ☑️ agreed by guo-shaoge.