Skip to content

feat: support binary types in octet_length - #24867

Merged
comphead merged 3 commits into
apache:mainfrom
eddietejeda:feat/octet-length-binary
Sep 10, 2026
Merged

feat: support binary types in octet_length#24867
comphead merged 3 commits into
apache:mainfrom
eddietejeda:feat/octet-length-binary

Conversation

@eddietejeda

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

octet_length should count the raw bytes of binary data without expensive encoding workaround.

What changes are included in this PR?

  • Adds support for Binary, LargeBinary, and BinaryView.
  • Returns Int64 for LargeBinary and Int32 for the other types.
  • Updates the error message to say “strings or binary.”

What is the testing strategy for this PR?

Unit and SQL logic tests cover binary arrays and scalars, non-UTF-8 data, nulls, dictionary encoding, and return types.

Are there any user-facing changes?

Yes. octet_length now counts the raw bytes of binary values. Existing string behavior is unchanged.

octet_length now accepts Binary, LargeBinary, and BinaryView. The
signature uses the same shape as md5 and digest. LargeBinary returns
Int64. The other two return Int32.

No coercion to Utf8 runs. Thus the function counts the raw bytes of
non-UTF-8 input.

Also reword the message in get_optimal_return_type! to "strings or
binary".
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Sep 2, 2026
Comment thread datafusion/sqllogictest/test_files/functions.slt Outdated

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eddietejeda for the PR, commented

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.60%. Comparing base (09a2aff) to head (bd167e1).
⚠️ Report is 67 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions/src/string/octet_length.rs 98.91% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #24867   +/-   ##
=======================================
  Coverage   81.60%   81.60%           
=======================================
  Files        1123     1123           
  Lines      411514   411604   +90     
  Branches   411514   411604   +90     
=======================================
+ Hits       335811   335894   +83     
- Misses      55924    55932    +8     
+ Partials    19779    19778    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Switch the binary signature from Native(logical_binary()) to
TypeSignatureClass::Binary, which admits FixedSizeBinary.

TypeSignatureClass::Binary now coerces a NULL argument to Binary
instead of passing DataType::Null downstream.

Updated end user documentation.
@github-actions github-actions Bot added documentation Improvements or additions to documentation logical-expr Logical plan and expressions labels Sep 2, 2026
@eddietejeda
eddietejeda requested a review from comphead September 3, 2026 03:24
@eddietejeda

eddietejeda commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for your feedback, @comphead. Anything else before merging?

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eddietejeda looks good to me!

@comphead
comphead added this pull request to the merge queue Sep 10, 2026
Merged via the queue into apache:main with commit 5e42247 Sep 10, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation functions Changes to functions implementation logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

octet_length should accept binary types

3 participants