Skip to content

perf: preserve dictionary encoding for btrim, ltrim, and rtrim - #24100

Open
lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:preserve-dictionary-trim-functions
Open

perf: preserve dictionary encoding for btrim, ltrim, and rtrim#24100
lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:preserve-dictionary-trim-functions

Conversation

@lyne7-sc

@lyne7-sc lyne7-sc commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Previously, coercion materialized dictionary-encoded inputs for btrim, ltrim, and rtrim. This lost the encoding and evaluated the function for every row instead of once per dictionary value entry.

This PR extends dictionary preservation to these trim functions.

What changes are included in this PR?

  • Preserve dictionary encoding for btrim, ltrim, and rtrim.
  • Add tests and benchmarks.

Are these changes tested?

Yes, covered by slt

Are there any user-facing changes?

These functions now preserve dictionary encoding in their output.

Benchmarks

group                                                optimize                               main
-----                                                --------------                         ---------
dictionary_encoding/string/cardinality_10/btrim      1.00   286.8±13.36ns        ? ?/sec    137.69    39.5±4.56µs        ? ?/sec
dictionary_encoding/string/cardinality_10/ltrim      1.00    282.3±4.68ns        ? ?/sec    151.48    42.8±8.96µs        ? ?/sec
dictionary_encoding/string/cardinality_10/rtrim      1.00   278.8±13.01ns        ? ?/sec    119.91    33.4±3.85µs        ? ?/sec
dictionary_encoding/string/cardinality_100/btrim     1.00    718.4±4.53ns        ? ?/sec    51.89    37.3±0.92µs        ? ?/sec
dictionary_encoding/string/cardinality_100/ltrim     1.00    696.4±6.33ns        ? ?/sec    54.02    37.6±1.68µs        ? ?/sec
dictionary_encoding/string/cardinality_100/rtrim     1.00    659.2±6.91ns        ? ?/sec    55.33    36.5±6.84µs        ? ?/sec
dictionary_encoding/string/cardinality_1000/btrim    1.00      4.5±0.03µs        ? ?/sec    9.71     43.3±8.35µs        ? ?/sec
dictionary_encoding/string/cardinality_1000/ltrim    1.00      4.2±0.04µs        ? ?/sec    9.06     37.8±4.46µs        ? ?/sec
dictionary_encoding/string/cardinality_1000/rtrim    1.00      3.9±0.05µs        ? ?/sec    8.69     33.9±1.96µs        ? ?/sec
dictionary_encoding/string/cardinality_8192/btrim    1.01     37.4±1.64µs        ? ?/sec    1.00     37.1±0.68µs        ? ?/sec
dictionary_encoding/string/cardinality_8192/ltrim    1.00     34.7±1.75µs        ? ?/sec    1.01     35.1±1.55µs        ? ?/sec
dictionary_encoding/string/cardinality_8192/rtrim    1.00     31.9±1.18µs        ? ?/sec    1.17     37.3±5.68µs        ? ?/sec

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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.75510% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.91%. Comparing base (db0c31b) to head (0c46510).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions/src/string/btrim.rs 88.23% 1 Missing and 1 partial ⚠️
datafusion/functions/src/string/ltrim.rs 87.50% 1 Missing and 1 partial ⚠️
datafusion/functions/src/string/rtrim.rs 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24100      +/-   ##
==========================================
+ Coverage   80.89%   80.91%   +0.01%     
==========================================
  Files        1102     1102              
  Lines      376111   377100     +989     
  Branches   376111   377100     +989     
==========================================
+ Hits       304251   305115     +864     
- Misses      53753    53787      +34     
- Partials    18107    18198      +91     

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

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.

2 participants