Skip to content

Commit 0b9752c

Browse files
author
Guian Gumpac
committed
Addressed some PR comments and handled escaping wildcards
Signed-off-by: Guian Gumpac <[email protected]>
1 parent 5c225ce commit 0b9752c

File tree

7 files changed

+118
-32
lines changed

7 files changed

+118
-32
lines changed

‎core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@ public class OpenSearchFunctionsTest extends ExpressionTestBase {
3030
"body", ExprValueUtils.floatValue(.3F))))));
3131
private final NamedArgumentExpression query = new NamedArgumentExpression(
3232
"query", DSL.literal("search query"));
33-
34-
private final NamedArgumentExpression wildcardQuery = new NamedArgumentExpression(
35-
"query", DSL.literal("search query*"));
3633
private final NamedArgumentExpression analyzer = new NamedArgumentExpression(
3734
"analyzer", DSL.literal("keyword"));
3835
private final NamedArgumentExpression autoGenerateSynonymsPhrase = new NamedArgumentExpression(
@@ -203,9 +200,9 @@ void query_string() {
203200

204201
@Test
205202
void wildcard_query() {
206-
FunctionExpression expr = dsl.wildcard_query(field, wildcardQuery);
203+
FunctionExpression expr = dsl.wildcard_query(field, query);
207204
assertEquals(String.format("wildcard_query(field=%s, query=%s)",
208-
field.getValue(), wildcardQuery.getValue()),
205+
field.getValue(), query.getValue()),
209206
expr.toString());
210207
}
211208
}

‎docs/user/dql/functions.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3137,7 +3137,7 @@ Description
31373137

31383138
``wildcard_query(field_expression, query_expression[, option=<option_value>]*)``
31393139

3140-
The wildcard_query function maps to the wildcard_query query used in search engine, to return the documents that match a provided text with a given field.
3140+
The wildcard_query function maps to the wildcard_query query used in search engine. It returns documents that match provided text in the specified field.
31413141
Supported wildcard characters can be found here: https://opensearch.org/docs/latest/opensearch/query-dsl/term/#wildcards
31423142

31433143
Available parameters include:

‎docs/user/ppl/functions/relevance.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ Description
366366

367367
``wildcard_query(field_expression, query_expression[, option=<option_value>]*)``
368368

369-
The wildcard_query function maps to the wildcard_query query used in search engine, to return the documents that match a provided text with a given field.
369+
The wildcard_query function maps to the wildcard_query query used in search engine. It returns documents that match provided text in the specified field.
370370
Supported wildcard characters can be found here: https://opensearch.org/docs/latest/opensearch/query-dsl/term/#wildcards
371371

372372
Available parameters include:

‎opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/FunctionParameterRepository.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,9 @@ public class FunctionParameterRepository {
170170
public static final Map<String, RelevanceQuery.QueryBuilderStep<WildcardQueryBuilder>>
171171
WildcardQueryBuildActions = ImmutableMap.<String,
172172
RelevanceQuery.QueryBuilderStep<WildcardQueryBuilder>>builder()
173-
.put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue())))
174-
.put("case_insensitive", (b, v) -> b.caseInsensitive(Boolean.parseBoolean(v.stringValue())))
175-
.put("rewrite", (b, v) -> b.rewrite(v.stringValue()))
173+
.put("boost", (b, v) -> b.boost(convertFloatValue(v, "boost")))
174+
.put("case_insensitive", (b, v) -> b.caseInsensitive(convertBoolValue(v, "case_insensitive")))
175+
.put("rewrite", (b, v) -> b.rewrite(checkRewrite(v, "rewrite")))
176176
.build();
177177

178178
public static final Map<String, String> ArgumentLimitations =

‎opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/WildcardQuery.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,42 @@ public WildcardQuery() {
2121
super(FunctionParameterRepository.WildcardQueryBuildActions);
2222
}
2323

