kosiew commented on code in PR #22545:
URL: https://github.com/apache/datafusion/pull/22545#discussion_r3331520112
##########
datafusion/expr/src/expr.rs:
##########
@@ -3945,6 +3972,112 @@ mod test {
}
}
+ #[test]
Review Comment:
This regression is visible through SQL (`PREPARE ... WHERE $1 = ANY/<> ALL
(SELECT ...)`), but the added coverage is currently limited to direct
`Expr::infer_placeholder_types` unit tests.
That verifies the expression-level behavior, but it doesn't confirm that the
parser/planner prepared-statement path actually constructs `SetComparison`,
runs placeholder inference, and executes correctly with a bound value.
Could we add end-to-end sqllogictest coverage in
`datafusion/sqllogictest/test_files/prepare.slt`, near the existing `$1 IN
(SELECT age FROM person)` cases, for both `$1 = ANY (SELECT age FROM person)`
and `$1 <> ALL (SELECT age FROM person)`?
##########
datafusion/expr/src/expr.rs:
##########
@@ -2212,6 +2212,33 @@ impl Expr {
}
}
}
+ Expr::SetComparison(SetComparison {
Review Comment:
`InSubquery` and `SetComparison` now contain very similar logic for
rewriting placeholders based on a single-column subquery schema.
As a follow-up, it might be worth extracting a small helper such as
`rewrite_placeholder_from_subquery_expr(kind, expr, subquery)?` so the
one-column invariant and associated error formatting stay centralized.
--
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]
