You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/07/15 15:08:30 UTC
(datafusion) branch gh-readonly-queue/main/pr-22903-e2ef25f23423e81ca4a66aaea8339bd01151c3b6 created (now 69c3a78a41)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-22903-e2ef25f23423e81ca4a66aaea8339bd01151c3b6
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 69c3a78a41 fix: optimize_projections failure with struct-field join keys (#22903)
This branch includes the following new commits:
new 69c3a78a41 fix: optimize_projections failure with struct-field join keys (#22903)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: optimize_projections failure with struct-field join keys (#22903)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-22903-e2ef25f23423e81ca4a66aaea8339bd01151c3b6
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 69c3a78a4162694a77e7da7047b8f50d86e4b058
Author: Kumar Ujjawal <uj...@gmail.com>
AuthorDate: Wed Jul 15 20:38:08 2026 +0530
fix: optimize_projections failure with struct-field join keys (#22903)
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->
- Closes #22895.
## Rationale for this change
Join-key extraction can add helper projections and change a child plan's
output schema. Some parent nodes cache their schema, so after the child
rewrite they can still expose stale fields. Later projection pruning
then uses the stale schema and fails with missing-column errors.
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
- Refresh a parent schema only when a rewritten child reports an actual
schema change.
- Re-check whether the plan has subqueries before each rule, so plans
that are decorrelated by earlier rules can use the in-place path in the
same optimizer pass.
- Add regression coverage for the reported struct-field join-key
failures.
- Add a regression case for union output labels while union-to-filter
rewriting is enabled.
- Add a focused optimizer unit test for parent schema refresh after
child schema changes.
- Update one existing optimized-plan expectation. The new plan removes a
redundant dedup step; the surrounding semi joins already preserve
distinctness.
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
## Are these changes tested?
Yes
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
## Are there any user-facing changes?
No
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
---------
Co-authored-by: Andrew Lamb <an...@nerdnetworks.org>
---
datafusion/optimizer/src/optimizer.rs | 102 ++++++++++++++++++---
.../optimizer/tests/optimizer_integration.rs | 11 +--
.../test_files/projection_pushdown.slt | 36 ++++++++
datafusion/sqllogictest/test_files/union.slt | 8 ++
4 files changed, 138 insertions(+), 19 deletions(-)
diff --git a/datafusion/optimizer/src/optimizer.rs b/datafusion/optimizer/src/optimizer.rs
index a765d7f27a..db7ad84752 100644
--- a/datafusion/optimizer/src/optimizer.rs
+++ b/datafusion/optimizer/src/optimizer.rs
@@ -518,10 +518,23 @@ fn rewrite_plan_in_place(
}
}
- // Recurse into children using Arc::make_mut (zero-cost when refcount == 1)
- changed |= map_children_mut(plan, |child| {
- rewrite_plan_in_place(child, apply_order, rule, config)
+ let mut child_schema_changed = false;
+ let children_changed = map_children_mut(plan, |child| {
+ let old_schema = Arc::clone(child.schema());
+ let child_changed = rewrite_plan_in_place(child, apply_order, rule, config)?;
+ if child_changed && old_schema.as_ref() != child.schema().as_ref() {
+ child_schema_changed = true;
+ }
+ Ok(child_changed)
})?;
+ changed |= children_changed;
+
+ if child_schema_changed {
+ // Child rewrites can change their output schemas. Recompute the current
+ // node before later rules use positional requirements from that schema.
+ let owned = std::mem::take(plan);
+ *plan = owned.recompute_schema()?;
+ }
// f_up phase
if apply_order == ApplyOrder::BottomUp {
@@ -604,13 +617,11 @@ impl Optimizer {
while i < options.optimizer.max_passes {
log_plan(&format!("Optimizer input (pass {i})"), &new_plan);
- // Check once per pass whether the plan contains subquery
- // expressions. When there are no subqueries, we use the
- // cheaper `rewrite` traversal instead of
- // `rewrite_with_subqueries`, avoiding the per-node
- // map_subqueries call that walks all expression trees
- // via ownership-based transform_down.
- let has_subqueries = plan_has_subqueries(&new_plan);
+ // Track subquery presence across the pass. Refresh after changed
+ // rules so decorrelation can move later rules onto the in-place
+ // path; that path refreshes parent schemas after child schemas
+ // change.
+ let mut has_subqueries = plan_has_subqueries(&new_plan);
for rule in &self.rules {
// If skipping failed rules, copy plan before attempting to rewrite
@@ -690,6 +701,7 @@ impl Optimizer {
new_plan = data;
observer(&new_plan, rule.as_ref());
if transformed {
+ has_subqueries = plan_has_subqueries(&new_plan);
log_plan(rule.name(), &new_plan);
} else {
debug!(
@@ -773,13 +785,15 @@ mod tests {
use datafusion_common::tree_node::Transformed;
use datafusion_common::{
- DFSchema, DFSchemaRef, DataFusionError, Result, assert_contains, plan_err,
+ Column, DFSchema, DFSchemaRef, DataFusionError, Result, assert_contains, plan_err,
};
use datafusion_expr::logical_plan::EmptyRelation;
- use datafusion_expr::{LogicalPlan, LogicalPlanBuilder, Projection, col, lit};
+ use datafusion_expr::{
+ Expr, JoinType, LogicalPlan, LogicalPlanBuilder, Projection, col, lit,
+ };
use crate::optimizer::Optimizer;
- use crate::test::test_table_scan;
+ use crate::test::{test_table_scan, test_table_scan_with_name};
use crate::{OptimizerConfig, OptimizerContext, OptimizerRule};
use super::ApplyOrder;
@@ -863,6 +877,34 @@ mod tests {
Ok(())
}
+ #[test]
+ fn in_place_rewrite_recomputes_parent_schema_when_child_schema_changes() -> Result<()>
+ {
+ let left = LogicalPlanBuilder::from(test_table_scan_with_name("left")?)
+ .project(vec![col("left.a"), col("left.b"), col("left.c")])?
+ .build()?;
+ let right = LogicalPlanBuilder::from(test_table_scan_with_name("right")?)
+ .project(vec![col("right.a"), col("right.b"), col("right.c")])?
+ .build()?;
+ let mut plan = LogicalPlanBuilder::from(left)
+ .join_on(right, JoinType::Inner, [col("left.a").eq(col("right.a"))])?
+ .build()?;
+
+ assert_eq!(plan.schema().fields().len(), 6);
+
+ let changed = super::rewrite_plan_in_place(
+ &mut plan,
+ ApplyOrder::TopDown,
+ &KeepOnlyAProjectionRule {},
+ &OptimizerContext::new(),
+ )?;
+
+ assert!(changed);
+ assert_eq!(plan.schema().fields().len(), 2);
+ assert!(plan.schema().has_column_with_unqualified_name("a"));
+ Ok(())
+ }
+
#[test]
fn optimizer_detects_plan_equal_to_the_initial() -> Result<()> {
// Run a goofy optimizer, which rotates projection columns
@@ -980,6 +1022,40 @@ mod tests {
}
}
+ #[derive(Default, Debug)]
+ struct KeepOnlyAProjectionRule {}
+
+ impl OptimizerRule for KeepOnlyAProjectionRule {
+ fn name(&self) -> &str {
+ "keep_only_a_projection"
+ }
+
+ fn apply_order(&self) -> Option<ApplyOrder> {
+ Some(ApplyOrder::TopDown)
+ }
+
+ fn supports_rewrite(&self) -> bool {
+ true
+ }
+
+ fn rewrite(
+ &self,
+ plan: LogicalPlan,
+ _config: &dyn OptimizerConfig,
+ ) -> Result<Transformed<LogicalPlan>> {
+ let projection = match plan {
+ LogicalPlan::Projection(p) => p,
+ _ => return Ok(Transformed::no(plan)),
+ };
+
+ let expr = Expr::from(Column::from(projection.schema.qualified_field(0)));
+
+ Ok(Transformed::yes(LogicalPlan::Projection(
+ Projection::try_new(vec![expr], Arc::clone(&projection.input))?,
+ )))
+ }
+ }
+
/// A goofy rule doing rotation of columns in all projections.
///
/// Useful to test cycle detection.
diff --git a/datafusion/optimizer/tests/optimizer_integration.rs b/datafusion/optimizer/tests/optimizer_integration.rs
index d7440a4384..26b48c5e1f 100644
--- a/datafusion/optimizer/tests/optimizer_integration.rs
+++ b/datafusion/optimizer/tests/optimizer_integration.rs
@@ -275,13 +275,12 @@ fn intersect() -> Result<()> {
format!("{plan}"),
@r"
LeftSemi Join: left.col_int32 = test.col_int32, left.col_utf8 = test.col_utf8
- Aggregate: groupBy=[[left.col_int32, left.col_utf8]], aggr=[[]]
- LeftSemi Join: left.col_int32 = right.col_int32, left.col_utf8 = right.col_utf8
- Aggregate: groupBy=[[left.col_int32, left.col_utf8]], aggr=[[]]
- SubqueryAlias: left
- TableScan: test projection=[col_int32, col_utf8]
- SubqueryAlias: right
+ LeftSemi Join: left.col_int32 = right.col_int32, left.col_utf8 = right.col_utf8
+ Aggregate: groupBy=[[left.col_int32, left.col_utf8]], aggr=[[]]
+ SubqueryAlias: left
TableScan: test projection=[col_int32, col_utf8]
+ SubqueryAlias: right
+ TableScan: test projection=[col_int32, col_utf8]
TableScan: test projection=[col_int32, col_utf8]
"
);
diff --git a/datafusion/sqllogictest/test_files/projection_pushdown.slt b/datafusion/sqllogictest/test_files/projection_pushdown.slt
index 4fedf297cb..e6046cd496 100644
--- a/datafusion/sqllogictest/test_files/projection_pushdown.slt
+++ b/datafusion/sqllogictest/test_files/projection_pushdown.slt
@@ -2071,6 +2071,42 @@ SELECT s, id FROM simple_struct WHERE s['value'] > 100 AND id < 4;
{value: 200, label: beta} 2
{value: 150, label: gamma} 3
+#####################
+# Section 9: Join key extraction with pruned outputs
+#####################
+
+statement ok
+CREATE TABLE issue_22895_rt2 AS SELECT * FROM (VALUES
+ (named_struct('msg','user auth failed','sid','a'), 1, 'svc1'),
+ (named_struct('msg','login token','sid','b'), 2, 'svc2')
+) v(attributes, id, name);
+
+query IT
+SELECT a.id, b.name
+FROM issue_22895_rt2 a JOIN issue_22895_rt2 b
+ ON a.attributes['sid'] = b.attributes['sid']
+WHERE a.attributes['msg'] LIKE '%auth%'
+ORDER BY a.id, b.name;
+----
+1 svc1
+
+statement ok
+CREATE TABLE issue_22895_rt AS SELECT * FROM (VALUES
+ (named_struct('uid','u1','t','t1'), TIMESTAMP '2026-06-08T10:00:00', 'a'),
+ (named_struct('uid','u2','t','t2'), TIMESTAMP '2026-06-08T11:00:00', 'b')
+) v(attributes, start_timestamp, span_name);
+
+query P
+SELECT r.start_timestamp
+FROM issue_22895_rt r
+JOIN (SELECT attributes['uid'] AS uid FROM issue_22895_rt) f
+ ON f.uid = r.attributes['uid']
+WHERE r.attributes['t'] IN (SELECT attributes['t'] FROM issue_22895_rt)
+ORDER BY r.start_timestamp;
+----
+2026-06-08T10:00:00
+2026-06-08T11:00:00
+
# Config reset
# The SLT runner sets `target_partitions` to 4 instead of using the default, so
diff --git a/datafusion/sqllogictest/test_files/union.slt b/datafusion/sqllogictest/test_files/union.slt
index 41021299fb..cb5a06f729 100644
--- a/datafusion/sqllogictest/test_files/union.slt
+++ b/datafusion/sqllogictest/test_files/union.slt
@@ -341,6 +341,14 @@ physical_plan
05)--------FilterExec: id@0 = 1 OR id@0 = 2
06)----------DataSourceExec: partitions=1, partition_sizes=[1]
+# Regression: schema recomputation must preserve the unqualified UNION
+# output labels while unions_to_filter is enabled.
+query IT rowsort
+SELECT id, name FROM t1 WHERE id = 1 UNION SELECT id, name FROM t1 WHERE id = 2
+----
+1 Alex
+2 Bob
+
statement ok
set datafusion.optimizer.enable_unions_to_filter = false;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org