Skip to content

Conversation

@LantaoJin
Copy link
Member

@LantaoJin LantaoJin commented Feb 21, 2025

Description

Build integration test framework, includes

  1. Add two kinds of integration tests: standalone and remote. standalone IT can debug in IDE, remote IT run tests in the min OS cluster.
  2. Add two configs plugins.calcite.enabled and plugins.calcite.fallback.allowed
  3. Add CalciteToolsHelper.java, this class is used to create customized Connection, JavaTypeFactory, RelBuilder and RelRunner
  4. Add a method valueForCalcite() in ExprValue, calcite read opensearch data by valueForCalcite() instead of value()
  5. Add EnumerableIndexScanRule to convert a CalciteLogicalTableScan to CalciteOpenSearchIndexScan`

Related Issues

Resolves #3330

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: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin LantaoJin added the calcite calcite migration releated label Feb 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
Copy link
Member Author

Current CI will still fail due to missing security policy in job-schedule plugin. cc @xinyuan

Review required cc @qianheng-aws @penghuo @dai-chen

fallbackAllowed = settings.getSettingValue(Settings.Key.CALCITE_FALLBACK_ALLOWED);
}
if (!fallbackAllowed) {
throw e;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use listener to handle this exception?

listener.onFailure(e);

* This abstract test case provide a standalone env to run PPL query, IT extends this class could
* debug the service side execution of PPL in IDE.
*/
public abstract class CalcitePPLTestCase extends PPLIntegTestCase {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious why do we change this name? The current name doesn't explicitly indicate that this's an integ test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

@@ -0,0 +1,185 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add calcite's license header after ours since we copy code from that repo.

There is similar example in

Signed-off-by: Lantao Jin <ltjin@amazon.com>
…estCase

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin LantaoJin merged commit 4ba78d3 into opensearch-project:feature/calcite-engine Feb 26, 2025
8 of 13 checks passed
@LantaoJin
Copy link
Member Author

Thanks, merging to dev branch.

penghuo pushed a commit to penghuo/os-sql that referenced this pull request Mar 12, 2025
* Build integration test framework

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* make local work

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* Fix the timestamp issue

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* address comments

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix java style and rename CalcitePPLTestCase back to CalcitePPLIntegTestCase

Signed-off-by: Lantao Jin <ltjin@amazon.com>

---------

Signed-off-by: Lantao Jin <ltjin@amazon.com>
penghuo pushed a commit that referenced this pull request Jun 16, 2025
* Build integration test framework

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* make local work

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* Fix the timestamp issue

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* address comments

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix java style and rename CalcitePPLTestCase back to CalcitePPLIntegTestCase

Signed-off-by: Lantao Jin <ltjin@amazon.com>

---------

Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: xinyual <xinyual@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

calcite calcite migration releated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants