Repository navigation
fix: keep a projection's field names when decorrelating a predicate s… - #26657
Open
SingletonYe wants to merge 4 commits into
Open
SingletonYe wants to merge 4 commits into
SingletonYe wants to merge 4 commits into
Conversation
…ubquery
A set comparison in a projection without an alias failed with
Internal error: Assertion failed: compatible: Failed due to a difference in schemas:
original schema: ... Field { name: "Int64(2) > ALL", data_type: Boolean, nullable: true }
new schema: ... Field { name: "CASE WHEN EXISTS THEN Boolean(false) ...", ... }
This rule rebuilds a projection from its expressions, and it preserved the output field
names from `Expr::schema_name()` of the expression it was given. That is not the name the
rebuilt projection uses, for two reasons: `schema_name()` keeps a column's qualifier
(`t.k`) while the field is named after the bare column, and earlier rules rewrite
expressions with `map_expressions`, which keeps the schema but can leave an expression
whose display name no longer matches its field name. `RewriteSetComparison` does exactly
that -- it replaces `x > ALL (..)` with a `CASE WHEN EXISTS ..` -- so this rule rebuilt the
projection under the stale name and the optimizer's "the schema is invariant under
optimization" check failed. `datafusion.optimizer.skip_failed_rules` defaults to false, so
an ordinary query returned an internal error asking the user to file a bug report.
Take the field names from the projection's own schema, and compare them against the name
the rebuilt projection would give the expression -- the bare column name for a column, the
alias name for an alias, the display name otherwise -- so an alias is only added when the
rebuilt name would really differ. Comparing against the schema name alone would alias a
qualified column and lose its qualifier, which breaks the same check from the other side.
Tested with three cases in datafusion/sqllogictest/test_files/subquery.slt, and with
subquery.slt, null_aware_mark_join.slt, prepare.slt and cargo test -p datafusion-optimizer.
…projection-field-name Only subquery.slt conflicted: upstream and this branch both appended tests at the end of the file. Kept upstream's new cases and appended this branch's three cases after them.
|
Verified: fails on main exactly as you describe, returns Taking the name from the projection's schema is right — Also checked the subquery.slt + 948 optimizer tests green locally. |
…projection-field-name Only subquery.slt conflicted: upstream (apache#25258) and this branch both appended tests at the end of the file. Kept upstream's new cases and left this branch's three cases above them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch fix/set-comparison-projection-field-name (commit 0973cfa, 2 files) on SingletonYe/datafusion. Open at: https://github.com/apache/datafusion/compare/main...SingletonYe:fix/set-comparison-projection-field-name?expand=1
Post the issue first (text in set-comparison-projection-internal-error.md) and put its number in the first line below, or leave the line as it is and let the body carry the report. Paste from the horizontal rule down.
Which issue does this PR close?
Closes # — if no issue was filed, this body contains the report.
Rationale for this change
A set comparison in a projection that has no alias fails with an internal error:
Not on the release line: 55.2.0 does not contain #24972, so the same query still reports Physical plan does not support logical expression Exists(…) there.
What changes are included in this PR?
datafusion/optimizer/src/decorrelate_predicate_subquery.rs: take the field names from the projection's own schema, and compare them against the name the rebuilt projection would use (the bare column name for a column, the alias name for an alias, the display name otherwise), so an alias is added only when it would really differ. Comparing against the schema name alone would alias a qualified column and lose its qualifier — the same check failing from the other side.
datafusion/sqllogictest/test_files/subquery.slt: three cases — unaliased, unaliased with a second projected column, and aliased.
Are these changes tested?
Yes. Cherry-picked onto main@2717e25 and rebuilt: all six query shapes return the right answer, and subquery.slt (by then also containing #25391's new cases), null_aware_mark_join.slt and prepare.slt pass with 0 errors. On the base 5f21582 the same checks passed, plus cargo test -p datafusion-optimizer (941 + 29 + 5 tests) and cargo fmt --all --check; the optimizer unit tests were not re-run on 2717e25.
Are there any user-facing changes?
No.