LinSimon-901101 commented on PR #5844: URL: https://github.com/apache/datafusion-comet/pull/5844#issuecomment-5636220756
Thanks for the review. I've added the requested coverage and benchmark. ### Tests The LAST_WIN SQL tests now cover null/empty arrays, null values (including a duplicate key whose last value is null), null keys, and unequal array lengths. An additional Scala test verifies dispatcher execution and Spark exception parity for the error cases. Local validation passed: - Spark 3.4.3, 3.5.9, and 4.0.4: 4 focused tests each. - Spark 4.1.3: 26 SQL/map tests and 18 Parquet encryption integration tests. - Formatting, style, license checks, and Scalafix CHECK on Spark 4.0. The SQL fixture disables Spark's nested-column vectorized reader to avoid Spark 3.4.3's primitive-array null-copy issue ([SPARK-48019](https://issues.apache.org/jira/browse/SPARK-48019)). Comet operator and dispatcher assertions remain enabled. This setting is not applied to the benchmark or production. ### Benchmark Added `CometMapFromArraysBenchmark`, covering unique/duplicate keys, array lengths 4/256, and standalone/mixed projections. It compares dispatcher execution, Spark projection fallback, and pure Spark, verifying results and execution paths before timing. Two release-build runs used Apple M1 Max, Spark 4.1.3, JDK 21, local[1], and a 4 GiB heap. Average time in milliseconds; fallback shows start/end measurements: | Keys / length / projection | Run 1 fallback → dispatch | Run 2 fallback → dispatch | | --- | --- | --- | | Unique / 4 / standalone | 165/170 → 155 | 177/178 → 165 | | Unique / 4 / mixed | 174/179 → 159 | 184/186 → 174 | | Duplicate / 4 / standalone | 191/174 → 170 | 134/145 → 158 | | Duplicate / 4 / mixed | 187/183 → 181 | 156/165 → 175 | | Unique / 256 / standalone | 145/147 → 161 | 140/137 → 149 | | Unique / 256 / mixed | 150/148 → 158 | 143/143 → 158 | | Duplicate / 256 / standalone | 134/131 → 138 | 126/129 → 139 | | Duplicate / 256 / mixed | 140/132 → 140 | 133/132 → 131 | Small unique-key cases were faster with dispatch; large unique-key cases were slower. Duplicate-key results varied. These local measurements include scanning and query execution and do not establish a universal speedup. Both runs completed successfully, with a Hadoop shutdown-hook classloading warning after completion. The production implementation and the separate native null-key limitation in #4680 remain unchanged. These are local validation results, not confirmation of GitHub CI. Attached are the raw logs from both benchmark runs, including execution settings and timing statistics. [Benchmark_run_01.log](https://github.com/user-attachments/files/32113507/CometMapFromArraysBenchmark.log) [Benchmark_run_02.log](https://github.com/user-attachments/files/32113514/CometMapFromArraysBenchmark.log) -- 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]
