Skip to content

Commit 3e263b5

Browse files
fa-assistantfornwallgshankfindepi
committed
Make modules.re methods accept an already compiled pattern (#5430)
* Make `modules.re` methods accept an already compiled pattern COPYBARA PUBLIC PR: #601 Fixes part of #581 ("Compilation failures: The user's example still produces empty files in Fusion"). Methods in `modules.re` (such as `search` and `match`) can take both a string for the initial pattern argument as well an already compiled expression. Before this change only the string variant was correctly supported: ```jinja2 {%- set pattern = '.*' -%} {%- set result = re.search(pattern, 'xyz') -%} ``` This change makes compiled patterns work as well, restoring compatibility with [pythons re](https://docs.python.org/3/library/re.html) / dbt-core / jinja2: ```jinja2 {%- set pattern = re.compile('.*') -%} {%- set result = re.search(pattern, 'xyz') -%} ``` The rust edition in the `minijinja-contrib` crate was bumped to `2024` to allow the [let chain](rust-lang/rust#132833) used here. Bumping the edition means that `cargo fmt` formats differently - the (otherwise unrelated) formatting changes are done in the second commit in this PR, while the first commit contains the actual change. GitOrigin-RevId: 99d438c * Make `modules.re` methods accept an already compiled pattern COPYBARA PUBLIC PR: #601 Fixes part of #581 ("Compilation failures: The user's example still produces empty files in Fusion"). Methods in `modules.re` (such as `search` and `match`) can take both a string for the initial pattern argument as well an already compiled expression. Before this change only the string variant was correctly supported: ```jinja2 {%- set pattern = '.*' -%} {%- set result = re.search(pattern, 'xyz') -%} ``` This change makes compiled patterns work as well, restoring compatibility with [pythons re](https://docs.python.org/3/library/re.html) / dbt-core / jinja2: ```jinja2 {%- set pattern = re.compile('.*') -%} {%- set result = re.search(pattern, 'xyz') -%} ``` The rust edition in the `minijinja-contrib` crate was bumped to `2024` to allow the [let chain](rust-lang/rust#132833) used here. Bumping the edition means that `cargo fmt` formats differently - the (otherwise unrelated) formatting changes are done in the second commit in this PR, while the first commit contains the actual change. GitOrigin-RevId: 9dd9420 * Cargo fmt * remove old changelog * Change code to avoid clippy complaint about unstable let * rename conflicting test name * update test output * Improve sdf-make-sql-functions build command Let cargo consider only the actually necessary dependencies. When `-p` is not present, cargo will do dependency feature unification across all crates first. * Remove table qualifiers in UNION plan After UNION (or other set operation), the names are no longer qualified. Keeping them qualified is incorrect (allows referencing which would be rejected by CDW) and causes planning failures in DataFusion 48. * Make `modules.re` methods accept an already compiled pattern COPYBARA PUBLIC PR: #601 Fixes part of #581 ("Compilation failures: The user's example still produces empty files in Fusion"). Methods in `modules.re` (such as `search` and `match`) can take both a string for the initial pattern argument as well an already compiled expression. Before this change only the string variant was correctly supported: ```jinja2 {%- set pattern = '.*' -%} {%- set result = re.search(pattern, 'xyz') -%} ``` This change makes compiled patterns work as well, restoring compatibility with [pythons re](https://docs.python.org/3/library/re.html) / dbt-core / jinja2: ```jinja2 {%- set pattern = re.compile('.*') -%} {%- set result = re.search(pattern, 'xyz') -%} ``` The rust edition in the `minijinja-contrib` crate was bumped to `2024` to allow the [let chain](rust-lang/rust#132833) used here. Bumping the edition means that `cargo fmt` formats differently - the (otherwise unrelated) formatting changes are done in the second commit in this PR, while the first commit contains the actual change. GitOrigin-RevId: 99d438c * Cargo fmt * fix bad merge * Update output for flaky test test_regression_tests_internal_analytics_build_selector_cloud_users_1 * Remove old changelog --------- Co-authored-by: Fredrik Fornwall <[email protected]> Co-authored-by: Gerda Shank <[email protected]> Co-authored-by: Gerda Shank <[email protected]> Co-authored-by: Piotr Findeisen <[email protected]> GitOrigin-RevId: dafcf71594623b1ce94e126eb1dd6f9a6896656a
1 parent f4af669 commit 3e263b5

File tree

3 files changed

+39
-11
lines changed

3 files changed

+39
-11
lines changed

crates/dbt-jinja/minijinja-contrib/src/filters/datetime.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fn value_to_datetime(
3939
ErrorKind::InvalidOperation,
4040
"not a valid date or timestamp",
4141
)
42-
.with_source(original_err))
42+
.with_source(original_err));
4343
}
4444
},
4545
},

