Skip to content

fix: support Dictionary arrays in regex_match_dyn (#23709) #23722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jithendra2608 wants to merge 3 commits into
base: main
Choose a base branch
from
+46 −0
Open

fix: support Dictionary arrays in regex_match_dyn (#23709) #23722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d9c57c
fix: support Dictionary arrays in regex_match_dyn (#23709)
Jul 20, 2026
1b82685
address reviewer feedback: add unit test, restore docs, fix imports
Jul 21, 2026
b77ad24
refactor: use public take API per copilot suggestion
Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions datafusion/physical-expr/src/expressions/binary/kernels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use arrow::compute::kernels::bitwise::{
};
use arrow::compute::kernels::boolean::not;
use arrow::compute::kernels::comparison::{regexp_is_match, regexp_is_match_scalar};
use arrow::compute::take;
use arrow::datatypes::DataType;
use datafusion_common::{Result, ScalarValue};
use datafusion_common::{exec_err, internal_err, plan_err};
Expand Down Expand Up @@ -213,6 +214,11 @@ pub(crate) fn regex_match_dyn(
DataType::LargeUtf8 => {
regexp_is_match_flag!(left, right, LargeStringArray, not_match, flag)
}
DataType::Dictionary(_, _) => {
let dict = left.as_any_dictionary();
let unpacked_left = take(dict.values().as_ref(), dict.keys(), None)?;
regex_match_dyn(&unpacked_left, right, not_match, flag)

@u70b3 u70b3 Jul 21, 2026

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.

One case worth knowing about: on this branch's base, a dictionary whose value width differs from the pattern — e.g. arrow_cast(s, 'Dictionary(Int32, Utf8View)') SIMILAR TO pat — unpacks fine here, but this recursive call then hits .expect("failed to downcast array") in regexp_is_match_flag! and panics (before this PR, the same query returned a clean "not supported" internal error, so on this base the PR turns an error into a panic for that combo). It's the same pre-existing panic class as #22886 for plain arrays, and #23704 on current main already replaced those .expect()s with exec_err! — so rebasing onto main turns this into a clean execution error. I verified that on a cherry-pick of this commit; more context in my summary comment.

}
other => internal_err!(
"Data type {} not supported for regex_match_dyn on string array",
other
Expand Down Expand Up @@ -298,3 +304,23 @@ pub(crate) fn regex_match_dyn_scalar(
};
Some(result)
}

#[test]
fn test_regex_match_dyn_dictionary() -> Result<()> {
let values = StringArray::from(vec![Some("user auth failed"), Some("anonymous")]);
let keys = Int32Array::from(vec![Some(0), Some(1), None, Some(0)]);
let left: ArrayRef = Arc::new(DictionaryArray::new(keys, Arc::new(values)));
let right: ArrayRef = Arc::new(StringArray::from(vec![
Some("(auth|login)"),
Some("^anon"),
Some("x"),
Some("nope"),
]));

let result = regex_match_dyn(&left, &right, false, false)?;
assert_eq!(
result.as_any().downcast_ref::<BooleanArray>().unwrap(),
&BooleanArray::from(vec![Some(true), Some(true), None, Some(false)])
);
Ok(())
}
20 changes: 20 additions & 0 deletions datafusion/sqllogictest/test_files/regexp/regexp_match.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,23 @@ query B
select null !~* 'abc';
----
NULL


# Test for Issue 23709: regex_match_dyn with Dictionary encoded value side
statement ok
CREATE TABLE dict_regex_t AS SELECT * FROM (VALUES ('user auth failed')) v(s);

statement ok
CREATE TABLE dict_regex_p AS SELECT * FROM (VALUES ('(auth|login)')) v(pat);

query B
SELECT arrow_cast(dict_regex_t.s, 'Dictionary(Int32, Utf8)') SIMILAR TO dict_regex_p.pat FROM dict_regex_t CROSS JOIN dict_regex_p;

@u70b3 u70b3 Jul 21, 2026

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.

Heads-up: after a rebase onto current main, this test passes without exercising the new kernel arm — #23704's analyzer coercion now inserts CAST(... AS Utf8) around the dictionary value side (EXPLAIN shows CAST(CAST(s@0 AS Dictionary(Int32, Utf8)) AS Utf8) ~ pat@1), so the query decodes before reaching regex_match_dyn. Definitely keep this as end-to-end regression coverage for the #23709 repro, but please also add a Rust unit test that calls regex_match_dyn with a DictionaryArray directly so the arm itself is covered — an example is in my summary comment.

----
true

statement ok
DROP TABLE dict_regex_t;

statement ok
DROP TABLE dict_regex_p;