24+
private static final char DEFAULT_ESCAPE = '\\';
25+
2426
private String convertSqlWildcardToLucene(String text) {
25-
return text.replace('%', '*')
26-
.replace('_', '?');
27+
StringBuilder convertedString = new StringBuilder(text.length());
28+
boolean escaped = false;
29+
30+
for (char currentChar : text.toCharArray()) {
31+
switch (currentChar) {
32+
case DEFAULT_ESCAPE:
33+
escaped = true;
34+
convertedString.append(currentChar);
35+
break;
36+
case '%':
37+
if (escaped) {
38+
convertedString.deleteCharAt(convertedString.length() - 1);
39+
convertedString.append("%");
40+
} else {
41+
convertedString.append("*");
42+
}
43+
escaped = false;
44+
break;
45+
case '_':
46+
if (escaped) {
47+
convertedString.deleteCharAt(convertedString.length() - 1);
48+
convertedString.append("_");
49+
} else {
50+
convertedString.append('?');
51+
}
52+
escaped = false;
53+
break;
54+
default:
55+
convertedString.append(currentChar);
56+
escaped = false;
57+
}
58+
}
59+
return convertedString.toString();
2760
}
2861

2962
@Override

‎opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,69 @@ void wildcard_query_convert_sql_wildcard_to_lucene() {
682682
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));
683683
}
684684

685+
@Test
686+
void wildcard_query_escape_sql_wildcard_characters() {
687+
var expected = "{\n"
688+
+ " \"wildcard\" : {\n"
689+
+ " \"field\" : {\n"
690+
+ " \"wildcard\" : \"search query%\",\n"
691+
+ " \"boost\" : 1.0\n"
692+
+ " }\n"
693+
+ " }\n"
694+
+ "}";
695+
var actual = buildQuery(dsl.wildcard_query(
696+
dsl.namedArgument("field", literal("field")),
697+
dsl.namedArgument("query", literal("search query\\%"))));
698+
699+
assertTrue(new JSONObject(expected).similar(new JSONObject(actual)),
700+
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));
701+
702+
expected = "{\n"
703+
+ " \"wildcard\" : {\n"
704+
+ " \"field\" : {\n"
705+
+ " \"wildcard\" : \"search query_\",\n"
706+
+ " \"boost\" : 1.0\n"
707+
+ " }\n"
708+
+ " }\n"
709+
+ "}";
710+
actual = buildQuery(dsl.wildcard_query(
711+
dsl.namedArgument("field", literal("field")),
712+
dsl.namedArgument("query", literal("search query\\_"))));
713+
714+
assertTrue(new JSONObject(expected).similar(new JSONObject(actual)),
715+
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));
716+
717+
expected = "{\n"
718+
+ " \"wildcard\" : {\n"
719+
+ " \"field\" : {\n"
720+
+ " \"wildcard\" : \"search query\\\\*\",\n"
721+
+ " \"boost\" : 1.0\n"
722+
+ " }\n"
723+
+ " }\n"
724+
+ "}";
725+
actual = buildQuery(dsl.wildcard_query(
726+
dsl.namedArgument("field", literal("field")),
727+
dsl.namedArgument("query", literal("search query\\*"))));
728+
729+
assertTrue(new JSONObject(expected).similar(new JSONObject(actual)),
730+
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));
731+
732+
expected = "{\n"
733+
+ " \"wildcard\" : {\n"
734+
+ " \"field\" : {\n"
735+
+ " \"wildcard\" : \"search query\\\\?\",\n"
736+
+ " \"boost\" : 1.0\n"
737+
+ " }\n"
738+
+ " }\n"
739+
+ "}";
740+
actual = buildQuery(dsl.wildcard_query(
741+
dsl.namedArgument("field", literal("field")),
742+
dsl.namedArgument("query", literal("search query\\?"))));
743+
744+
assertTrue(new JSONObject(expected).similar(new JSONObject(actual)),
745+
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));
746+
}
747+
685748
@Test
686749
void should_build_wildcard_query_with_default_parameters() {
687750
var expected = "{\n"
@@ -708,7 +771,7 @@ void should_build_wildcard_query_query_with_custom_parameters() {
708771
+ " \"wildcard\" : \"search query*\",\n"
709772
+ " \"boost\" : 0.6,\n"
710773
+ " \"case_insensitive\" : true,\n"
711-
+ " \"rewrite\" : \"top_terms_boost_N\"\n"
774+
+ " \"rewrite\" : \"constant_score_boolean\"\n"
712775
+ " }\n"
713776
+ " }\n"
714777
+ "}";
@@ -717,7 +780,7 @@ void should_build_wildcard_query_query_with_custom_parameters() {
717780
dsl.namedArgument("query", literal("search query*")),
718781
dsl.namedArgument("boost", literal("0.6")),
719782
dsl.namedArgument("case_insensitive", literal("true")),
720-
dsl.namedArgument("rewrite", literal("top_terms_boost_N"))));
783+
dsl.namedArgument("rewrite", literal("constant_score_boolean"))));
721784

