Repository navigation
fix: preserve dictionary-value nulls in scalar regex operators - #23966
Merged
neilconway merged 2 commits intoJul 30, 2026
Merged
Conversation
Signed-off-by: discord9 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23966 +/- ##
========================================
Coverage 80.75% 80.75%
========================================
Files 1096 1096
Lines 373282 373651 +369
Branches 373282 373651 +369
========================================
+ Hits 301440 301746 +306
- Misses 53869 53892 +23
- Partials 17973 18013 +40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
neilconway
reviewed
Jul 29, 2026
neilconway
left a comment
neilconway
left a comment
Contributor
There was a problem hiding this comment.
Overall looks good! Nice fix, good test coverage.
Signed-off-by: discord9 <[email protected]>
neilconway
approved these changes
Jul 30, 2026
kosiew
pushed a commit
to kosiew/datafusion
that referenced
this pull request
Aug 12, 2026
…e#23966) ## Which issue does this PR close? - Closes apache#23963. ## Rationale for this change The scalar regex fast path for Dictionary arrays evaluates each dictionary value once and gathers the resulting booleans with `BooleanArray::take_iter`. That gather preserves null keys but drops null validity from the evaluated dictionary values. A key referencing a null dictionary value therefore becomes valid `false`, and negated regex operators can incorrectly turn it into `true`. This is distinct from apache#23722, which updates the array-pattern `regex_match_dyn` path. This PR fixes null propagation in `regex_match_dyn_scalar`. ## What changes are included in this PR? - Replace the validity-dropping dictionary gather with Arrow `take`, preserving both key validity and evaluated child validity. - Add a physical-expression regression test for `RegexMatch`, `RegexIMatch`, `RegexNotMatch`, and `RegexNotIMatch`. The test covers a null key and a key referencing a null dictionary value, and compares the Dictionary result with the equivalent cast-to-`Utf8` evaluation. ## Are these changes tested? Yes. - Focused regression: 1 passed. - `datafusion-physical-expr`: 1,649 passed, including the new regression. - Remaining workspace: 7,928 passed, 0 failed, 7 ignored; sqllogictest completed 499/499 files. - `datafusion` library: 461 passed. - `datafusion` core integration: 1,057 passed; four local memory-validation runners exceeded their host RSS thresholds and were isolated as environment-sensitive, unrelated to this patch. - `datafusion-cli`: 106 passed. - Formatting, workspace clippy with warnings denied, TOML formatting, license headers, changed-file typo checks, and `git diff --check` passed. ## Are there any user-facing changes? Dictionary-encoded string inputs now preserve SQL null semantics for scalar regex operators. There are no public API changes. --------- Signed-off-by: discord9 <[email protected]>
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
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?
Rationale for this change
The scalar regex fast path for Dictionary arrays evaluates each dictionary value once and gathers the resulting booleans with
BooleanArray::take_iter. That gather preserves null keys but drops null validity from the evaluated dictionary values. A key referencing a null dictionary value therefore becomes validfalse, and negated regex operators can incorrectly turn it intotrue.This is distinct from #23722, which updates the array-pattern
regex_match_dynpath. This PR fixes null propagation inregex_match_dyn_scalar.What changes are included in this PR?
take, preserving both key validity and evaluated child validity.RegexMatch,RegexIMatch,RegexNotMatch, andRegexNotIMatch. The test covers a null key and a key referencing a null dictionary value, and compares the Dictionary result with the equivalent cast-to-Utf8evaluation.Are these changes tested?
Yes.
datafusion-physical-expr: 1,649 passed, including the new regression.datafusionlibrary: 461 passed.datafusioncore integration: 1,057 passed; four local memory-validation runners exceeded their host RSS thresholds and were isolated as environment-sensitive, unrelated to this patch.datafusion-cli: 106 passed.git diff --checkpassed.Are there any user-facing changes?
Dictionary-encoded string inputs now preserve SQL null semantics for scalar regex operators. There are no public API changes.