Skip to content

[branch-56] fix: preserve constant equivalence properties through projection (backport #27123) - #27179

Open
alamb wants to merge 1 commit into
apache:branch-56from
alamb:alamb/backport-27123
Open

alamb wants to merge 1 commit into
apache:branch-56from
alamb:alamb/backport-27123

Conversation

…che#27123)

## Which issue does this PR close?

Closes apache#26624.

## Rationale for this change

With `prefer_hash_join = false`, a query can fail physical plan
validation when a filter makes a computed sort or join key constant.
Embedding a projection into the sort-merge join loses the constant
property after sorting has already relied on it.

## What changes are included in this PR?

Preserve each projected equivalence class's constant metadata. A
projected member has the same value as its source member on every row,
and projection keeps rows in the same partitions. Its known value and
uniform or partition-local status therefore still hold. Outer joins
clear constants on the NULL-padded side before projecting their output.
Partition merges clear `Heterogeneous` constants.

Also preserve scalar-subquery leaves during indirect projection. They
read a shared subquery result without depending on the input schema. The
arithmetic `ORDER BY` reproduction needs the metadata copy; the
scalar-subquery join reproduction needs both changes.

## What is the testing strategy for this PR?

SQL cases in `joins.slt` cover arithmetic, `upper()`, left joins, and
the scalar-subquery join. A NULL-padded left join checks that matched
values sort before the NULL rows. The metadata unit test covers known,
unknown, and partition-local constants, dropped arguments, and partition
merging.

The original SQL cases and metadata test fail before the fix and pass
afterward. Validation includes the extended workspace tests, full lint
suite, and strict all-features Clippy.

## Are there any user-facing changes?

Affected queries plan and execute successfully. No public API changes.

Needs a backport to `branch-56` for the 56.0.0 regression tracker
(apache#24461).

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit ad8578a)
@alamb alamb mentioned this pull request Nov 6, 2026
28 of 54 tasks
@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Nov 6, 2026

@mbutrovich mbutrovich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a clean cherry-pick, approved pending CI. Thanks @alamb!

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.26%. Comparing base (6339fcb) to head (bb7a6e3).
⚠️ Report is 2 commits behind head on branch-56.

Files with missing lines Patch % Lines
datafusion/physical-expr/src/equivalence/class.rs 91.42% 0 Missing and 3 partials ⚠️
Flag Coverage Δ
all-tests 83.26% <91.42%> (+<0.01%) ⬆️
sqllogictests 60.72% <100.00%> (-0.02%) ⬇️

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

@@              Coverage Diff              @@
##           branch-56   #27179      +/-   ##
=============================================
- Coverage      83.26%   83.26%   -0.01%     
=============================================
  Files           1169     1169              
  Lines         474269   474302      +33     
  Branches      474269   474302      +33     
=============================================
+ Hits          394908   394928      +20     
- Misses         55455    55463       +8     
- Partials       23906    23911       +5     
Files with missing lines Coverage Δ
datafusion/physical-expr/src/equivalence/class.rs 88.71% <91.42%> (+0.07%) ⬆️

... and 7 files with indirect coverage changes

🚀 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.

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

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants