andygrove opened a new pull request, #5134: URL: https://github.com/apache/datafusion-comet/pull/5134
## Which issue does this PR close? Cherry-pick of #5127 (merged to `main` as `038fd6e25`) onto `branch-1.0`. Closes #4786 for the 1.0.0 release branch. ## Rationale for this change Comet's native `in` kernel always returns `false` for any operand against an empty `IN` list. Spark only behaves that way under the non-legacy behavior; under the legacy behavior a `NULL` operand evaluates to `NULL` (SPARK-44550), so Comet returns `false` where Spark returns `NULL`. That legacy behavior applies more often than the config name suggests: - Spark 3.4: no config exists and `In`/`InSet` always use the legacy behavior - Spark 3.5: `spark.sql.legacy.nullInEmptyListBehavior` defaults to `true` - Spark 4.0+: the config is optional and defaults to `!spark.sql.ansi.enabled`, so the legacy behavior is in effect whenever ANSI mode is disabled This is a correctness divergence on every supported Spark version, so it is worth carrying into 1.0.0 rather than leaving it as a documented known issue. ## What changes are included in this PR? A straight `git cherry-pick -x` of `038fd6e25`. The diff is byte-identical to the commit on `main`; there were no conflicts and no adaptation was needed. - `CometIn` / `CometInSet` return `Unsupported` for an empty list when the legacy behavior is in effect. Both serdes already mix in `CodegenDispatchFallback`, so these cases run Spark's own `doGenCode` inside the Comet pipeline instead of falling the whole operator back to Spark. - `CometNot`'s `Not(In(...))` fast path skips the same case so it defers to `CometIn` (otherwise `NOT (null IN ())` returned `true` instead of `NULL`). - Removes the now-fixed divergence from the compatibility guide (the entry added to `branch-1.0` by #5117), and replaces `CometIn`'s compatible note with an unsupported-case reason. `OptimizeIn` rewrites empty `IN` lists away, so this only affects plans where that rule is excluded; non-empty `IN` lists are unaffected. ## How are these changes tested? By the test added in #5127: `CometExpressionSuite` covers `a IN ()` and `NOT (a IN ())` over a Parquet scan with a `NULL` operand, across `spark.sql.legacy.nullInEmptyListBehavior` set to `true`, `false`, and unset (which exercises the version- and ANSI-derived default) with ANSI mode both on and off. Verified locally on this branch under both `-Pspark-3.5` and `-Pspark-4.0`; the test passes on each, and fails without the serde change. -- 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]
