Skip to content

Commit ac9b63a

Browse files
MitchellGaleMitchellGale
authored
Add any case of arguments in relevancy based functions to be allowed (#744)
* Added new functionality around argument names not case specific for relevancy based functions. Added unit tests to cover updates. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Fixed test cases that used uppercase values in arguments. Signed-off-by: MitchellGale-BitQuill <[email protected]> Signed-off-by: MitchellGale-BitQuill <[email protected]>
1 parent deececb commit ac9b63a

File tree

16 files changed

+99
-23
lines changed

16 files changed

+99
-23
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public MatchPhrasePrefixQuery() {
2323
.put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue())))
2424
.put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())))
2525
.put("zero_terms_query", (b, v) -> b.zeroTermsQuery(
26-
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())))
26+
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v))))
2727
.put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue())))
2828
.build());
2929
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public MatchPhraseQuery() {
3434
.put("analyzer", (b, v) -> b.analyzer(v.stringValue()))
3535
.put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue())))
3636
.put("zero_terms_query", (b, v) -> b.zeroTermsQuery(
37-
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())))
37+
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v))))
3838
.build());
3939
}
4040

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public MatchQuery() {
2424
.put("analyzer", (b, v) -> b.analyzer(v.stringValue()))
2525
.put("auto_generate_synonyms_phrase_query",
2626
(b, v) -> b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue())))
27-
.put("fuzziness", (b, v) -> b.fuzziness(v.stringValue()))
27+
.put("fuzziness", (b, v) -> b.fuzziness(valueOfToUpper(v)))
2828
.put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())))
2929
.put("prefix_length", (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue())))
3030
.put("fuzzy_transpositions",
@@ -34,7 +34,7 @@ public MatchQuery() {
3434
.put("operator", (b, v) -> b.operator(Operator.fromString(v.stringValue())))
3535
.put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue()))
3636
.put("zero_terms_query", (b, v) -> b.zeroTermsQuery(
37-
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())))
37+
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v))))
3838
.put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue())))
3939
.build());
4040
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public MultiMatchQuery() {
4141
.put("type", (b, v) -> b.type(v.stringValue()))
4242
.put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue())))
4343
.put("zero_terms_query", (b, v) -> b.zeroTermsQuery(
44-
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())))
44+
org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v))))
4545
.build());
4646
}
4747

