andygrove commented on issue #5087: URL: https://github.com/apache/datafusion-comet/issues/5087#issuecomment-5152050580
## Verdicts for the eleven configs listed here I covered this list while re-running the #4180 audit against `apache/main` @ `ba21f02de` (default Maven profile, Spark 4.1.2, macOS aarch64, debug build). Full write-up is [on #4180](https://github.com/apache/datafusion-comet/issues/4180#issuecomment-5152048117). Method as requested here: differential run over Parquet-backed columns, each config tested at both `true` and `false`, comparing results, exceptions and executed plans between `spark.comet.enabled=false` and `true`. **None of the eleven diverged.** | Config | Verdict | What was exercised | |---|---|---| | `decimal.retainFractionDigitsOnTruncate` | Safe to ignore | `dec / dec`, `dec * dec`, and a six-way multiply plus chained divide that forces truncation at the precision limit. Comet ran the arithmetic (`CometProject` visible) | | `literal.pickMinimumPrecision` | Safe to ignore | `dec * 2.5`, `dec / 3` against a `decimal(10,2)` column, Comet executing | | `allowParameterlessCount` | Safe to ignore | `count()` with and without `GROUP BY`. Analyzer-side: with the flag off, both engines raise `WRONG_NUM_ARGS.WITH_SUGGESTION` identically | | `doLooseUpcast` | Safe to ignore | `cast(bigint as string)`, Comet executing | | `duplicateBetweenInput` | Safe to ignore | `n BETWEEN 1 AND 3`, `CometFilter` visible | | `inSubqueryNullability` | Safe to ignore | `n IN (SELECT ...)` as both a projection and a filter | | `scalarSubqueryCountBugBehavior` | Safe to ignore | Correlated `(SELECT count(*) ...)` in both the empty-match shape that triggers the count bug and the matching shape | | `disableMapKeyNormalization` | **Verified respected, and it does run natively** | `map(d, n)`, `map_from_arrays` and `str_to_map` over a Parquet column holding genuine `-0.0` and `NaN` doubles, with `CometProject` in the plan. This was the one flagged here as needing an actual differential check, and it passes at both values | | `setopsPrecedence.enabled` | Safe to ignore | `UNION ALL` combined with `INTERSECT` and with `EXCEPT`, where the flag changes which operator binds tighter | | `viewSchemaCompensation` | Safe to ignore | A view with a computed column, selected through with an extra cast, Comet executing the projection | | `readFileSourceTableCacheIgnoreOptions` | Safe to ignore | Catalog table over a Parquet location, filtered, `CometFilter` visible | ### One caveat worth recording rather than glossing My "did Comet actually execute this" probe walks `executedPlan` and does not descend into AQE query stages, so for the subquery and aggregate shapes above it under-reports Comet operators. Spark and Comet agree on results in every case, but for `inSubqueryNullability`, `scalarSubqueryCountBugBehavior`, `setopsPrecedence.enabled` and `allowParameterlessCount` I can confirm parity without confirming which engine evaluated the affected sub-plan. Those four are also the four most clearly consumed by the analyzer or optimizer, so Comet receiving an already-transformed plan is the expected reason they are safe. `disableMapKeyNormalization` is the one that genuinely needed the check, and it was confirmed on a non-AQE plan with `CometProject` present. ### Suggested disposition The eleven now have verified decisions, which was what this issue asked for. What is not done is the second half: capturing them in the compatibility guide so a reader can tell "considered and safe" from "not yet looked at", and adding regression tests. That gap is shared with #4180 and is tracked there. Happy for this issue to be closed in favour of #4180, or narrowed to just the documentation task, whichever the maintainers prefer. -- 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]
