[Calcite Engine] Support In expression #3429

qianheng-aws · 2025-03-14T07:01:25Z

Description

Support In expression, also add data anonymizer for it.

./gradlew :integ-test:integTest --tests '*Calcite*IT succeed locally.

../../../ppl/src/test/java/org/opensearch/sql/ppl/calcite/* UT succeed locally.

Related Issues

Resolves #3420

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws · 2025-03-14T07:15:57Z

core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java

@@ -139,6 +140,16 @@ public RexNode visitNot(Not node, CalcitePlanContext context) {
    return context.relBuilder.not(expr);
  }

+  @Override
+  public RexNode visitIn(In node, CalcitePlanContext context) {


Calcite will optimize In expression to equal comparison if there is only 1 element in value list. We don't need to do such optimization ourself here like v2.

We don't need to do such optimization ourself here like v2.

Do u mean we push down IN to OpenSearch as terms query, so Calcite optimized is not needed?

LantaoJin · 2025-03-14T07:16:27Z

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteWhereCommandIT.java

+  @Ignore("https://github.com/opensearch-project/sql/issues/3428")
+  @Override
+  public void testIsNotNullFunction() throws IOException {}
+  ;


remove this ;

Signed-off-by: Heng Qian <[email protected]>

LantaoJin · 2025-03-14T14:35:03Z

ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java

@@ -154,6 +154,11 @@ public void testNotExpression() {
    assertEquals("source=t | where not a = ***", anonymize("source=t | where not a=1 "));
  }

+  @Test
+  public void testInExpression() {
+    assertEquals("source=t | where a in (***)", anonymize("source=t | where a in (1, 2, 3) "));


add a test for

where a not in (1, 2, 3)

"not in" is not supported in our g4, which mean v2 engine don't support it either. Need to add that implementation for v2 engine as well.

LantaoJin · 2025-03-14T14:58:43Z

integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java

+    JSONObject result =
+        executeQuery(
+            String.format(
+                "source=%s | where firstname in ('Amber') | fields firstname", TEST_INDEX_ACCOUNT));


need a test for

where age in (36, 36.0, '36')

Signed-off-by: Heng Qian <[email protected]>

…ine' into feature/calcite-engine-in

…ine' into feature/calcite-engine-in # Conflicts: # ppl/src/main/antlr/OpenSearchPPLParser.g4 # ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java

Signed-off-by: Heng Qian <[email protected]>

penghuo · 2025-03-17T22:00:40Z

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteWhereCommandIT.java

 public class CalciteWhereCommandIT extends WhereCommandIT {
  @Override
  public void init() throws IOException {
    enableCalcite();
    disallowCalciteFallback();
    super.init();
  }
+
+  @Ignore("https://github.com/opensearch-project/sql/issues/3428")


#3428 is LIKE function, why releated to testIsNotNull?

this test case contains a like function as well.

"source=%s | where isnotnull(age) and like(firstname, 'Ambe_') | fields firstname"

WhereCommandIT is an existed ppl IT which is extended by CalciteWhereCommandIT

penghuo · 2025-03-17T22:08:26Z

core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java

@@ -139,6 +140,16 @@ public RexNode visitNot(Not node, CalcitePlanContext context) {
    return context.relBuilder.not(expr);
  }

+  @Override
+  public RexNode visitIn(In node, CalcitePlanContext context) {


We don't need to do such optimization ourself here like v2.

Do u mean we push down IN to OpenSearch as terms query, so Calcite optimized is not needed?

penghuo · 2025-03-17T22:12:32Z

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteWhereCommandIT.java

+  @Override
+  protected String getIncompatibleTypeErrMsg() {
+    return "In expression types are incompatible: fields type BIGINT, values type [INTEGER,"
+        + " INTEGER, CHAR(4)]";


CHAR(4) is not a valid OpenSearch PPL datatype.

No, I mean v2 has optimization to transform column in (a) to column = a when converting. Since calcite will do such optimization in its optimization phase, we just need to convert it to the former expression directly without too much consideration.

penghuo · 2025-03-17T22:15:51Z

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteWhereCommandIT.java

@@ -9,12 +9,25 @@
 import org.junit.Ignore;
 import org.opensearch.sql.ppl.WhereCommandIT;

-@Ignore("Not all boolean functions are supported in Calcite now")
 public class CalciteWhereCommandIT extends WhereCommandIT {
  @Override
  public void init() throws IOException {
    enableCalcite();


should we disableCalcite also? otherwise it may impact other tests?

We have exclude 'org/opensearch/sql/calcite/remote/**' in build.gradle, it won't really run in CI. The parent class WhereCommandIT is the suite for test cases without calcite enabled.

Signed-off-by: Heng Qian <[email protected]>

[Calcite Engine] Support In expression

8c17ba0

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws requested review from ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, mengweieric, Swiddis, penghuo, seankao-az, MaxKsyunz, Yury-Fridlyand, anirudha, forestmvey, acarbonetto, GumpacG, ykmr1224, LantaoJin and noCharger as code owners March 14, 2025 07:01

qianheng-aws commented Mar 14, 2025

View reviewed changes

LantaoJin reviewed Mar 14, 2025

View reviewed changes

Address comments

4e00944

Signed-off-by: Heng Qian <[email protected]>

LantaoJin added the calcite calcite migration releated label Mar 14, 2025

LantaoJin reviewed Mar 14, 2025

View reviewed changes

qianheng-aws added 4 commits March 17, 2025 16:28

Support NOT IN

7ef4897

Signed-off-by: Heng Qian <[email protected]>

Merge remote-tracking branch 'refs/remotes/origin/feature/calcite-eng…

67a65af

…ine' into feature/calcite-engine-in

Merge remote-tracking branch 'refs/remotes/origin/feature/calcite-eng…

24c5a9e

…ine' into feature/calcite-engine-in # Conflicts: # ppl/src/main/antlr/OpenSearchPPLParser.g4 # ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java

Address comments

1bfa823

Signed-off-by: Heng Qian <[email protected]>

Add more UT

e480134

Signed-off-by: Heng Qian <[email protected]>

penghuo reviewed Mar 17, 2025

View reviewed changes

Address comments

1714f78

Signed-off-by: Heng Qian <[email protected]>

LantaoJin approved these changes Mar 18, 2025

View reviewed changes

LantaoJin merged commit 29c7625 into opensearch-project:feature/calcite-engine Mar 18, 2025
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Calcite Engine] Support In expression #3429

[Calcite Engine] Support In expression #3429

qianheng-aws commented Mar 14, 2025 •

edited by LantaoJin

Loading

qianheng-aws Mar 14, 2025

penghuo Mar 17, 2025

LantaoJin Mar 14, 2025

LantaoJin Mar 14, 2025

qianheng-aws Mar 17, 2025 •

edited

Loading

LantaoJin Mar 14, 2025

penghuo Mar 17, 2025

qianheng-aws Mar 18, 2025

LantaoJin Mar 18, 2025

penghuo Mar 17, 2025

penghuo Mar 17, 2025

qianheng-aws Mar 18, 2025 •

edited

Loading

penghuo Mar 17, 2025

qianheng-aws Mar 18, 2025 •

edited

Loading

[Calcite Engine] Support In expression #3429

[Calcite Engine] Support In expression #3429

Conversation

qianheng-aws commented Mar 14, 2025 • edited by LantaoJin Loading

Description

Related Issues

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qianheng-aws Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qianheng-aws Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qianheng-aws Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

qianheng-aws commented Mar 14, 2025 •

edited by LantaoJin

Loading

qianheng-aws Mar 17, 2025 •

edited

Loading

qianheng-aws Mar 18, 2025 •

edited

Loading

qianheng-aws Mar 18, 2025 •

edited

Loading