Skip to content

Commit 3a4f995

Browse files
committed
fix: align ASOF logical contracts
1 parent a71c19f commit 3a4f995

4 files changed

Lines changed: 42 additions & 78 deletions

File tree

datafusion/core/src/physical_planner.rs

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ use datafusion_expr::physical_planning_context::{
9494
use datafusion_expr::utils::{expr_to_columns, split_conjunction};
9595
use datafusion_expr::{
9696
Analyze, BinaryExpr, DescribeTable, DmlStatement, Explain, ExplainFormat, Extension,
97-
FetchType, Filter, JoinConstraint, JoinType, Operator, RecursiveQuery, SkipType,
98-
StringifiedPlan, WindowFrame, WindowFrameBound, WriteOp,
97+
FetchType, Filter, JoinType, Operator, RecursiveQuery, SkipType, StringifiedPlan,
98+
WindowFrame, WindowFrameBound, WriteOp,
9999
};
100100
use datafusion_physical_expr::aggregate::{
101101
AggregateFunctionExpr, LoweredAggregate, LoweredAggregateBuilder,
@@ -1903,22 +1903,8 @@ impl DefaultPhysicalPlanner {
19031903
planning_ctx,
19041904
)?,
19051905
);
1906-
let omitted_right = if join.join_constraint == JoinConstraint::Using {
1907-
join.on
1908-
.iter()
1909-
.map(|(_, right)| {
1910-
let column = right.get_as_join_column().ok_or_else(|| {
1911-
internal_datafusion_err!("ASOF USING key is not a column")
1912-
})?;
1913-
join.right.schema().index_of_column(column)
1914-
})
1915-
.collect::<Result<HashSet<_>>>()?
1916-
} else {
1917-
HashSet::new()
1918-
};
1919-
let right_output_indices = (0..join.right.schema().fields().len())
1920-
.filter(|index| !omitted_right.contains(index))
1921-
.collect();
1906+
let right_output_indices =
1907+
(0..join.right.schema().fields().len()).collect();
19221908
Arc::new(AsOfJoinExec::try_new(
19231909
physical_left,
19241910
physical_right,

datafusion/expr/src/logical_plan/builder.rs

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,38 +1840,10 @@ pub fn build_join_schema(
18401840

18411841
/// Creates the schema for a left-preserving ASOF join.
18421842
///
1843-
/// `ON` emits all left fields followed by nullable right fields. `USING` emits
1844-
/// each equality key once by omitting the corresponding right field.
1845-
pub fn build_asof_join_schema(
1846-
left: &DFSchema,
1847-
right: &DFSchema,
1848-
on: &[(Expr, Expr)],
1849-
join_constraint: JoinConstraint,
1850-
) -> Result<DFSchema> {
1851-
let omitted_right_indices = if join_constraint == JoinConstraint::Using {
1852-
on.iter()
1853-
.map(|(_, right_expr)| {
1854-
let column = right_expr.get_as_join_column().ok_or_else(|| {
1855-
plan_datafusion_err!("ASOF USING keys must be columns")
1856-
})?;
1857-
right.index_of_column(column)
1858-
})
1859-
.collect::<Result<HashSet<_>>>()?
1860-
} else {
1861-
HashSet::new()
1862-
};
1863-
1864-
let full_schema = build_join_schema(left, right, &JoinType::Left)?;
1865-
let left_len = left.fields().len();
1866-
let fields = full_schema
1867-
.iter()
1868-
.enumerate()
1869-
.filter(|(index, _)| {
1870-
*index < left_len || !omitted_right_indices.contains(&(*index - left_len))
1871-
})
1872-
.map(|(_, (qualifier, field))| (qualifier.cloned(), Arc::clone(field)))
1873-
.collect();
1874-
DFSchema::new_with_metadata(fields, full_schema.metadata().clone())?
1843+
/// Both `ON` and `USING` preserve all qualified input fields. SQL wildcard
1844+
/// expansion handles the unqualified `USING` key as a single column.
1845+
pub fn build_asof_join_schema(left: &DFSchema, right: &DFSchema) -> Result<DFSchema> {
1846+
build_join_schema(left, right, &JoinType::Left)?
18751847
.with_functional_dependencies(left.functional_dependencies().clone())
18761848
}
18771849

datafusion/expr/src/logical_plan/plan.rs

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,6 @@ pub enum LogicalPlan {
240240
/// Join two logical plans on one or more join columns.
241241
/// This is used to implement SQL `JOIN`
242242
Join(Join),
243-
/// Match each left row with at most one ordered row from the right input.
244-
/// This is used to implement SQL `ASOF JOIN`.
245-
AsOfJoin(AsOfJoin),
246243
/// Repartitions the input based on a partitioning scheme. This is
247244
/// used to add parallelism and is sometimes referred to as an
248245
/// "exchange" operator in other systems
@@ -299,6 +296,9 @@ pub enum LogicalPlan {
299296
Unnest(Unnest),
300297
/// A variadic query (e.g. "Recursive CTEs")
301298
RecursiveQuery(RecursiveQuery),
299+
/// Match each left row with at most one ordered row from the right input.
300+
/// This is used to implement SQL `ASOF JOIN`.
301+
AsOfJoin(AsOfJoin),
302302
}
303303

304304
impl Default for LogicalPlan {
@@ -4455,8 +4455,7 @@ impl AsOfJoin {
44554455
return plan_err!("ASOF USING keys must be columns");
44564456
}
44574457

4458-
let schema =
4459-
build_asof_join_schema(left.schema(), right.schema(), &on, join_constraint)?;
4458+
let schema = build_asof_join_schema(left.schema(), right.schema())?;
44604459
Ok(Self {
44614460
left,
44624461
right,
@@ -6867,6 +6866,32 @@ mod tests {
68676866
Ok(())
68686867
}
68696868

6869+
#[test]
6870+
fn test_asof_using_preserves_qualified_keys() -> Result<()> {
6871+
let schema = Schema::new(vec![
6872+
Field::new("id", DataType::Int32, false),
6873+
Field::new("ts", DataType::Int64, false),
6874+
]);
6875+
let left = Arc::new(table_scan(Some("t1"), &schema, None)?.build()?);
6876+
let right = Arc::new(table_scan(Some("t2"), &schema, None)?.build()?);
6877+
let join = AsOfJoin::try_new(
6878+
left,
6879+
right,
6880+
vec![(col("t1.id"), col("t2.id"))],
6881+
AsOfMatch::new(col("t1.ts"), Operator::GtEq, col("t2.ts")),
6882+
JoinConstraint::Using,
6883+
)?;
6884+
6885+
assert_eq!(join.schema.fields().len(), 4);
6886+
assert_eq!(
6887+
join.schema
6888+
.index_of_column(&Column::from_qualified_name("t2.id"))?,
6889+
2
6890+
);
6891+
assert!(join.schema.field(2).is_nullable());
6892+
Ok(())
6893+
}
6894+
68706895
#[test]
68716896
fn test_join_try_new_schema_validation() -> Result<()> {
68726897
let left_schema = Schema::new(vec![

datafusion/optimizer/src/optimize_projections/mod.rs

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ use crate::{OptimizerConfig, OptimizerRule};
2424
use std::sync::Arc;
2525

2626
use datafusion_common::{
27-
Column, DFSchema, HashMap, JoinConstraint, JoinType, Result,
28-
assert_eq_or_internal_err, get_required_group_by_exprs_indices,
29-
internal_datafusion_err, internal_err,
27+
Column, DFSchema, HashMap, JoinType, Result, assert_eq_or_internal_err,
28+
get_required_group_by_exprs_indices, internal_datafusion_err, internal_err,
3029
};
3130
use datafusion_expr::expr::Alias;
3231
use datafusion_expr::{
@@ -410,31 +409,13 @@ fn optimize_projections(
410409
}
411410
LogicalPlan::AsOfJoin(join) => {
412411
let left_len = join.left.schema().fields().len();
413-
let omitted_right = if join.join_constraint == JoinConstraint::Using {
414-
join.on
415-
.iter()
416-
.map(|(_, right)| {
417-
let column = right.get_as_join_column().ok_or_else(|| {
418-
internal_datafusion_err!("ASOF USING key is not a column")
419-
})?;
420-
join.right.schema().index_of_column(column)
421-
})
422-
.collect::<Result<std::collections::HashSet<_>>>()?
423-
} else {
424-
std::collections::HashSet::new()
425-
};
426-
let right_output_indices = (0..join.right.schema().fields().len())
427-
.filter(|index| !omitted_right.contains(index))
428-
.collect::<Vec<_>>();
429412
let mut left_required = Vec::new();
430413
let mut right_required = Vec::new();
431414
for index in indices.indices() {
432415
if *index < left_len {
433416
left_required.push(*index);
434-
} else if let Some(right_index) =
435-
right_output_indices.get(*index - left_len)
436-
{
437-
right_required.push(*right_index);
417+
} else {
418+
right_required.push(*index - left_len);
438419
}
439420
}
440421
let left_indices = RequiredIndices::new_from_indices(left_required)

0 commit comments

Comments
 (0)