neilconway commented on code in PR #19857:
URL: https://github.com/apache/datafusion/pull/19857#discussion_r3105141854
##########
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:
Can you elaborate on this? It isn't clear to me why the type coercion
requirements for multi-column IN would be different than for a single-column IN.
##########
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:
Matching based on the function name here seems a bit fragile, which is
unfortunate because we do it repeatedly in this PR. Is there a way to make the
distinction more principled / manifest in the type system?
--
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]