@@ -67,14 +67,15 @@ public QueryBuilder build(FunctionExpression func) {
6767
.fields(fieldsAndWeights);
6868
while (iterator.hasNext()) {
6969
NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next();
70-
if (!queryBuildActions.containsKey(arg.getArgName())) {
70+
String argNormalized = arg.getArgName().toLowerCase();
71+
if (!queryBuildActions.containsKey(argNormalized)) {
7172
throw new SemanticCheckException(
7273
String.format("Parameter %s is invalid for %s function.",
73-
arg.getArgName(), queryBuilder.getWriteableName()));
74+
argNormalized, queryBuilder.getWriteableName()));
7475
}
7576
(Objects.requireNonNull(
7677
queryBuildActions
77-
.get(arg.getArgName())))
78+
.get(argNormalized)))
7879
.apply(queryBuilder, arg.getValue().valueOf(null));
7980
}
8081
return queryBuilder;

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public QueryStringQuery() {
5959
.put("phrase_slop", (b, v) -> b.phraseSlop(Integer.parseInt(v.stringValue())))
6060
.put("quote_field_suffix", (b, v) -> b.quoteFieldSuffix(v.stringValue()))
6161
.put("rewrite", (b, v) -> b.rewrite(v.stringValue()))
62-
.put("type", (b, v) -> b.type(MultiMatchQueryBuilder.Type.parse(v.stringValue(),
62+
.put("type", (b, v) -> b.type(MultiMatchQueryBuilder.Type.parse(valueOfToLower(v),
6363
LoggingDeprecationHandler.INSTANCE)))
6464
.put("tie_breaker", (b, v) -> b.tieBreaker(Float.parseFloat(v.stringValue())))
6565
.put("time_zone", (b, v) -> b.timeZone(v.stringValue()))
@@ -93,14 +93,15 @@ public QueryBuilder build(FunctionExpression func) {
9393
.fields(fieldsAndWeights);
9494
while (iterator.hasNext()) {
9595
NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next();
96-
if (!queryBuildActions.containsKey(arg.getArgName())) {
96+
String argNormalized = arg.getArgName().toLowerCase();
97+
if (!queryBuildActions.containsKey(argNormalized)) {
9798
throw new SemanticCheckException(
9899
String.format("Parameter %s is invalid for %s function.",
99-
arg.getArgName(), queryBuilder.getWriteableName()));
100+
argNormalized, queryBuilder.getWriteableName()));
100101
}
101102
(Objects.requireNonNull(
102103
queryBuildActions
103-
.get(arg.getArgName())))
104+
.get(argNormalized)))
104105
.apply(queryBuilder, arg.getValue().valueOf(null));
105106
}
106107
return queryBuilder;

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,15 @@ public QueryBuilder build(FunctionExpression func) {
4646
Iterator<Expression> iterator = arguments.listIterator(2);
4747
while (iterator.hasNext()) {
4848
NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next();
49-
if (!queryBuildActions.containsKey(arg.getArgName())) {
49+
String argNormalized = arg.getArgName().toLowerCase();
50+
if (!queryBuildActions.containsKey(argNormalized)) {
5051
throw new SemanticCheckException(
5152
String.format("Parameter %s is invalid for %s function.",
52-
arg.getArgName(), queryBuilder.getWriteableName()));
53+
argNormalized, queryBuilder.getWriteableName()));
5354
}
5455
(Objects.requireNonNull(
5556
queryBuildActions
56-
.get(arg.getArgName())))
57+
.get(argNormalized)))
5758
.apply(queryBuilder, arg.getValue().valueOf(null));
5859
}
5960
return queryBuilder;
@@ -70,4 +71,12 @@ public interface QueryBuilderStep<T extends QueryBuilder> extends
7071
BiFunction<T, ExprValue, T> {
7172

7273
}
74+
75+
public static String valueOfToUpper(ExprValue v) {
76+
return v.stringValue().toUpperCase();
77+
}
78+
79+
public static String valueOfToLower(ExprValue v) {
80+
return v.stringValue().toLowerCase();
81+
}
7382
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public SimpleQueryStringQuery() {
3232
b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue())))
3333
.put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue())))
3434
.put("default_operator", (b, v) -> b.defaultOperator(Operator.fromString(v.stringValue())))
35-
.put("flags", (b, v) -> b.flags(Arrays.stream(v.stringValue().split("\\|"))
35+
.put("flags", (b, v) -> b.flags(Arrays.stream(valueOfToUpper(v).split("\\|"))
3636
.map(SimpleQueryStringFlag::valueOf)
3737
.toArray(SimpleQueryStringFlag[]::new)))
3838
.put("fuzzy_max_expansions", (b, v) ->
@@ -69,14 +69,15 @@ public QueryBuilder build(FunctionExpression func) {
6969
.fields(fieldsAndWeights);
7070
while (iterator.hasNext()) {
7171
NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next();
72-
if (!queryBuildActions.containsKey(arg.getArgName())) {
72+
String argNormalized = arg.getArgName().toLowerCase();
73+
if (!queryBuildActions.containsKey(argNormalized)) {
7374
throw new SemanticCheckException(
7475
String.format("Parameter %s is invalid for %s function.",
75-
arg.getArgName(), queryBuilder.getWriteableName()));
76+
argNormalized, queryBuilder.getWriteableName()));
7677
}
7778
(Objects.requireNonNull(
7879
queryBuildActions
79-
.get(arg.getArgName())))
80+
.get(argNormalized)))
8081
.apply(queryBuilder, arg.getValue().valueOf(null));
8182
}
8283
return queryBuilder;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ void match_phrase_invalid_value_ztq() {
851851
dsl.namedArgument("query", literal("search query")),
852852
dsl.namedArgument("zero_terms_query", literal("meow")));
853853
var msg = assertThrows(IllegalArgumentException.class, () -> buildQuery(expr)).getMessage();
854-
assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.meow",
854+
assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.MEOW",
855855
msg);
856856
}
857857

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ public void test_zero_terms_query_parameter() {
9494
Assertions.assertNotNull(matchPhrasePrefixQuery.build(new MatchPhraseExpression(arguments)));
9595
}
9696