crates/dbt-jinja/minijinja-contrib/src/modules/py_datetime/time.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ impl PyTimeClass {
9292
return Err(Error::new(
9393
ErrorKind::InvalidArgument,
9494
format!("Invalid iso time format: {iso_str}: {e}"),
95-
))
95+
));
9696
}
9797
};
9898

crates/dbt-jinja/minijinja-contrib/src/modules/re.rs

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ fn re_compile(args: &[Value]) -> Result<Value, Error> {
4949
#[derive(Debug, Clone)]
5050
pub struct Pattern {
5151
raw: String,
52-
_compiled: Regex,
52+
compiled: Regex,
5353
}
5454

5555
impl Pattern {
5656
pub fn new(raw: &str, compiled: Regex) -> Self {
5757
Self {
5858
raw: raw.to_string(),
59-
_compiled: compiled, // TODO: use this in re methods
59+
compiled,
6060
}
6161
}
6262
}
@@ -307,13 +307,27 @@ fn get_or_compile_regex_and_text(args: &[Value]) -> Result<(Box<Regex>, &str), E
307307
}
308308

309309
// First arg: either compiled or raw pattern
310-
let pattern = args[0].to_string();
311-
let compiled = Box::new(Regex::new(&pattern).map_err(|e| {
312-
Error::new(
313-
ErrorKind::InvalidOperation,
314-
format!("Failed to compile regex: {e}"),
315-
)
316-
})?);
310+
let compiled = if let Some(object) = args[0].as_object() {
311+
if let Some(pattern) = object.downcast_ref::<Pattern>() {
312+
Box::new(pattern.compiled.clone())
313+
} else {
314+
let pattern = args[0].to_string();
315+
Box::new(Regex::new(&pattern).map_err(|e| {
316+
Error::new(
317+
ErrorKind::InvalidOperation,
318+
format!("Failed to compile regex: {e}"),
319+
)
320+
})?)
321+
}
322+
} else {
323+
let pattern = args[0].to_string();
324+
Box::new(Regex::new(&pattern).map_err(|e| {
325+
Error::new(
326+
ErrorKind::InvalidOperation,
327+
format!("Failed to compile regex: {e}"),
328+
)
329+
})?)
330+
};
317331

318332
// Second arg: the text to match against
319333
let text = args[1].to_string();
@@ -472,4 +486,18 @@ mod tests {
472486
assert!(!result.is_true());
473487
assert_eq!(result.to_string(), "none");
474488
}
489+
490+
#[test]
491+
fn test_re_glob_search() {
492+
let result = re_search(&[
493+
Value::from(".*".to_string()),
494+
Value::from("xyz".to_string()),
495+
])
496+
.unwrap();
497+
assert!(result.is_true());
498+
499+
let compiled_pattern = re_compile(&[Value::from(".*".to_string())]).unwrap();
500+
let result = re_search(&[compiled_pattern, Value::from("xyz".to_string())]).unwrap();
501+
assert!(result.is_true());
502+
}
475503
}

0 commit comments

Comments
 (0)