Skip to content

Commit e9db144

Browse files
committed
refactor(join): consolidate projection pushdown
The ColumnIndex-aware path was added beside the public positional helper to preserve API compatibility, leaving duplicate implementations that could diverge as join output schemas evolve. Keep the public API as a compatibility wrapper over the schema-aware path, and use existing output-origin metadata for symmetric hash joins. This preserves semver compatibility while keeping ownership decisions in one implementation. Refs #23010 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
1 parent a62b5c6 commit e9db144

2 files changed

Lines changed: 56 additions & 115 deletions

File tree

datafusion/physical-plan/src/joins/symmetric_hash_join.rs

Lines changed: 29 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ use crate::joins::utils::{
4747
matchable_join_keys, symmetric_join_output_partitioning, update_hash,
4848
};
4949
use crate::projection::{
50-
ProjectionExec, join_allows_pushdown, join_table_borders, new_join_children,
51-
physical_to_column_exprs, update_join_filter, update_join_on,
50+
JoinData, ProjectionExec, try_pushdown_through_join_with_column_indices,
5251
};
5352
use crate::stream::EmptyRecordBatchStream;
5453
use crate::{
@@ -589,69 +588,36 @@ impl ExecutionPlan for SymmetricHashJoinExec {
589588
&self,
590589
projection: &ProjectionExec,
591590
) -> Result<Option<Arc<dyn ExecutionPlan>>> {
592-
// Convert projected PhysicalExpr's to columns. If not possible, we cannot proceed.
593-
let Some(projection_as_columns) = physical_to_column_exprs(projection.expr())
594-
else {
595-
return Ok(None);
596-
};
597-
598-
let (far_right_left_col_ind, far_left_right_col_ind) = join_table_borders(
599-
self.left().schema().fields().len(),
600-
&projection_as_columns,
601-
);
602-
603-
if !join_allows_pushdown(
604-
&projection_as_columns,
605-
&self.schema(),
606-
far_right_left_col_ind,
607-
far_left_right_col_ind,
608-
) {
609-
return Ok(None);
610-
}
611-
612-
let Some(new_on) = update_join_on(
613-
&projection_as_columns[0..=far_right_left_col_ind as _],
614-
&projection_as_columns[far_left_right_col_ind as _..],
615-
self.on(),
616-
self.left().schema().fields().len(),
617-
) else {
618-
return Ok(None);
619-
};
620-
621-
let new_filter = if let Some(filter) = self.filter() {
622-
match update_join_filter(
623-
&projection_as_columns[0..=far_right_left_col_ind as _],
624-
&projection_as_columns[far_left_right_col_ind as _..],
625-
filter,
626-
self.left().schema().fields().len(),
627-
) {
628-
Some(updated_filter) => Some(updated_filter),
629-
None => return Ok(None),
630-
}
631-
} else {
632-
None
633-
};
634-
635-
let (new_left, new_right) = new_join_children(
636-
&projection_as_columns,
637-
far_right_left_col_ind,
638-
far_left_right_col_ind,
591+
let schema = self.schema();
592+
if let Some(JoinData {
593+
projected_left_child,
594+
projected_right_child,
595+
join_filter,
596+
join_on,
597+
}) = try_pushdown_through_join_with_column_indices(
598+
projection,
639599
self.left(),
640600
self.right(),
641-
)?;
642-
643-
SymmetricHashJoinExec::try_new(
644-
Arc::new(new_left),
645-
Arc::new(new_right),
646-
new_on,
647-
new_filter,
648-
self.join_type(),
649-
self.null_equality(),
650-
self.right().output_ordering().cloned(),
651-
self.left().output_ordering().cloned(),
652-
self.partition_mode(),
653-
)
654-
.map(|e| Some(Arc::new(e) as _))
601+
self.on(),
602+
&schema,
603+
self.filter(),
604+
self.column_indices.as_slice(),
605+
)? {
606+
SymmetricHashJoinExec::try_new(
607+
Arc::new(projected_left_child),
608+
Arc::new(projected_right_child),
609+
join_on,
610+
join_filter,
611+
self.join_type(),
612+
self.null_equality(),
613+
self.right().output_ordering().cloned(),
614+
self.left().output_ordering().cloned(),
615+
self.partition_mode(),
616+
)
617+
.map(|e| Some(Arc::new(e) as _))
618+
} else {
619+
Ok(None)
620+
}
655621
}
656622
}
657623

