Skip to content

Commit d04916b

Browse files
GabeFernandez310GabeFernandez310
committed
Made Changes To Address PR Comments
Signed-off-by: GabeFernandez310 <[email protected]>
1 parent fdddc9b commit d04916b

File tree

5 files changed

+184
-41
lines changed

5 files changed

+184
-41
lines changed

‎core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,6 @@ private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) {
6060
return new RelevanceFunctionResolver(funcName, STRING);
6161
}
6262

63-
private static FunctionResolver matchphrasequery(BuiltinFunctionName matchPhrase) {
64-
FunctionName funcName = BuiltinFunctionName.MATCHPHRASEQUERY.getName();
65-
return new RelevanceFunctionResolver(funcName, STRING);
66-
}
67-
6863
private static FunctionResolver multi_match() {
6964
FunctionName funcName = BuiltinFunctionName.MULTI_MATCH.getName();
7065
return new RelevanceFunctionResolver(funcName, STRUCT);

‎docs/user/dql/functions.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2784,7 +2784,7 @@ It is an alternate syntax for the `match_phrase`_ function. Available parameters
27842784
- slop
27852785
- zero_terms_query
27862786

2787-
For backward compatibility, matchphrase is also supported and mapped to match_phrase query as well.
2787+
For backward compatibility, matchphrase is also supported and mapped to match_phrase query.
27882788

27892789
Example with only ``field`` and ``query`` expressions, and all other parameters are set default values::
27902790

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

Lines changed: 181 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.junit.jupiter.api.DisplayNameGenerator;
1515
import org.junit.jupiter.api.Test;
1616
import org.opensearch.sql.common.antlr.SyntaxCheckException;
17+
import org.opensearch.sql.common.grok.Match;
1718
import org.opensearch.sql.data.model.ExprValue;
1819
import org.opensearch.sql.data.type.ExprType;
1920
import org.opensearch.sql.exception.SemanticCheckException;
@@ -30,27 +31,22 @@ public class MatchPhraseQueryTest {
3031

3132
private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository());
3233
private final MatchPhraseQuery matchPhraseQuery = new MatchPhraseQuery();
33-
private final FunctionName matchPhraseName = FunctionName.of("match_phrase");
34-
//TODO: Consider also testing 'matchphrase'
35-
private final FunctionName mathPhraseQueryName = FunctionName.of("matchphrasequery");
36-
private final FunctionName[] functionNames = {matchPhraseName, mathPhraseQueryName};
34+
private final FunctionName matchPhraseName = FunctionName.of("matchphrase");
35+
private final FunctionName matchPhraseWithUnderscoreName = FunctionName.of("match_phrase");
36+
private final FunctionName matchPhraseQueryName = FunctionName.of("matchphrasequery");
3737

3838
@Test
3939
public void test_SyntaxCheckException_when_no_arguments() {
4040
List<Expression> arguments = List.of();
41-
for (FunctionName funcName: functionNames) {
42-
assertThrows(SyntaxCheckException.class,
43-
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
44-
}
41+
assertThrows(SyntaxCheckException.class,
42+
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
4543
}
4644

4745
@Test
4846
public void test_SyntaxCheckException_when_one_argument() {
4947
List<Expression> arguments = List.of(dsl.namedArgument("field", "test"));
50-
for (FunctionName funcName: functionNames) {
51-
assertThrows(SyntaxCheckException.class,
52-
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments,funcName)));
53-
}
48+
assertThrows(SyntaxCheckException.class,
49+
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
5450
}
5551

5652
@Test
@@ -59,10 +55,8 @@ public void test_SyntaxCheckException_when_invalid_parameter() {
5955
dsl.namedArgument("field", "test"),
6056
dsl.namedArgument("query", "test2"),
6157
dsl.namedArgument("unsupported", "3"));
62-
for (FunctionName funcName: functionNames) {
63-
Assertions.assertThrows(SemanticCheckException.class,
64-
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
65-
}
58+
Assertions.assertThrows(SemanticCheckException.class,
59+
() -> matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
6660
}
6761

6862
@Test
@@ -72,19 +66,15 @@ public void test_analyzer_parameter() {
7266
dsl.namedArgument("query", "t2"),
7367
dsl.namedArgument("analyzer", "standard")
7468
);
75-
for (FunctionName funcName: functionNames) {
76-
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
77-
}
69+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
7870
}
7971

8072
@Test
8173
public void build_succeeds_with_two_arguments() {
8274
List<Expression> arguments = List.of(
8375
dsl.namedArgument("field", "test"),
8476
dsl.namedArgument("query", "test2"));
85-
for (FunctionName funcName: functionNames) {
86-
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
87-
}
77+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
8878
}
8979

9080
@Test
@@ -94,9 +84,7 @@ public void test_slop_parameter() {
9484
dsl.namedArgument("query", "t2"),
9585
dsl.namedArgument("slop", "2")
9686
);
97-
for (FunctionName funcName: functionNames) {
98-
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
99-
}
87+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
10088
}
10189

10290
@Test
@@ -106,9 +94,7 @@ public void test_zero_terms_query_parameter() {
10694
dsl.namedArgument("query", "t2"),
10795
dsl.namedArgument("zero_terms_query", "ALL")
10896
);
109-
for (FunctionName funcName: functionNames) {
110-
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
111-
}
97+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
11298
}
11399

