andygrove commented on PR #4514: URL: https://github.com/apache/datafusion-comet/pull/4514#issuecomment-4580624684
Good call. I dug into the two failing tests and you are right that both check query results, not just plan structure: - `SQLQuerySuite` "Common subexpression elimination" uses `verifyCallCount`, which runs `checkAnswer` *and* asserts the UDF invocation count. - `SQLQueryTestSuite` `udf-select_having.sql` is golden-file output matching. So I switched away from disabling the dispatcher and went with adjusting the expectations instead, keeping the feature exercised: - CSE test: expected count is now `if (isCometEnabled) 3 else 1` for the aggregate case. `CometProject`/`CometHashAggregate` do not do cross-sibling subexpression elimination over `ScalaUDF`, so the UDF runs once per reference. The result is unchanged. - `udf-select_having.sql`: `1/udf(a)` now divides by zero natively and surfaces as `CometNativeException` instead of `SparkArithmeticException`. I normalize both sides to a `DIVIDE_BY_ZERO` placeholder, gated on `isCometEnabled`. I filed follow-ons for the underlying gaps and linked them in the diff comments: #4516 (cross-sibling CSE over ScalaUDF) and #4517 (native divide-by-zero surfaces the wrong exception type). Regenerated all four diffs (3.4.3 / 3.5.8 / 4.0.2 / 4.1.1) via the clone-apply-modify-regenerate workflow and merged latest main. -- 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]