datafusion/physical-plan/src/projection.rs

Lines changed: 27 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -650,60 +650,35 @@ pub fn try_pushdown_through_join(
650650
schema: &SchemaRef,
651651
filter: Option<&JoinFilter>,
652652
) -> Result<Option<JoinData>> {
653-
// Convert projected expressions to columns. We can not proceed if this is not possible.
654-
let Some(projection_as_columns) = physical_to_column_exprs(projection.expr()) else {
655-
return Ok(None);
656-
};
657-
658-
let (far_right_left_col_ind, far_left_right_col_ind) =
659-
join_table_borders(join_left.schema().fields().len(), &projection_as_columns);
660-
661-
if !join_allows_pushdown(
662-
&projection_as_columns,
663-
schema,
664-
far_right_left_col_ind,
665-
far_left_right_col_ind,
666-
) {
667-
return Ok(None);
668-
}
669-
670-
let new_filter = if let Some(filter) = filter {
671-
match update_join_filter(
672-
&projection_as_columns[0..=far_right_left_col_ind as _],
673-
&projection_as_columns[far_left_right_col_ind as _..],
674-
filter,
675-
join_left.schema().fields().len(),
676-
) {
677-
Some(updated_filter) => Some(updated_filter),
678-
None => return Ok(None),
679-
}
680-
} else {
681-
None
682-
};
683-
684-
let Some(new_on) = update_join_on(
685-
&projection_as_columns[0..=far_right_left_col_ind as _],
686-
&projection_as_columns[far_left_right_col_ind as _..],
687-
join_on,
688-
join_left.schema().fields().len(),
689-
) else {
690-
return Ok(None);
691-
};
653+
let left_field_count = join_left.schema().fields().len();
654+
let column_indices = schema
655+
.fields()
656+
.iter()
657+
.enumerate()
658+
.map(|(index, _)| {
659+
if index < left_field_count {
660+
ColumnIndex {
661+
index,
662+
side: JoinSide::Left,
663+
}
664+
} else {
665+
ColumnIndex {
666+
index: index - left_field_count,
667+
side: JoinSide::Right,
668+
}
669+
}
670+
})
671+
.collect::<Vec<_>>();
692672

693-
let (new_left, new_right) = new_join_children(
694-
&projection_as_columns,
695-
far_right_left_col_ind,
696-
far_left_right_col_ind,
673+
try_pushdown_through_join_with_column_indices(
674+
projection,
697675
join_left,
698676
join_right,
699-
)?;
700-
701-
Ok(Some(JoinData {
702-
projected_left_child: new_left,
703-
projected_right_child: new_right,
704-
join_filter: new_filter,
705-
join_on: new_on,
706-
}))
677+
join_on,
678+
schema,
679+
filter,
680+
&column_indices,
681+
)
707682
}
708683

709684
pub(crate) fn try_pushdown_through_join_with_column_indices(
@@ -959,7 +934,7 @@ pub fn new_join_children(
959934
/// join's `ColumnIndex`). Unlike [`new_join_children`], this does not infer
960935
/// child ownership from output position, so it is safe for join schemas whose
961936
/// output is not a plain `left ++ right` (used by the schema-aware
962-
/// `try_pushdown_through_join`).
937+
/// `try_pushdown_through_join_with_column_indices`).
963938
fn new_join_children_from_groups(
964939
left_proj: &[(Column, String)],
965940
right_proj: &[(Column, String)],

0 commit comments

Comments
 (0)