parthchandra commented on PR #6447: URL: https://github.com/apache/datafusion-comet/pull/6447#issuecomment-5919313999
Notes on the comparison-builder normalization: - **`spark/src/test/resources/sql-tests/expressions/conditional/float_comparisons.sql:88`** — the fixture covers `=`, `<=>`, `<`, `<=`, `>`, `>=`, `!=`, but there's no `IS DISTINCT FROM` query, which is a separate native operator with its own routing. It's covered in the Rust unit tests, but since this SQL file is the Spark-comparison fixture, please add an `a IS DISTINCT FROM b` case end to end. - **`float_comparisons.sql`** (end of file) — the nested cases cover one level of `ARRAY<DOUBLE>` and `STRUCT<v: DOUBLE>`. Please add a deeper shape like `ARRAY<STRUCT<v: DOUBLE>>` or `STRUCT<a: ARRAY<DOUBLE>>`. `normalize_nested_floats` recurses, so it should just work, but a deeper column proves it for the ordering operators. - **`spark/src/test/scala/org/apache/comet/exec/CometNativeReaderSuite.scala:1538`** — the pruning test writes `range(0,1000)` cast to double, so there's no NaN or `-0.0` in the data. It confirms pruning still fires, but not the other direction: a row group Spark would match because it holds a NaN (Spark treats `NaN > 500.0` as true) must not be pruned away by the raw comparison. Can you confirm a NaN-bearing row group survives here, and consider a small NaN case? This is unchanged by the PR, so a coverage question, not a regression. - **`native/spark-expr/src/array_funcs/nested_comparison.rs:272`** — for nested ordering operators both operands are deep-copied through `normalize_nested_floats` per batch, unlike nested `=`/`<>` which compare in place. The PR body and module doc already note the planned no-copy comparator; just flagging the cost is real for wide nested columns. -- 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]
