Skip to content

Commit 7da44e8

Browse files
LantaoJinLantaoJin
committed
Fix IT
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 2bf9e89 commit 7da44e8

7 files changed

Lines changed: 63 additions & 54 deletions

File tree

‎integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -553,19 +553,21 @@ public void testKeywordLikeFunctionExplain() throws IOException {
553553
expected,
554554
explainQueryYaml(
555555
"source=opensearch-sql_test_index_account | where like(firstname, '%mbe%', true)"));
556-
withSettings(
557-
Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED,
558-
"false",
559-
() -> {
560-
try {
561-
assertYamlEqualsIgnoreId(
562-
expected,
563-
explainQueryYaml(
564-
"source=opensearch-sql_test_index_account | where like(firstname, '%mbe%')"));
565-
} catch (IOException e) {
566-
throw new RuntimeException(e);
567-
}
568-
});
556+
if (isCalciteEnabled()) {
557+
withSettings(
558+
Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED,
559+
"false",
560+
() -> {
561+
try {
562+
assertYamlEqualsIgnoreId(
563+
expected,
564+
explainQueryYaml(
565+
"source=opensearch-sql_test_index_account | where like(firstname, '%mbe%')"));
566+
} catch (IOException e) {
567+
throw new RuntimeException(e);
568+
}
569+
});
570+
}
569571
}
570572

571573
@Test
@@ -575,19 +577,22 @@ public void testTextLikeFunctionExplain() throws IOException {
575577
expected,
576578
explainQueryYaml(
577579
"source=opensearch-sql_test_index_account | where like(address, '%Holmes%', true)"));
578-
withSettings(
579-
Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED,
580-
"false",
581-
() -> {
582-
try {
583-
assertYamlEqualsIgnoreId(
584-
expected,
585-
explainQueryYaml(
586-
"source=opensearch-sql_test_index_account | where like(address, '%Holmes%')"));
587-
} catch (IOException e) {
588-
throw new RuntimeException(e);
589-
}
590-
});
580+
if (isCalciteEnabled()) {
581+
withSettings(
582+
Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED,
583+
"false",
584+
() -> {
585+
try {
586+
assertYamlEqualsIgnoreId(
587+
expected,
588+
explainQueryYaml(
589+
"source=opensearch-sql_test_index_account | where like(address,"
590+
+ " '%Holmes%')"));
591+
} catch (IOException e) {
592+
throw new RuntimeException(e);
593+
}
594+
});
595+
}
591596
}
592597

593598
@Ignore("The serialized string is unstable because of function properties")

