andygrove opened a new pull request, #7237: URL: https://github.com/apache/datafusion-comet/pull/7237
## Which issue does this PR close? Part of #6834. This PR is stacked on #7235, which is stacked on #6853 and #6988, and holds all of their commits. The last two commits are the change here (`011718272e`, `0f01c13981`). The rest are #7235's four commits, #6853's four and #6988's one. The diff shrinks to those two once the others land. ## Rationale for this change `ArrowCachedBatchSerializer.gatherColumnStats` records a lower and an upper bound for each string column of every cached batch. For each row, `gatherStringStats` called `getUTF8String`, which allocates a `UTF8String`, and compared it through a boxed `Ordering[Any]`. That left strings as the most expensive column type in the statistics loop after the decimals fixed in #7235, and #6834 lists them as the next step. ## What changes are included in this PR? - For a plain 32-bit Arrow string vector under the default `StringType`, `gatherStringStats` compares the bytes in the vector's data buffer and builds no `UTF8String` per row. The comparison is that of `UTF8String.compareTo`: eight bytes at a time as big-endian unsigned words, then byte by byte as unsigned values, and a shorter string sorts before a longer one it prefixes. A row is compared with the lower bound first, and with the upper bound only when it is not below it. Only the rows of the two bounds become `UTF8String`s, as owned copies. - A collated string, a large-offset vector and a column that is not a plain Arrow vector keep the earlier loop, as `gatherStringStatsByValue`, and so keep comparing through the ordering `boundsOrderings` resolved for the column. The default collation is recognised by `dataType == StringType`, which a collated `StringType` is not equal to, so no version shim is needed. ## How are these changes tested? Four new tests in `ArrowCachedBatchStatsSuite` compare the bounds with the minimum and maximum of the strings' UTF-8 bytes under an unsigned byte-wise order: - lengths on both sides of the 8-byte words, a shared prefix of every length, a value that is a prefix of another, and one- to four-byte characters whose bytes are at or above 0x80, in three orders, with duplicates and nulls at the start, between and at the end, and single-value columns; - the values that sort after and before a long shared prefix, so that the byte after it decides the bound; - random strings over a small alphabet of one- and two-byte characters, so that many share long prefixes, with nulls; - a large-offset vector, which takes the fallback loop. Each of these fails if the comparison is changed: treating bytes as signed, comparing the words in native byte order, or dropping the tie-break on length. `CometInMemoryCacheSuite`, `CometInMemoryCacheKryoSuite` and `ArrowCachedBatchStatsSuite` pass on Spark 4.1 (107 tests), and the stats and Kryo suites pass on 3.5 and 3.4. Scalafix and the strict-warnings build pass. Benchmark (`CometInMemoryCacheBenchmark --stats-only`, Apple M3 Max, `local[1]`, 5M rows, Arrow vectors), run before, after, before in one session so that drift would show: | Case | before | after | before, again | | --- | ---: | ---: | ---: | | 3 long and 3 string columns | 161 ms | 63 ms | 157 ms | | 3 decimal columns (#7235) | 21 ms | 21 ms | 20 ms | The decimal case did not move, so the machine was steady between the runs. The string columns' share of the first case is not measured separately. -- 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]
