HyukjinKwon commented on code in PR #59227:
URL: https://github.com/apache/spark/pull/59227#discussion_r4229142746
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2StrategySuite.scala:
##########
@@ -350,6 +350,39 @@ class DataSourceV2StrategySuite extends SharedSparkSession
{
assert(V2ExpressionUtils.toCatalyst(unknownPred).isEmpty)
}
+ Seq(true, false).foreach { ansiEnabled =>
+ test(s"boolean operators wrap boolean casts for V2 pushdown with ANSI
$ansiEnabled") {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> ansiEnabled.toString) {
+ val intCol = AttributeReference("c", IntegerType)()
+ val boolCast = Cast(intCol, BooleanType)
+ val predicate = GreaterThan(intCol, Literal(0))
+ assert(new V2ExpressionBuilder(boolCast).build().isDefined ==
ansiEnabled)
+ val conditions = Seq[Expression](
+ Not(boolCast),
+ And(boolCast, predicate),
+ Or(predicate, boolCast))
+ conditions.foreach { condition =>
+ val filter = Coalesce(Seq(condition, Literal(false)))
+ assert(DataSourceV2Strategy.translateFilterV2(filter).isDefined ==
ansiEnabled)
Review Comment:
**Nit (P3):** These assertions (here and in the `buildPredicate()` check
just below) only compare `isDefined`, so they never observe the wrapping that
the test name describes. Under ANSI on, any defined result passes. A wrong but
defined AND/OR tree would still be green, for example one with a dropped or
duplicated operand, a missing or misplaced `BOOLEAN_EXPRESSION` wrapper, or the
operator wrapped instead of the operand. The JDBC test pins only the NOT shape,
so the AND/OR translation that connectors now receive has no shape-sensitive
check.
This suite already pins exact trees with `testTranslateFilter` (and the
VariantGet test checks the `BOOLEAN_EXPRESSION` wrapper and its child). Could
these cases assert the expected predicates instead? For example,
`COALESCE(V2And(BOOLEAN_EXPRESSION(CAST(c AS BOOLEAN)), c > 0), ALWAYS_FALSE)`
for the AND case under ANSI on, and `None` under ANSI off or with wrapping
disabled.
**Verification:**
- **Regression:** Exact-tree assertions for nested NOT, AND and OR over a
Boolean cast fail if the operand wrapper or the operator structure changes.
- **Behavior:** The DSv2 strategy translation tests pass under both ANSI
modes and with predicate wrapping disabled.
--
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]
