Repository navigation
perf: opt substr into make_scalar_function dict fast path - #25874
Open
Rich-T-kid wants to merge 1 commit into
Open
Rich-T-kid wants to merge 1 commit into
Rich-T-kid wants to merge 1 commit into
Conversation
Rich-T-kid
force-pushed
the
rich-T-kid/substr-dict-encoding-preservation
branch
from
September 29, 2026 13:26
71ad106 to
0f9e4cd
Compare
Contributor
Author
|
will wait until we can enable the dictionary flag to run the benchmarks. This should have a significant affect on query performance |
Rich-T-kid
force-pushed
the
rich-T-kid/substr-dict-encoding-preservation
branch
from
October 9, 2026 20:22
0f9e4cd to
497b608
Compare
Rich-T-kid
force-pushed
the
rich-T-kid/substr-dict-encoding-preservation
branch
2 times, most recently
from
October 9, 2026 20:43
6318fb4 to
d172c89
Compare
For scalar start/length, substr can run over the dict's unique values and reuse the keys instead of casting the dict to its value type. - Signature uses EncodingPreservation::dictionary() so coercion keeps the dict wrapper. - Fast path in invoke_with_args runs substr on dict.values() and rewraps via with_values(..). - Per-row start/length still materializes the dict. - return_field_from_args declares Dictionary for the scalar case and the value type for the per-row case so the planner's expected schema matches runtime output. Closes apache#26170
Rich-T-kid
force-pushed
the
rich-T-kid/substr-dict-encoding-preservation
branch
from
October 9, 2026 20:50
d172c89 to
c11a73c
Compare
Rich-T-kid
marked this pull request as ready for review
October 9, 2026 20:51
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
1 participant
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 #26170.
Rationale for this change
substr(dict_col, 1, 3)was casting the dict to its value type before running. For scalar start/length we can just substr the unique values and reuse the keys.What changes are included in this PR?
Keep the dict wrapper through coercion (
EncodingPreservation::dictionary()) and add a fast path that runssubstrondict.values(), then re-wraps viawith_values(..). Per-row start/length still falls back to materializing the dict.Are these changes tested?
Unit test covers the fast path. SLT regression (
dictionary_utf8.slt) assertsarrow_typeofstaysDictionary(..)for scalar start/length.Are there any user-facing changes?
No.