Repository navigation
feat(arrow-string): support run-end encoded operands in like kernels - #12154
Conversation
Accept run-end encoded operands, including dictionary-encoded run values, in like, ilike, nlike, nilike, starts_with, ends_with, contains and eq_ignore_ascii_case, for both string and binary values. Run-end encoded operands are rewritten into encodings the kernels already support, so type checks, null handling and errors are shared with the unencoded path. A scalar pattern is matched once per run covered by the slice and the result is repeated across the run. Array patterns look up each row through a dictionary view of the runs, which does not copy values. The existing string test macros now also run every case on run-end encoded arrays. Add an ree_like benchmark group alongside ree_comparison. Prepared with AI assistance. Co-authored-by: Amp <[email protected]>
LIKE and ILIKE type coercion used to cast run-end encoded operands to their string value type, decoding every row before matching. Keep a run-end encoded operand uncast when its values (optionally dictionary encoded) already have the common LIKE type, so the kernels match each run once. Other operands are still cast to the common type, and SIMILAR TO still decodes because the regex kernels need plain strings. The rule lives next to `like_coercion` in `datafusion-expr-common` and is shared by the analyzer and the physical `like` type check. Depends on apache/arrow-rs#12154, which adds run-end encoded support to the Arrow LIKE kernels. Without it these queries fail with "Invalid string/binary operation".
LIKE type coercion decoded run-end encoded operands to their string values, which expands every run before matching. Look through run-end encoding in `like_value_type`, so a run-end encoded operand whose values (optionally dictionary encoded) already have the common type is kept and the kernels match each run once. Depends on apache/arrow-rs#12154, which adds run-end encoded support to the Arrow LIKE kernels. Without it these queries fail with "Invalid string/binary operation".
Rich-T-kid
left a comment
There was a problem hiding this comment.
can we split the benchmarks into another pr or make it the first commit so we can use the bot to compare
|
The benchmarks would just error without this change so there is nothing to compare against. |
|
Friendly ping. Does the PR look good otherwise? |
|
sorry I haven't taken too close of a look. i've been busy with other work, ill try and take a look soon |
Keep an outer RunEndEncoded over Utf8 or dictionary-encoded Utf8 when the coerced pattern is Utf8. Preserve ordinary casts for other layouts and SIMILAR TO. Requires Arrow LIKE kernel support from apache/arrow-rs#12154. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c09d-06c3-76af-8d8a-d3c4ebe25726 Co-authored-by: Amp <[email protected]>
Use arrow-string from apache/arrow-rs#12154 at 5d192d080196a116075a18712ea2d572e00547b1. Keep all other Arrow packages on registry 60.1.0 so the branch has one Arrow type graph. Replace this wiring with a released Arrow dependency before upstream integration. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c09d-06c3-76af-8d8a-d3c4ebe25726 Co-authored-by: Amp <[email protected]>
Keep an outer RunEndEncoded over Utf8 or dictionary-encoded Utf8 when the coerced pattern is Utf8. Preserve ordinary casts for other layouts and SIMILAR TO. Requires Arrow LIKE kernel support from apache/arrow-rs#12154. Co-authored-by: Amp <[email protected]> Backport note: the SIMILAR TO cases in run_end_encoded.slt use the pattern 'Ab_c' instead of 'Ab%'. This branch lacks apache#23188, which translates SQL wildcards in SIMILAR TO, so 'Ab%' matches a literal %. The cases still check that SIMILAR TO keeps the cast. (cherry picked from commit 019dde2) Amp-Thread-ID: https://ampcode.com/threads/T-01a1cfce-da43-7260-bbd6-097791371a27
Keep an outer RunEndEncoded over Utf8 or Dictionary(_, Utf8) uncast when the LIKE pattern coerces to Utf8, as LIKE already does for Dictionary(_, Utf8). Other layouts and SIMILAR TO keep their casts. Requires Arrow LIKE kernel support from apache/arrow-rs#12154. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a1d03b-18cf-75e2-9af2-58698a0382fd
Use arrow-string from apache/arrow-rs#12154 at 5d192d080196a116075a18712ea2d572e00547b1. Keep all other Arrow packages on registry 60.1.0 so the branch has one Arrow type graph. Drop this commit once an Arrow release includes apache/arrow-rs#12154. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a1d03b-18cf-75e2-9af2-58698a0382fd
| Ok(expand_runs(left, &like_op(op, &run_values(left), rhs)?)) | ||
| } else if left_is_scalar { | ||
| Ok(expand_runs(right, &like_op(op, lhs, &run_values(right))?)) | ||
| } else if left.is_empty() { |
There was a problem hiding this comment.
This returns Ok([]) before validating the underlying types. Could we preserve type validation here and add a mismatched empty-input test?
let run: RunArray<Int32Type> = std::iter::empty::<&str>().collect();
let right = Int32Array::from(Vec::<i32>::new());
assert!(like(&run, &right).is_err());This currently returns Ok(BooleanArray::from([])).
There was a problem hiding this comment.
Good catch, done!
Empty run-end encoded inputs returned an empty result without checking operand types. Match their run values through like_op instead, which checks types and avoids building a dictionary without values.
alamb
left a comment
There was a problem hiding this comment.
I took a good look at this too -- thank you @Rich-T-kid and @asubiotto
| } else if left_is_scalar { | ||
| Ok(expand_runs(right, &like_op(op, lhs, &run_values(right))?)) | ||
| } else if left.is_empty() { | ||
| // The dictionary kernels panic on a dictionary without values, so match the |
There was a problem hiding this comment.
is there a reason that the dictionary kernels panic on empty values? Maybe we should fix that (and we could remove this special case) -- to consider for a follow on PR
Which issue does this PR close?
Rationale for this change
The string matching kernels return an error for
RunEndEncodedinputs, so callers have to decode before filtering.What changes are included in this PR?
like,ilike,nlike,nilike,starts_with,ends_with,containsandeq_ignore_ascii_caseaccept run-end encoded operands, including run-end encoded dictionaries. Value types are the same as for unencoded inputs, so binary values also work withstarts_with,ends_withandcontains.Run-end encoded operands are rewritten into encodings the kernels already support:
Adds an
ree_likebenchmark group.Are these changes tested?
Yes. The existing string test macros also run each case on
RunArray, and new tests cover slicing mid-run, nulls, dictionary values, run-end encoded patterns, empty inputs, type errors and binary values.Benchmarks for 65,536 rows:
like_flat_scalarlike_ree_scalarlike_ree_dict_scalarlike_ree_decoded_scalarlike_ree_flatAre there any significant user-facing changes?
No. Run-end encoded inputs that previously returned an error are now supported.
AI assistance was used for this PR but I thoroughly reviewed this change before opening a PR.
cc @alamb @Rich-T-kid