722785
assertTrue(new JSONObject(expected).similar(new JSONObject(actual)),
723786
StringUtils.format("Actual %s doesn't match neither expected %s", actual, expected));

‎opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/WildcardQueryTest.java

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.opensearch.sql.opensearch.storage.script.filter.lucene;
77

88
import static org.junit.jupiter.api.Assertions.assertThrows;
9+
import static org.opensearch.sql.expression.DSL.namedArgument;
910

1011
import java.util.List;
1112
import java.util.stream.Stream;
@@ -22,27 +23,23 @@
2223
import org.opensearch.sql.expression.DSL;
2324
import org.opensearch.sql.expression.Expression;
2425
import org.opensearch.sql.expression.FunctionExpression;
25-
import org.opensearch.sql.expression.NamedArgumentExpression;
26-
import org.opensearch.sql.expression.config.ExpressionConfig;
2726
import org.opensearch.sql.expression.env.Environment;
2827
import org.opensearch.sql.expression.function.FunctionName;
2928
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.WildcardQuery;
3029

3130
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
3231
class WildcardQueryTest {
33-
private static final DSL dsl = new ExpressionConfig()
34-
.dsl(new ExpressionConfig().functionRepository());
3532
private final WildcardQuery wildcardQueryQuery = new WildcardQuery();
36-
private final FunctionName wildcardQueryFunc = FunctionName.of("wildcard_query");
33+
private static final FunctionName wildcardQueryFunc = FunctionName.of("wildcard_query");
3734

3835
static Stream<List<Expression>> generateValidData() {
3936
return Stream.of(
4037
List.of(
41-
dsl.namedArgument("field", DSL.literal("title")),
42-
dsl.namedArgument("query", DSL.literal("query_value*")),
43-
dsl.namedArgument("boost", DSL.literal("0.7")),
44-
dsl.namedArgument("case_insensitive", DSL.literal("false")),
45-
dsl.namedArgument("rewrite", DSL.literal("constant_score_boolean"))
38+
DSL.namedArgument("field", DSL.literal("title")),
39+
DSL.namedArgument("query", DSL.literal("query_value*")),
40+
DSL.namedArgument("boost", DSL.literal("0.7")),
41+
DSL.namedArgument("case_insensitive", DSL.literal("false")),
42+
DSL.namedArgument("rewrite", DSL.literal("constant_score_boolean"))
4643
)
4744
);
4845
}
@@ -63,25 +60,21 @@ public void test_SyntaxCheckException_when_no_arguments() {
6360

6461
@Test
6562
public void test_SyntaxCheckException_when_one_argument() {
66-
List<Expression> arguments = List.of(namedArgument("field", "title"));
63+
List<Expression> arguments = List.of(namedArgument("field", DSL.literal("title")));
6764
assertThrows(SyntaxCheckException.class,
6865
() -> wildcardQueryQuery.build(new WildcardQueryExpression(arguments)));
6966
}
7067

7168
@Test
7269
public void test_SemanticCheckException_when_invalid_parameter() {
7370
List<Expression> arguments = List.of(
74-
namedArgument("field", "title"),
75-
namedArgument("query", "query_value*"),
76-
namedArgument("unsupported", "unsupported_value"));
71+
namedArgument("field", DSL.literal("title")),
72+
namedArgument("query", DSL.literal("query_value*")),
73+
namedArgument("unsupported", DSL.literal("unsupported_value")));
7774
Assertions.assertThrows(SemanticCheckException.class,
7875
() -> wildcardQueryQuery.build(new WildcardQueryExpression(arguments)));
7976
}
8077

81-
private NamedArgumentExpression namedArgument(String name, String value) {
82-
return dsl.namedArgument(name, DSL.literal(value));
83-
}
84-
8578
private class WildcardQueryExpression extends FunctionExpression {
8679
public WildcardQueryExpression(List<Expression> arguments) {
8780
super(WildcardQueryTest.this.wildcardQueryFunc, arguments);

0 commit comments

Comments
 (0)