[POC] Make PPL execute successfully on Calcite engine #3258

qianheng-aws · 2025-01-21T10:42:58Z

Description

Before this PR, PPL plan will fail executing on calcite engine and fallback to the original v2 engine. This PR aims to make the plan execute successfully on calcite engine.

The changes includes:

Replace RelRunners with CalciteConnection to do execution
Add OpenSearchSchema to the rootSchema of above connection
Change ExprValue to Object in OpenSearchTable

Related Issues

Resolves #3307

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.

noCharger

@qianheng-aws is this PR plan for review?

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws · 2025-01-24T08:18:02Z

@qianheng-aws is this PR plan for review?

Yeah, but it should only be merged to the feature branch calcite-engine. It requires the review from @LantaoJin as he initiated that feature branch.

penghuo

This PR ensure CalciteStandaloneIT passes?

Could u refine the PR description? Make Calcite execute is generic.
Could u enable CalciteStandaloneIT, and disable other PPL ITs. we expected IT should passed.

penghuo · 2025-01-29T22:24:55Z

opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java

      RelNode rel, CalcitePlanContext context, ResponseListener<QueryResponse> listener) {
-    try (PreparedStatement statement = RelRunners.run(rel)) {
+    Connection connection = context.connection;
+    final RelShuttle shuttle = new RelHomogeneousShuttle();


does it required?

penghuo · 2025-01-29T22:27:13Z

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

+    @Override
+    public Table get(Object key) {
+      if (!super.containsKey(key)) {
+        registerTable((String) key);


could we do registerTable(new QualifiedName((String)key)) instead?

penghuo · 2025-01-29T22:28:44Z

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

+    try {
+      this.statement = connection.createStatement().unwrap(CalciteServerStatement.class);
+    } catch (Exception e) {
+      throw new RuntimeException("create statement failed", e);
+    }
+    this.prepare = new CalcitePrepareImpl();
+    this.relBuilder = prepare.perform(statement, config,
+        (cluster, relOptSchema, rootSchema, statement) -> new OSRelBuilder(config.getContext(),
+            cluster, relOptSchema));


could u help explain the purpose this statement?

It aims to construct a relBuilder with the same logic as RelBuilder.create(config) while reusing the same statement created by connection.

I've removed this change because I realized that I just need to reuse the schema of connection and I've set that schema into the config by my second commit.

…ble when visitRelation. Signed-off-by: Heng Qian <qianheng@amazon.com>

Signed-off-by: Heng Qian <qianheng@amazon.com>

LantaoJin

LGTM except a less comments

LantaoJin · 2025-02-06T08:10:54Z

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

+    this.connection = connection;
+    try {
+      this.statement = connection.createStatement().unwrap(CalciteServerStatement.class);
+    } catch (Exception e) {


minor: this try-catch is for unwrap or createStatement failure, better to separate the NPE out, or else mass of UT will go through via NPE here.

if (connection != null) { // try-catch block }

LantaoJin · 2025-02-06T08:15:19Z

core/src/main/java/org/opensearch/sql/calcite/plan/OpenSearchTable.java

  }

-  public abstract Enumerable<ExprValue> search();
+  public abstract Enumerable<Object> search();


What's reason to replace ExprValue?

It will throw exception and says that ExprValue is loaded in app classloader while Object is loaded in bootstrap classloader.

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

Signed-off-by: Heng Qian <qianheng@amazon.com>

…ject#3258) * [POC] Make Calcite execute successfully Signed-off-by: Heng Qian <qianheng@amazon.com> * [POC] Change caching schema to simple schema and avoid registering table when visitRelation. Signed-off-by: Heng Qian <qianheng@amazon.com> * spotlessApply Signed-off-by: Heng Qian <qianheng@amazon.com> * address comments Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com>

* [POC] Make Calcite execute successfully Signed-off-by: Heng Qian <qianheng@amazon.com> * [POC] Change caching schema to simple schema and avoid registering table when visitRelation. Signed-off-by: Heng Qian <qianheng@amazon.com> * spotlessApply Signed-off-by: Heng Qian <qianheng@amazon.com> * address comments Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> Signed-off-by: xinyual <xinyual@amazon.com>

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

noCharger reviewed Jan 21, 2025

View reviewed changes

[POC] Make Calcite execute successfully

01c93d7

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws force-pushed the feature/calcite-engine branch from 148b79c to cf34fab Compare January 24, 2025 08:13

penghuo added the calcite calcite migration releated label Jan 28, 2025

penghuo reviewed Jan 29, 2025

View reviewed changes

qianheng-aws changed the title ~~[POC] Make Calcite execute successfully~~ [POC] Make PPL execute successfully on Calcite engine Feb 5, 2025

qianheng-aws added 2 commits February 5, 2025 16:25

[POC] Change caching schema to simple schema and avoid registering ta…

27714e0

…ble when visitRelation. Signed-off-by: Heng Qian <qianheng@amazon.com>

spotlessApply

8ad5251

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws force-pushed the feature/calcite-engine branch from cf34fab to 8ad5251 Compare February 5, 2025 08:25

LantaoJin reviewed Feb 6, 2025

View reviewed changes

address comments

876a03f

Signed-off-by: Heng Qian <qianheng@amazon.com>

LantaoJin merged commit 1314a31 into opensearch-project:feature/calcite-engine Feb 7, 2025
5 of 13 checks passed

LantaoJin mentioned this pull request Feb 7, 2025

[FEATURE] Calcite Engine Framework: add CalciteConnection and wrapped RelRunners #3307

Closed

Uh oh!

[POC] Make PPL execute successfully on Calcite engine #3258

[POC] Make PPL execute successfully on Calcite engine #3258

Uh oh!

Conversation

qianheng-aws commented Jan 21, 2025 • edited by LantaoJin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

noCharger left a comment

Choose a reason for hiding this comment

Uh oh!

qianheng-aws commented Jan 24, 2025

Uh oh!

penghuo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penghuo Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

penghuo Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

penghuo Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin left a comment

Choose a reason for hiding this comment

Uh oh!

LantaoJin Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qianheng-aws commented Jan 21, 2025 •

edited by LantaoJin

Loading

penghuo left a comment •

edited

Loading