comphead commented on code in PR #19857:
URL: https://github.com/apache/datafusion/pull/19857#discussion_r4197557438
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -1622,6 +1644,36 @@ fn project_with_column_index(
.map(LogicalPlan::Projection)
}
+/// Coerces a multi-column `(a, b, ...) IN (SELECT x, y, ...)`.
+///
+/// Each tuple element and the subquery column at the same position are cast
+/// to their common comparison type, like the single-column form does for its
+/// one pair.
+fn coerce_multi_column_in_subquery(
Review Comment:
This PR now conflicts with `main` in this file. #25094 changed the scalar
branch of this arm to `comparison_coercion_with_session_timezone(..,
self.session_time_zone)`. This helper is a free function, so a resolution that
only patches the scalar branch still compiles and leaves tuple elements on the
timezone-unaware `comparison_coercion`. It needs the session time zone passed
in as well. A tuple with a naive and a tz-aware timestamp element, with the
session time zone set, would catch a miss.
##########
datafusion/expr/src/expr.rs:
##########
@@ -1401,6 +1402,54 @@ impl InSubquery {
}
}
+/// The tuple elements of a multi-column `(a, b, ...) IN (SELECT x, y, ...)`,
+/// given the compared expression `expr` and the `subquery` plan, or `None`
+/// for a single-column `IN`.
+///
+/// The tuple is planned as a `struct` call, which `simplify_expressions` folds
+/// into a struct literal when every element is a literal. It is a
+/// multi-column `IN` only when the subquery returns more than one column;
+/// against a single column the struct is one value compared with a
+/// struct-typed column.
+///
+/// Errors when the number of tuple elements does not match the number of
+/// subquery columns.
+pub fn in_subquery_tuple_values<'a>(
Review Comment:
This helper decides when an `InSubquery` is a tuple, but `Expr::nullable`
(`expr_schema.rs`, `InSubquery` arm) still assumes one value. It returns
`expr.nullable() | <first subquery column>.nullable()`. For a tuple, `expr` is
a `struct(..)` call, and `struct` always reports a non-nullable field
(`return_field_from_args`). So the elements never count, and only the first
subquery column does. `(a, b) IN (SELECT 1, 5)` is typed NOT NULL, although it
is UNKNOWN for `(NULL, 5)`.
`simplify_expressions` runs before decorrelation and folds `IS NULL` and `IS
UNKNOWN` to `false`, and `IS NOT NULL` to `true`, from that flag. By my reading
this gives a wrong result. I did not run it:
```sql
CREATE TABLE o(a INT, b INT) AS VALUES (NULL, 5), (1, 5);
SELECT a, ((a, b) IN (SELECT 1, 5)) IS NULL AS unk FROM o;
-- expected: (NULL, true) and (1, false)
-- by my reading: false for both rows
```
The scalar form is exact because `a.nullable | x.nullable` covers both
sides. For a tuple the arm could return true when any element or any subquery
column is nullable. An SLT case with `IS NULL` over a tuple `IN` and a NOT NULL
first subquery column would pin it.
--
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]
