Skip to content

perf: preserve dictionary encoding for bit_length, octet_length, and ascii#23743

Merged
Jefffrey merged 5 commits into
apache:mainfrom
lyne7-sc:preserve-dictionary-string-functions
Jul 23, 2026
Merged

perf: preserve dictionary encoding for bit_length, octet_length, and ascii#23743
Jefffrey merged 5 commits into
apache:mainfrom
lyne7-sc:preserve-dictionary-string-functions

Conversation

@lyne7-sc

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

String functions normally materialize Dictionary(K, Utf8) inputs before evaluation. This loses the dictionary encoding and applies the function to every row instead of only the unique dictionary values.

This pr extends the dictionary-preserving implementation from #22905 to ascii, bit_length, and octet_length.

What changes are included in this PR?

  • Preserve dictionary encoding for ascii, bit_length, and octet_length.
  • Preserve return types for regular and nested dictionaries.
  • Add slts and Dictionary cardinality benchmarks.

Are these changes tested?

Yes, covered by SLTs.

Are there any user-facing changes?

Yes. These functions now preserve Dictionary encoding in their output: Dictionary(K, Utf8) -> Dictionary(K, Int32)

Benchmarks

group                                                        main                                   new
-----                                                        ----------------                       ---
dictionary_string_functions/cardinality_10/ascii             28.71     5.0±0.05µs        ? ?/sec    1.00   172.5±12.31ns        ? ?/sec
dictionary_string_functions/cardinality_10/bit_length        4.01   698.7±25.75ns        ? ?/sec    1.00    174.1±2.45ns        ? ?/sec
dictionary_string_functions/cardinality_10/octet_length      3.63   651.3±58.51ns        ? ?/sec    1.00    179.4±3.27ns        ? ?/sec
dictionary_string_functions/cardinality_100/ascii            18.92     4.9±0.03µs        ? ?/sec    1.00    258.6±8.28ns        ? ?/sec
dictionary_string_functions/cardinality_100/bit_length       3.11   723.4±27.14ns        ? ?/sec    1.00   232.5±35.47ns        ? ?/sec
dictionary_string_functions/cardinality_100/octet_length     3.06   651.6±26.97ns        ? ?/sec    1.00    213.2±6.96ns        ? ?/sec
dictionary_string_functions/cardinality_1000/ascii           6.32      4.8±0.03µs        ? ?/sec    1.00   766.7±70.66ns        ? ?/sec
dictionary_string_functions/cardinality_1000/bit_length      3.06   776.1±67.85ns        ? ?/sec    1.00    253.9±6.93ns        ? ?/sec
dictionary_string_functions/cardinality_1000/octet_length    3.10  807.9±188.65ns        ? ?/sec    1.00   261.0±22.42ns        ? ?/sec
dictionary_string_functions/cardinality_8192/ascii           1.00      5.1±0.15µs        ? ?/sec    1.00      5.1±0.24µs        ? ?/sec
dictionary_string_functions/cardinality_8192/bit_length      1.00   699.1±17.54ns        ? ?/sec    1.09   763.7±36.57ns        ? ?/sec
dictionary_string_functions/cardinality_8192/octet_length    1.00   677.3±62.91ns        ? ?/sec    1.11   750.7±32.56ns        ? ?/sec

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jul 21, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.01299% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.69%. Comparing base (4184b07) to head (e03e14f).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions/src/string/ascii.rs 77.27% 2 Missing and 3 partials ⚠️
datafusion/functions/src/string/octet_length.rs 85.71% 3 Missing ⚠️
datafusion/functions/src/string/bit_length.rs 95.00% 1 Missing ⚠️
datafusion/functions/src/utils.rs 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23743      +/-   ##
==========================================
- Coverage   80.70%   80.69%   -0.02%     
==========================================
  Files        1089     1089              
  Lines      368038   368484     +446     
  Branches   368038   368484     +446     
==========================================
+ Hits       297031   297351     +320     
- Misses      53308    53415     +107     
- Partials    17699    17718      +19     

☔ 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.

ASCII(ascii_2) as c2,
ASCII(unicode_1) as c3,
ASCII(unicode_2) as c4
arrow_cast(ASCII(ascii_1), 'Int32') as c1,

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.

is this to preserve the return type above? query IIII

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this keeps the existing value test unchanged.

@Jefffrey
Jefffrey added this pull request to the merge queue Jul 23, 2026
@Jefffrey

Copy link
Copy Markdown
Contributor

thanks @lyne7-sc

Merged via the queue into apache:main with commit f4f43f7 Jul 23, 2026
38 checks passed
@lyne7-sc

Copy link
Copy Markdown
Contributor Author

Thanks for the review @Jefffrey

@lyne7-sc
lyne7-sc deleted the preserve-dictionary-string-functions branch July 23, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants