andygrove opened a new pull request, #7235: URL: https://github.com/apache/datafusion-comet/pull/7235
## Which issue does this PR close? Part of #6834. This PR is stacked on #6853 and #6988, and holds their commits as well. The last four commits are the change here (`e30bc831b9`, `8a9394fdaa`, `f20e6e123e`, `158e105a71`). The rest are #6853's four commits and #6988's one. The diff shrinks to those four once both land. ## Rationale for this change `ArrowCachedBatchSerializer.gatherColumnStats` records a lower and an upper bound for each decimal column of every cached batch. For each row, `gatherDecimalStats` called `CometVector.getDecimal`, which for a precision past 18 digits builds a `BigInteger` and a `BigDecimal` from the 16 bytes in Arrow's data buffer, and then a `Decimal` around them. Most of the statistics time of a decimal column was that allocation. #6834 lists it as the next step after the null counts of #6853. ## What changes are included in this PR? - `gatherDecimalStats` reads a plain Arrow decimal column's 128-bit unscaled values straight from its data buffer and compares them without building anything per row. A value is a signed high word over an unsigned low word, little-endian. A narrow decimal is the sign extension of its low word, so one comparison covers every precision. All the values of a column share one scale, so their unscaled order is their numeric order. Only the rows of the two bounds become a `Decimal`, through the same `getDecimal` as before, so the bounds keep the type they had. - A decimal column that is not a plain Arrow vector (a dictionary-encoded one, for instance) takes the earlier by-value loop, which is kept as `gatherDecimalStatsByValue`. - `CometInMemoryCacheBenchmark` gets a decimal case in its statistics benchmark: precisions 10, 18 and 38 at scale 2, one row in ten null, and values on both sides of zero. ## How are these changes tested? New `ArrowCachedBatchStatsSuite` builds Arrow `DecimalVector`s at precisions 5, 10, 18, 19, 20, 30 and 38 with a range of scales, and compares the bounds and null count with a `BigDecimal` minimum and maximum: - values on either side of each word boundary of a 128-bit value (the low word's sign bit, a carry into the high word, both signs), in three orders; - the same values without the largest magnitudes, in positive-only and negative-only sets, because at the full range the high word alone decides the bounds and the low word's comparison never matters; - random values of random widths, with and without nulls, and single-value columns. Comparing the low word as signed instead of unsigned fails the suite at 19, 20, 30 and 38 digits. `CometInMemoryCacheSuite`, `CometInMemoryCacheKryoSuite` and the new suite pass on Spark 4.1 (103 tests). The new suite and the Kryo suite also pass on 3.5 and 3.4. Benchmark (`CometInMemoryCacheBenchmark --stats-only`, Apple M3 Max, `local[1]`, 5M rows of the three decimal columns above, Arrow vectors), run old, new, old in one session so that drift would show: | Collector | Time for 5M rows | | --- | ---: | | before (by value) | 153 ms | | after (data buffer) | 21 ms | | before, again | 154 ms | The long and string cases of the same benchmark did not move (160, 160 and 161 ms), so the machine was steady between the runs. -- 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]
