feat: run length on binary input natively - #5874
Open
dwsmith1983 wants to merge 1 commit into
Open
Conversation
Register datafusion-spark's length function in the native session so length, char_length, character_length and len return the byte count for binary input and the character count for strings, always as Int32. Drop the BinaryType fallback from CometLength. The native Parquet scan casts dictionary-typed columns to the required Spark type before any expression sees them, so the replaced character_length's dictionary handling is not needed; a Rust roundtrip test and a Scala test over an Arrow-written dictionary-typed file pin that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #2348.
Rationale for this change
length(binary)currently falls back to Spark because Comet registered DataFusion'scharacter_length, which accepts string types only.datafusion-sparknow ships a Spark-compatiblelengththat returns the byte count for binary and the character count for strings, so the fallback can go.What changes are included in this PR?
SparkLengthFuncin the native session underlength,char_length,character_lengthandlen, replacingcharacter_length. The result isInt32for every input width, matching Spark.BinaryTypefallback fromCometLength.Utf8/Binarybefore any expression sees them, so the replaced function's dictionary handling is not needed downstream. A Scala test reads a small Arrow-written Parquet file whose embedded schema declares dictionary-typed columns and checkslengthruns natively on it.bit_length/octet_lengthfixtures.How are these changes tested?
length.sqlgains a binary table and queries covering multi-byte text, bytes that are not valid UTF-8, embedded NUL bytes, empty, NULL and all-NULL columns, binary reached through a struct field, an array element, a map value,unhex,substringandCAST(string AS BINARY), theInt32result in arithmetic, a filter, aGROUP BYaggregate and after a native shuffle, and thechar_length/character_lengthaliases. Every block asserts a fully native plan.CometStringExpressionSuiterunslengthon 1000 binary rows with and without dictionary encoding, and on the dictionary-typed file described above.lengthresolves to the Spark function returningInt32forUtf8,LargeUtf8,Utf8View,Binary,LargeBinaryandBinaryViewwith no cast inserted.CometSqlFileTestSuite length,CometStringExpressionSuite,cargo test,cargo clippy -D warnings, spotless.