114100
@Test
@@ -118,12 +104,176 @@ public void test_zero_terms_query_parameter_lower_case() {
118104
dsl.namedArgument("query", "t2"),
119105
dsl.namedArgument("zero_terms_query", "all")
120106
);
121-
for (FunctionName funcName: functionNames) {
122-
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments, funcName)));
123-
}
107+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments)));
108+
}
109+
110+
@Test
111+
public void test_SyntaxCheckException_when_no_arguments_match_phrase_syntax() {
112+
List<Expression> arguments = List.of();
113+
assertThrows(SyntaxCheckException.class,
114+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
115+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
116+
}
117+
118+
@Test
119+
public void test_SyntaxCheckException_when_one_argument_match_phrase_syntax() {
120+
List<Expression> arguments = List.of(dsl.namedArgument("field", "test"));
121+
assertThrows(SyntaxCheckException.class,
122+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
123+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
124+
125+
}
126+
127+
@Test
128+
public void test_SyntaxCheckException_when_invalid_parameter_match_phrase_syntax() {
129+
List<Expression> arguments = List.of(
130+
dsl.namedArgument("field", "test"),
131+
dsl.namedArgument("query", "test2"),
132+
dsl.namedArgument("unsupported", "3"));
133+
Assertions.assertThrows(SemanticCheckException.class,
134+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
135+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
136+
}
137+
138+
@Test
139+
public void test_analyzer_parameter_match_phrase_syntax() {
140+
List<Expression> arguments = List.of(
141+
dsl.namedArgument("field", "t1"),
142+
dsl.namedArgument("query", "t2"),
143+
dsl.namedArgument("analyzer", "standard")
144+
);
145+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
146+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
147+
}
148+
149+
@Test
150+
public void build_succeeds_with_two_arguments_match_phrase_syntax() {
151+
List<Expression> arguments = List.of(
152+
dsl.namedArgument("field", "test"),
153+
dsl.namedArgument("query", "test2"));
154+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
155+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
156+
}
157+
158+
@Test
159+
public void test_slop_parameter_match_phrase_syntax() {
160+
List<Expression> arguments = List.of(
161+
dsl.namedArgument("field", "t1"),
162+
dsl.namedArgument("query", "t2"),
163+
dsl.namedArgument("slop", "2")
164+
);
165+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
166+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
167+
}
168+
169+
@Test
170+
public void test_zero_terms_query_parameter_match_phrase_syntax() {
171+
List<Expression> arguments = List.of(
172+
dsl.namedArgument("field", "t1"),
173+
dsl.namedArgument("query", "t2"),
174+
dsl.namedArgument("zero_terms_query", "ALL")
175+
);
176+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
177+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
178+
}
179+
180+
@Test
181+
public void test_zero_terms_query_parameter_lower_case_match_phrase_syntax() {
182+
List<Expression> arguments = List.of(
183+
dsl.namedArgument("field", "t1"),
184+
dsl.namedArgument("query", "t2"),
185+
dsl.namedArgument("zero_terms_query", "all")
186+
);
187+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
188+
arguments, MatchPhraseQueryTest.this.matchPhraseWithUnderscoreName)));
189+
}
190+
191+
@Test
192+
public void test_SyntaxCheckException_when_no_arguments_matchphrase_syntax() {
193+
List<Expression> arguments = List.of();
194+
assertThrows(SyntaxCheckException.class,
195+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
196+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
197+
}
198+
199+
@Test
200+
public void test_SyntaxCheckException_when_one_argument_matchphrase_syntax() {
201+
List<Expression> arguments = List.of(dsl.namedArgument("field", "test"));
202+
assertThrows(SyntaxCheckException.class,
203+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
204+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
205+
206+
}
207+
208+
@Test
209+
public void test_SyntaxCheckException_when_invalid_parameter_matchphrase_syntax() {
210+
List<Expression> arguments = List.of(
211+
dsl.namedArgument("field", "test"),
212+
dsl.namedArgument("query", "test2"),
213+
dsl.namedArgument("unsupported", "3"));
214+
Assertions.assertThrows(SemanticCheckException.class,
215+
() -> matchPhraseQuery.build(new MatchPhraseExpression(
216+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
217+
}
218+
219+
@Test
220+
public void test_analyzer_parameter_matchphrase_syntax() {
221+
List<Expression> arguments = List.of(
222+
dsl.namedArgument("field", "t1"),
223+
dsl.namedArgument("query", "t2"),
224+
dsl.namedArgument("analyzer", "standard")
225+
);
226+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
227+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
228+
}
229+
230+
@Test
231+
public void build_succeeds_with_two_arguments_matchphrase_syntax() {
232+
List<Expression> arguments = List.of(
233+
dsl.namedArgument("field", "test"),
234+
dsl.namedArgument("query", "test2"));
235+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
236+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
237+
}
238+
239+
@Test
240+
public void test_slop_parameter_matchphrase_syntax() {
241+
List<Expression> arguments = List.of(
242+
dsl.namedArgument("field", "t1"),
243+
dsl.namedArgument("query", "t2"),
244+
dsl.namedArgument("slop", "2")
245+
);
246+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
247+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
248+
}
249+
250+
@Test
251+
public void test_zero_terms_query_parameter_matchphrase_syntax() {
252+
List<Expression> arguments = List.of(
253+
dsl.namedArgument("field", "t1"),
254+
dsl.namedArgument("query", "t2"),
255+
dsl.namedArgument("zero_terms_query", "ALL")
256+
);
257+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
258+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
259+
}
260+
261+
@Test
262+
public void test_zero_terms_query_parameter_lower_case_matchphrase_syntax() {
263+
List<Expression> arguments = List.of(
264+
dsl.namedArgument("field", "t1"),
265+
dsl.namedArgument("query", "t2"),
266+
dsl.namedArgument("zero_terms_query", "all")
267+
);
268+
Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(
269+
arguments, MatchPhraseQueryTest.this.matchPhraseQueryName)));
124270
}
125271

126272
private class MatchPhraseExpression extends FunctionExpression {
273+
public MatchPhraseExpression(List<Expression> arguments) {
274+
super(MatchPhraseQueryTest.this.matchPhraseName, arguments);
275+
}
276+
127277
public MatchPhraseExpression(List<Expression> arguments, FunctionName funcName) {
128278
super(funcName, arguments);
129279
}

‎sql/src/main/antlr/OpenSearchSQLParser.g4

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,8 @@ systemFunctionName
425425
;
426426

427427
singleFieldRelevanceFunctionName
428-
: MATCH | MATCH_PHRASE | MATCHPHRASE
429-
| MATCHPHRASEQUERY | MATCH_BOOL_PREFIX
430-
| MATCH_PHRASE_PREFIX
428+
: MATCH | MATCH_PHRASE | MATCHPHRASE| MATCHPHRASEQUERY
429+
| MATCH_BOOL_PREFIX | MATCH_PHRASE_PREFIX
431430
;
432431

433432
multiFieldRelevanceFunctionName

‎sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,6 @@ public void filteredDistinctCount() {
451451
);
452452
}
453453

454-
//TODO: Verify what this test does
455454
@Test
456455
public void matchPhraseQueryAllParameters() {
457456
assertEquals(

0 commit comments

Comments
 (0)