Skip to content

Commit d8e7467

Browse files
jackylee-chjackylee-ch
authored
fix(duckdb): escape LIKE metacharacters in pushed string filters (#11072)
DuckDB matches the argument of `contains`, `prefix` and `suffix` literally, and its `LIKE` has no escape character. The converter passed both to Vortex `LIKE` unchanged, where `%` and `_` are wildcards and `\` escapes the next character, so filters pushed into a Vortex scan returned wrong rows: - `prefix(s, 'page_')`, and `s GLOB 'page_*'`, which DuckDB rewrites to it, also returned `pageXview`. - `contains(s, 'a_c')` also returned `abc`, and `contains(s, '%')` matched every string. - `s LIKE 'C:\%'` returned `C:%` instead of `C:\x`. The converter now escapes `%`, `_` and `\` in those arguments, as the Spark connector does for its string predicates, and `\` in a constant `LIKE` pattern. A `LIKE` whose pattern is not a constant is left to DuckDB. Arguments without these characters push the same pattern as before. Signed-off-by: jackylee-ch <[email protected]>
1 parent 9ba7f6e commit d8e7467

2 files changed

Lines changed: 89 additions & 4 deletions

File tree

‎vortex-duckdb/src/convert/expr.rs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,18 @@ fn string_matcher_pattern(children: &[&ExpressionRef]) -> Option<String> {
106106
}
107107
}
108108

109+
/// Escape each of `chars` in `pattern` with `\`, the escape character of Vortex LIKE.
110+
fn escape_like(pattern: &str, chars: &[char]) -> String {
111+
let mut escaped = String::with_capacity(pattern.len());
112+
for c in pattern.chars() {
113+
if chars.contains(&c) {
114+
escaped.push('\\');
115+
}
116+
escaped.push(c);
117+
}
118+
escaped
119+
}
120+
109121
/// Whether the expression's return type is a `LIST` or fixed-size `ARRAY`.
110122
fn returns_a_list(expr: &ExpressionRef) -> bool {
111123
matches!(
@@ -312,17 +324,17 @@ fn try_from_bound_function(
312324
like @ ("~~" | "!~~") => {
313325
let children: Vec<_> = func.children().collect();
314326
vortex_ensure_eq!(children.len(), 2);
315-
let Some(string) = try_from_expression_inner(children[0], ctx)? else {
327+
let Some(pattern) = from_bound_str(children[1]) else {
316328
return Ok(None);
317329
};
318-
let Some(target) = try_from_expression_inner(children[1], ctx)? else {
330+
let Some(string) = try_from_expression_inner(children[0], ctx)? else {
319331
return Ok(None);
320332
};
321333
let opts = LikeOptions {
322334
negated: like == "!~~",
323335
case_insensitive: false,
324336
};
325-
Like.new_expr(opts, [string, target])
337+
Like.new_expr(opts, [string, lit(escape_like(&pattern, &['\\']))])
326338
}
327339
matchers @ ("contains" | "prefix" | "suffix") => {
328340
let children: Vec<_> = func.children().collect();
@@ -332,6 +344,7 @@ fn try_from_bound_function(
332344
let Some(value) = try_from_expression_inner(children[0], ctx)? else {
333345
return Ok(None);
334346
};
347+
let pattern = escape_like(&pattern, &['%', '_', '\\']);
335348
let pattern = match matchers {
336349
"contains" => format!("%{pattern}%"),
337350
"prefix" => format!("{pattern}%"),
@@ -459,7 +472,8 @@ pub fn can_push_expression(value: &ExpressionRef) -> bool {
459472
ExpressionClass::BoundFunction(func) => {
460473
let children: Vec<_> = func.children().collect();
461474
let supported = match func.scalar_function.name() {
462-
"~~" | "!~~" | "strlen" => true,
475+
"~~" | "!~~" => children.len() == 2 && from_bound_str(children[1]).is_some(),
476+
"strlen" => true,
463477
"struct_extract" => children.len() == 2 && from_bound_str(children[1]).is_some(),
464478
"contains" | "prefix" | "suffix" => string_matcher_pattern(&children).is_some(),
465479
"array_length" => children.len() == 1,

‎vortex-sqllogictest/slt/duckdb/like_pushdown.slt‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
# SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
4+
include ../setup.slt.no
5+
6+
statement ok
7+
COPY (SELECT * FROM (VALUES ('a_c', 'a_c'), ('abc', 'b'), ('100%', '1%'), ('1000', '1%'),
8+
('page_view', 'p%'), ('pageXview', 'p%'), ('C:' || chr(92) || 'x', 'C:' || chr(92) || '_'), ('C:%', 'C:%'),
9+
(NULL, NULL)) t(s, p))
10+
TO '${WORK_DIR}/like-pushdown.vortex';
11+
12+
# contains, prefix and suffix match their argument literally
13+
query T
14+
SELECT s FROM '${WORK_DIR}/like-pushdown.vortex' WHERE contains(s, 'a_c');
15+
----
16+
a_c
17+
18+
query T
19+
SELECT s FROM '${WORK_DIR}/like-pushdown.vortex' WHERE prefix(s, 'page_');
20+
----
21+
page_view
22+
23+
query T
24+
SELECT s FROM '${WORK_DIR}/like-pushdown.vortex' WHERE s GLOB 'page_*';
25+
----
26+
page_view
27+
28+
query T
29+
SELECT s FROM '${WORK_DIR}/like-pushdown.vortex' WHERE suffix(s, '0%');
30+
----
31+
100%
32+
33+
query TT
34+
EXPLAIN (FORMAT json) SELECT s FROM '${WORK_DIR}/like-pushdown.vortex' WHERE prefix(s, 'page_');
35+
----
36+
<REGEX>:"Filters": "[^"]* like
37+
38+
# A backslash in a LIKE pattern matches itself
39+
query T
40+
SELECT replace(s, chr(92), '/') FROM '${WORK_DIR}/like-pushdown.vortex' WHERE s LIKE 'C:' || chr(92) || '%';
41+
----
42+
C:/x
43+
44+
query T
45+
SELECT replace(s, chr(92), '/') FROM '${WORK_DIR}/like-pushdown.vortex' WHERE s LIKE 'C:' || chr(92) || '_';
46+
----
47+
C:/x
48+
49+
# A pattern that isn't a constant is evaluated by DuckDB
50+
query T
51+
SELECT replace(s, chr(92), '/') FROM '${WORK_DIR}/like-pushdown.vortex' WHERE s LIKE p ORDER BY s;
52+
----
53+
100%
54+
1000
55+
C:%
56+
C:/x
57+
a_c
58+
pageXview
59+
page_view
60+
61+
query T
62+
SELECT replace(s, chr(92), '/') FROM '${WORK_DIR}/like-pushdown.vortex' WHERE s LIKE s ORDER BY s;
63+
----
64+
100%
65+
1000
66+
C:%
67+
C:/x
68+
a_c
69+
abc
70+
pageXview
71+
page_view

0 commit comments

Comments
 (0)