ErikBPF commented on PR #5828: URL: https://github.com/apache/datafusion-comet/pull/5828#issuecomment-5624036378
Thanks @andygrove for the detailed feedback and for independently checking the red/green regressions and full suites. Addressed all six points in e3c877e6b: - Added `assertExchangeReuseOver(plan, clue)(pf)` to `CometTestBase` and used it at all four call sites, including the aggregate regressions. - Added explicit `AdaptiveSparkPlanExec` assertions to the AQE-enabled join and generator cases. - Removed the `#5824` test-name prefixes and updated the PR's focused test command. - Included `outer` in `CometExplodeExec.stringArgs`. Broadcast join explain output stays unchanged. - Derived `isNullAwareAntiJoin` directly from the Spark operator in `createExec`. - Used `java.lang.Boolean` type ascriptions for both hash inputs, matching the surrounding conventions. Validation on the revised patch: 193 tests passed across the full Spark 4.1 join, generator, and aggregate suites, with 0 failures and 2 pre-existing ignored aggregate-metrics tests. All 21 focused cases passed on both Spark 4.1 and Spark 3.5/Scala 2.12. Packaging, semantic and syntactic Scalafix, Spotless, Scalastyle, RAT, Prettier, and suite-registration checks also passed locally. -- 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]
