viirya commented on code in PR #19857:
URL: https://github.com/apache/datafusion/pull/19857#discussion_r4202077201
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -487,23 +487,43 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
Arc::unwrap_or_clone(subquery.subquery),
)?
.data;
- let expr_type = expr.get_type(self.schema)?;
- let subquery_type = new_plan.schema().field(0).data_type();
- let common_type = comparison_coercion(&expr_type,
subquery_type).ok_or(
- plan_datafusion_err!(
- "expr type {expr_type} can't cast to {subquery_type} in
InSubquery"
- ),
- )?;
- let new_subquery = Subquery {
- subquery: Arc::new(new_plan),
- outer_ref_columns: subquery.outer_ref_columns,
- spans: subquery.spans,
- };
- Ok(Transformed::yes(Expr::InSubquery(InSubquery::new(
- Box::new(expr.cast_to(&common_type, self.schema)?),
- cast_subquery(new_subquery, &common_type)?,
- negated,
- ))))
+
+ // Check if this is a multi-column IN (struct expression)
+ let is_struct = matches!(*expr, Expr::ScalarFunction(ref func)
if func.func.name() == "struct");
+
+ if is_struct {
+ // For multi-column IN, we don't need type coercion at
this level
Review Comment:
This one is out of date: the multi-column form now coerces each tuple
element against its subquery column, the same way the single-column form
coerces its one pair (`coerce_multi_column_in_subquery`, which also applies the
session time zone).
##########
datafusion/expr/src/logical_plan/invariants.rs:
##########
@@ -224,14 +224,31 @@ pub fn check_subquery_expr(
check_correlations_in_subquery(inner_plan)
} else {
if let Expr::InSubquery(subquery) = expr {
- // InSubquery should only return one column
- if subquery.subquery.subquery.schema().fields().len() > 1 {
+ // InSubquery should only return one column UNLESS the left
expression is a struct
+ // (multi-column IN like: (a, b) NOT IN (SELECT x, y FROM ...))
+ let is_struct = matches!(*subquery.expr, Expr::ScalarFunction(ref
func) if func.func.name() == "struct");
Review Comment:
Agreed. The name check now lives in one place, `in_subquery_tuple_values`,
which every caller goes through (SQL planning, invariants, type coercion,
nullability, placeholder inference and decorrelation). It also recognizes the
struct literal that `simplify_expressions` folds an all-literal tuple into.
Making the tuple explicit in the type system, for example as a dedicated field
on `InSubquery`, would be cleaner but changes a public expression type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