97+
@Test
98+
public void test_zero_terms_query_parameter_lower_case() {
99+
List<Expression> arguments = List.of(
100+
dsl.namedArgument("field", "t1"),
101+
dsl.namedArgument("query", "t2"),
102+
dsl.namedArgument("zero_terms_query", "all")
103+
);
104+
Assertions.assertNotNull(matchPhrasePrefixQuery.build(new MatchPhraseExpression(arguments)));
105+
}
97106

98107
@Test
99108
public void test_boost_parameter() {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ public void test_zero_terms_query_parameter() {
9999
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
100100
}
101101

102+
@Test
103+
public void test_zero_terms_query_parameter_lower_case() {
104+
List<Expression> arguments = List.of(
105+
namedArgument("field", "t1"),
106+
namedArgument("query", "t2"),
107+
namedArgument("zero_terms_query", "all")
108+
);
109+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
110+
}
111+
102112
private class MatchPhraseExpression extends FunctionExpression {
103113
public MatchPhraseExpression(List<Expression> arguments) {
104114
super(MatchPhraseQueryTest.this.matchPhrase, arguments);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ static Stream<List<Expression>> generateValidData() {
9696
dsl.namedArgument("query", DSL.literal("query_value")),
9797
dsl.namedArgument("zero_terms_query", DSL.literal("NONE"))
9898
),
99+
List.of(
100+
dsl.namedArgument("field", DSL.literal("field_value")),
101+
dsl.namedArgument("query", DSL.literal("query_value")),
102+
dsl.namedArgument("zero_terms_query", DSL.literal("none"))
103+
),
99104
List.of(
100105
dsl.namedArgument("field", DSL.literal("field_value")),
101106
dsl.namedArgument("query", DSL.literal("query_value")),

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ static Stream<List<Expression>> generateValidData() {
120120
dsl.namedArgument("fields", fields_value),
121121
dsl.namedArgument("query", query_value),
122122
dsl.namedArgument("zero_terms_query", DSL.literal("ALL"))
123+
),
124+
List.of(
125+
dsl.namedArgument("fields", fields_value),
126+
dsl.namedArgument("query", query_value),
127+
dsl.namedArgument("zero_terms_query", DSL.literal("all"))
123128
)
124129
);
125130
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ static Stream<List<Expression>> generateValidData() {
5454
dsl.namedArgument("auto_generate_synonyms_phrase_query", DSL.literal("true")),
5555
dsl.namedArgument("boost", DSL.literal("1")),
5656
dsl.namedArgument("default_operator", DSL.literal("AND")),
57+
dsl.namedArgument("default_operator", DSL.literal("and")),
5758
dsl.namedArgument("enable_position_increments", DSL.literal("true")),
5859
dsl.namedArgument("escape", DSL.literal("false")),
5960
dsl.namedArgument("fuzziness", DSL.literal("1")),
@@ -70,7 +71,12 @@ static Stream<List<Expression>> generateValidData() {
7071
dsl.namedArgument("rewrite", DSL.literal("constant_score")),
7172
dsl.namedArgument("type", DSL.literal("best_fields")),
7273
dsl.namedArgument("tie_breaker", DSL.literal("0.3")),
73-
dsl.namedArgument("time_zone", DSL.literal("Canada/Pacific"))
74+
dsl.namedArgument("time_zone", DSL.literal("Canada/Pacific")),
75+
dsl.namedArgument("ANALYZER", DSL.literal("standard")),
76+
dsl.namedArgument("ANALYZE_wildcard", DSL.literal("true")),
77+
dsl.namedArgument("Allow_Leading_wildcard", DSL.literal("true")),
78+
dsl.namedArgument("Auto_Generate_Synonyms_Phrase_Query", DSL.literal("true")),
79+
dsl.namedArgument("Boost", DSL.literal("1"))
7480
).stream().map(arg -> List.of(field, query, arg));
7581
}
7682

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class RangeQueryTest {
2020

2121
@Test
2222
void should_throw_exception_for_unsupported_comparison() {
23-
// Note that since we do switch check on enum comparison, this should'be impossible
23+
// Note that since we do switch check on enum comparison, this should be impossible
2424
assertThrows(IllegalStateException.class, () ->
2525
new RangeQuery(Comparison.BETWEEN)
2626
.doBuild("name", STRING, ExprValueUtils.stringValue("John")));

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ static Stream<List<Expression>> generateValidData() {
8181
dsl.namedArgument("query", query_value),
8282
dsl.namedArgument("flags", DSL.literal("NOT|AND"))
8383
),
84+
List.of(
85+
dsl.namedArgument("fields", fields_value),
86+
dsl.namedArgument("query", query_value),
87+
dsl.namedArgument("flags", DSL.literal("PREFIX|not|AND"))
88+
),
89+
List.of(
90+
dsl.namedArgument("fields", fields_value),
91+
dsl.namedArgument("query", query_value),
92+
dsl.namedArgument("flags", DSL.literal("not|and"))
93+
),
8494
List.of(
8595
dsl.namedArgument("fields", fields_value),
8696
dsl.namedArgument("query", query_value),
@@ -106,6 +116,11 @@ static Stream<List<Expression>> generateValidData() {
106116
dsl.namedArgument("query", query_value),
107117
dsl.namedArgument("default_operator", DSL.literal("AND"))
108118
),
119+
List.of(
120+
dsl.namedArgument("fields", fields_value),
121+
dsl.namedArgument("query", query_value),
122+
dsl.namedArgument("default_operator", DSL.literal("and"))
123+
),
109124
List.of(
110125
dsl.namedArgument("fields", fields_value),
111126
dsl.namedArgument("query", query_value),
@@ -120,6 +135,20 @@ static Stream<List<Expression>> generateValidData() {
120135
dsl.namedArgument("fields", fields_value),
121136
dsl.namedArgument("query", query_value),
122137
dsl.namedArgument("boost", DSL.literal("1"))
138+
),
139+
List.of(
140+
dsl.namedArgument("FIELDS", fields_value),
141+
dsl.namedArgument("QUERY", query_value)
142+
),
143+
List.of(
144+
dsl.namedArgument("FIELDS", fields_value),
145+
dsl.namedArgument("query", query_value),
146+
dsl.namedArgument("ANALYZE_wildcard", DSL.literal("true"))
147+
),
148+
List.of(
149+
dsl.namedArgument("fields", fields_value),
150+
dsl.namedArgument("query", query_value),
151+
dsl.namedArgument("analyZER", DSL.literal("standard"))
123152
)
124153
);
125154
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void throws_SemanticCheckException_when_wrong_argument_name() {
7272

7373
SemanticCheckException exception =
7474
assertThrows(SemanticCheckException.class, () -> query.build(expr));
75-
assertEquals("Parameter wrongArg is invalid for mock_query function.", exception.getMessage());
75+
assertEquals("Parameter wrongarg is invalid for mock_query function.", exception.getMessage());
7676
}
7777

7878
@Test

0 commit comments

Comments
 (0)