Skip to content

feat: run length on binary input natively - #5874

Open
dwsmith1983 wants to merge 1 commit into
apache:mainfrom
dwsmith1983:feat/length-binary-native
Open

feat: run length on binary input natively#5874
dwsmith1983 wants to merge 1 commit into
apache:mainfrom
dwsmith1983:feat/length-binary-native

Conversation

@dwsmith1983

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #2348.

Rationale for this change

length(binary) currently falls back to Spark because Comet registered DataFusion's character_length, which accepts string types only. datafusion-spark now ships a Spark-compatible length that returns the byte count for binary and the character count for strings, so the fallback can go.

What changes are included in this PR?

  • Register SparkLengthFunc in the native session under length, char_length, character_length and len, replacing character_length. The result is Int32 for every input width, matching Spark.
  • Drop the BinaryType fallback from CometLength.
  • A Rust roundtrip test through the Parquet schema adapter shows dictionary-typed string and binary columns are cast to Utf8/Binary before 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 checks length runs natively on it.
  • Refresh the audit note in the string function audit doc and the stale notes in the bit_length/octet_length fixtures.

How are these changes tested?

  • length.sql gains 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, substring and CAST(string AS BINARY), the Int32 result in arithmetic, a filter, a GROUP BY aggregate and after a native shuffle, and the char_length/character_length aliases. Every block asserts a fully native plan.
  • CometStringExpressionSuite runs length on 1000 binary rows with and without dictionary encoding, and on the dictionary-typed file described above.
  • A Rust test asserts length resolves to the Spark function returning Int32 for Utf8, LargeUtf8, Utf8View, Binary, LargeBinary and BinaryView with no cast inserted.
  • Verified locally on Spark 3.5 and 4.0: CometSqlFileTestSuite length, CometStringExpressionSuite, cargo test, cargo clippy -D warnings, spotless.

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.
@github-actions github-actions Bot added enhancement New feature or request area:scan Parquet scan / data reading area:expressions Expression evaluation labels Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation area:scan Parquet scan / data reading enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support length function with non-string input

1 participant