‎integ-test/src/test/resources/expectedOutput/ppl/explain_text_like_function.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ root:
66
children:
77
- name: FilterOperator
88
description:
9-
conditions: "like(address, \"%Holmes%\")"
9+
conditions: "like(address, \"%Holmes%\", true)"
1010
children:
1111
- name: OpenSearchIndexScan
1212
description:

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ protected WildcardQueryBuilder createBuilder(String field, String query) {
4545
*/
4646
@Override
4747
public boolean canSupport(FunctionExpression func) {
48-
if (func.getArguments().size() == 2
48+
if ((func.getArguments().size() == 2 || func.getArguments().size() == 3)
4949
&& (func.getArguments().get(0) instanceof ReferenceExpression)
5050
&& (func.getArguments().get(1) instanceof LiteralExpression
5151
|| literalExpressionWrappedByCast(func))) {

‎ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.WcFieldExpressionContext;
7373
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParserBaseVisitor;
7474
import org.opensearch.sql.ppl.utils.ArgumentFactory;
75+
import org.opensearch.sql.ppl.utils.UnresolvedPlanHelper;
7576
import org.opensearch.sql.utils.DateTimeUtils;
7677

7778
/** Class of building AST Expression nodes. */
@@ -160,9 +161,10 @@ public UnresolvedExpression visitCompareExpr(CompareExprContext ctx) {
160161
String operator = ctx.comparisonOperator().getText();
161162
if ("==".equals(operator)) {
162163
operator = EQUAL.getName().getFunctionName();
163-
} else if (LIKE.getName().getFunctionName().equalsIgnoreCase(operator)) {
164+
} else if (LIKE.getName().getFunctionName().equalsIgnoreCase(operator)
165+
&& UnresolvedPlanHelper.isCalciteEnabled(astBuilder.getSettings())) {
164166
operator =
165-
ArgumentFactory.legacyPreferred(astBuilder.getSettings())
167+
UnresolvedPlanHelper.legacyPreferred(astBuilder.getSettings())
166168
? ILIKE.getName().getFunctionName()
167169
: LIKE.getName().getFunctionName();
168170
} else if (ILIKE.getName().getFunctionName().equalsIgnoreCase(operator)) {

‎ppl/src/main/java/org/opensearch/sql/ppl/utils/ArgumentFactory.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ public static List<Argument> getArgumentList(
7878
getArgumentValue(ctx1.bucketNullableArg(0).bucket_nullable))
7979
: new Argument(
8080
Argument.BUCKET_NULLABLE,
81-
legacyPreferred(settings) ? Literal.TRUE : Literal.FALSE)));
81+
UnresolvedPlanHelper.legacyPreferred(settings)
82+
? Literal.TRUE
83+
: Literal.FALSE)));
8284
if (ctx2 != null) {
8385
list.add(new Argument("dedupsplit", getArgumentValue(ctx2.dedupsplit)));
8486
} else {
@@ -87,12 +89,6 @@ public static List<Argument> getArgumentList(
8789
return list;
8890
}
8991

90-
public static boolean legacyPreferred(Settings settings) {
91-
return settings == null
92-
|| settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED) == null
93-
|| Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED));
94-
}
95-
9692
/**
9793
* Get list of {@link Argument}.
9894
*
@@ -125,7 +121,7 @@ public static List<Argument> getArgumentList(EventstatsCommandContext ctx, Setti
125121
Argument.BUCKET_NULLABLE, getArgumentValue(ctx.bucketNullableArg().bucket_nullable))
126122
: new Argument(
127123
Argument.BUCKET_NULLABLE,
128-
legacyPreferred(settings) ? Literal.TRUE : Literal.FALSE));
124+
UnresolvedPlanHelper.legacyPreferred(settings) ? Literal.TRUE : Literal.FALSE));
129125
}
130126

131127
/**
@@ -278,7 +274,7 @@ public static List<Argument> getArgumentList(
278274
RareTopN.Option.useNull.name(),
279275
opt.isPresent()
280276
? getArgumentValue(opt.get().useNull)
281-
: legacyPreferred(settings) ? Literal.TRUE : Literal.FALSE));
277+
: UnresolvedPlanHelper.legacyPreferred(settings) ? Literal.TRUE : Literal.FALSE));
282278
return list;
283279
}
284280

‎ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ public String visitRareTopN(RareTopN node, String context) {
400400
String fields = visitFieldList(node.getFields());
401401
String group = visitExpressionList(node.getGroupExprList());
402402
String options =
403-
isCalciteEnabled()
403+
UnresolvedPlanHelper.isCalciteEnabled(settings)
404404
? StringUtils.format(
405405
"countield='%s' showcount=%s usenull=%s ", countField, showCount, useNull)
406406
: "";
@@ -801,18 +801,6 @@ private String groupBy(String groupBy) {
801801
return Strings.isNullOrEmpty(groupBy) ? "" : StringUtils.format("by %s", groupBy);
802802
}
803803

804-
private boolean isCalciteEnabled() {
805-
return settings == null
806-
|| settings.getSettingValue(Settings.Key.CALCITE_ENGINE_ENABLED) == null
807-
|| Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.CALCITE_ENGINE_ENABLED));
808-
}
809-
810-
private boolean legacyPreferred() {
811-
return settings == null
812-
|| settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED) == null
813-
|| Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED));
814-
}
815-
816804
/** Expression Anonymizer. */
817805
private static class AnonymizerExpressionAnalyzer extends AbstractNodeVisitor<String, String> {
818806
private final PPLQueryDataAnonymizer queryAnonymizer;
@@ -883,10 +871,15 @@ public String visitFunction(Function node, String context) {
883871
node.getFuncArgs().stream()
884872
.map(unresolvedExpression -> analyze(unresolvedExpression, context))
885873
.collect(Collectors.joining(","));
886-
if (queryAnonymizer.isCalciteEnabled()
874+
if (UnresolvedPlanHelper.isCalciteEnabled(queryAnonymizer.settings)
887875
&& node.getFuncName().equalsIgnoreCase("LIKE")
888876
&& node.getFuncArgs().size() == 2) {
889-
arguments = arguments + "," + (queryAnonymizer.legacyPreferred() ? "false" : "true");
877+
arguments =
878+
arguments
879+
+ ","
880+
+ (UnresolvedPlanHelper.legacyPreferred(queryAnonymizer.settings)
881+
? "false"
882+
: "true");
890883
}
891884
return StringUtils.format("%s(%s)", node.getFuncName(), arguments);
892885
}

‎ppl/src/main/java/org/opensearch/sql/ppl/utils/UnresolvedPlanHelper.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.opensearch.sql.ast.expression.AllFields;
1111
import org.opensearch.sql.ast.tree.Project;
1212
import org.opensearch.sql.ast.tree.UnresolvedPlan;
13+
import org.opensearch.sql.common.setting.Settings;
1314

1415
/** The helper to add select to {@link UnresolvedPlan} if needed. */
1516
@UtilityClass
@@ -23,4 +24,16 @@ public UnresolvedPlan addSelectAll(UnresolvedPlan plan) {
2324
return new Project(ImmutableList.of(AllFields.of())).attach(plan);
2425
}
2526
}
27+
28+
public static boolean legacyPreferred(Settings settings) {
29+
return settings == null
30+
|| settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED) == null
31+
|| Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED));
32+
}
33+
34+
public static boolean isCalciteEnabled(Settings settings) {
35+
return settings == null
36+
|| settings.getSettingValue(Settings.Key.CALCITE_ENGINE_ENABLED) == null
37+
|| Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.CALCITE_ENGINE_ENABLED));
38+
}
2639
}

0 commit comments

Comments
 (0)