Skip to content

Commit 7418cda

Browse files
committed
Merge OpenSearchSqlToRelConverter in CalciteToolsHelper into the same subclass for validation
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent 35ac017 commit 7418cda

5 files changed

Lines changed: 29 additions & 42 deletions

File tree

core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,11 @@
6262
import org.apache.calcite.plan.RelOptRule;
6363
import org.apache.calcite.plan.RelOptSchema;
6464
import org.apache.calcite.plan.RelOptTable;
65-
import org.apache.calcite.plan.RelOptTable.ViewExpander;
6665
import org.apache.calcite.plan.hep.HepPlanner;
6766
import org.apache.calcite.plan.hep.HepProgram;
6867
import org.apache.calcite.plan.hep.HepProgramBuilder;
6968
import org.apache.calcite.prepare.CalciteCatalogReader;
7069
import org.apache.calcite.prepare.CalcitePrepareImpl;
71-
import org.apache.calcite.prepare.Prepare.CatalogReader;
7270
import org.apache.calcite.rel.RelHomogeneousShuttle;
7371
import org.apache.calcite.rel.RelNode;
7472
import org.apache.calcite.rel.RelRoot;
@@ -88,7 +86,6 @@
8886
import org.apache.calcite.sql.SqlKind;
8987
import org.apache.calcite.sql.parser.SqlParserPos;
9088
import org.apache.calcite.sql.validate.SqlValidator;
91-
import org.apache.calcite.sql2rel.RelFieldTrimmer;
9289
import org.apache.calcite.sql2rel.SqlRexConvertletTable;
9390
import org.apache.calcite.sql2rel.SqlToRelConverter;
9491
import org.apache.calcite.tools.FrameworkConfig;
@@ -98,11 +95,11 @@
9895
import org.apache.calcite.tools.RelRunner;
9996
import org.apache.calcite.util.Holder;
10097
import org.apache.calcite.util.Util;
101-
import org.checkerframework.checker.nullness.qual.Nullable;
10298
import org.opensearch.sql.calcite.CalcitePlanContext;
10399
import org.opensearch.sql.calcite.plan.Scannable;
104100
import org.opensearch.sql.calcite.plan.rule.OpenSearchRules;
105101
import org.opensearch.sql.calcite.plan.rule.PPLSimplifyDedupRule;
102+
import org.opensearch.sql.calcite.validate.converters.OpenSearchSqlToRelConverter;
106103
import org.opensearch.sql.expression.function.PPLBuiltinOperators;
107104
import org.opensearch.sql.monitor.profile.ProfileMetric;
108105
import org.opensearch.sql.monitor.profile.QueryProfiling;
@@ -361,34 +358,6 @@ protected SqlToRelConverter getSqlToRelConverter(
361358
}
362359
}
363360

364-
public static class OpenSearchSqlToRelConverter extends SqlToRelConverter {
365-
protected final RelBuilder relBuilder;
366-
367-
public OpenSearchSqlToRelConverter(
368-
ViewExpander viewExpander,
369-
@Nullable SqlValidator validator,
370-
CatalogReader catalogReader,
371-
RelOptCluster cluster,
372-
SqlRexConvertletTable convertletTable,
373-
Config config) {
374-
super(viewExpander, validator, catalogReader, cluster, convertletTable, config);
375-
this.relBuilder =
376-
config
377-
.getRelBuilderFactory()
378-
.create(
379-
cluster,
380-
validator != null
381-
? validator.getCatalogReader().unwrap(RelOptSchema.class)
382-
: null)
383-
.transform(config.getRelBuilderConfigTransform());
384-
}
385-
386-
@Override
387-
protected RelFieldTrimmer newFieldTrimmer() {
388-
return new OpenSearchRelFieldTrimmer(validator, this.relBuilder);
389-
}
390-
}
391-
392361
public static class OpenSearchRelRunners {
393362
/**
394363
* Runs a relational expression by existing connection. This class copied from {@link

core/src/main/java/org/opensearch/sql/calcite/validate/converters/PplRelToSqlNodeConverter.java renamed to core/src/main/java/org/opensearch/sql/calcite/validate/converters/OpenSearchRelToSqlConverter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
* <p>This converter is used during the validation phase to convert RelNode back to SqlNode for
3434
* validation and type checking using Calcite's SqlValidator.
3535
*/
36-
public class PplRelToSqlNodeConverter extends RelToSqlConverter {
36+
public class OpenSearchRelToSqlConverter extends RelToSqlConverter {
3737
/**
3838
* Creates a RelToSqlConverter for PPL.
3939
*
4040
* @param dialect the SQL dialect to use for conversion
4141
*/
42-
public PplRelToSqlNodeConverter(SqlDialect dialect) {
42+
public OpenSearchRelToSqlConverter(SqlDialect dialect) {
4343
super(dialect);
4444
}
4545

core/src/main/java/org/opensearch/sql/calcite/validate/converters/PplSqlToRelConverter.java renamed to core/src/main/java/org/opensearch/sql/calcite/validate/converters/OpenSearchSqlToRelConverter.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import java.util.List;
99
import org.apache.calcite.plan.RelOptCluster;
10+
import org.apache.calcite.plan.RelOptSchema;
1011
import org.apache.calcite.plan.RelOptTable;
1112
import org.apache.calcite.prepare.Prepare;
1213
import org.apache.calcite.rel.RelNode;
@@ -17,19 +18,36 @@
1718
import org.apache.calcite.sql.SqlJoin;
1819
import org.apache.calcite.sql.SqlNode;
1920
import org.apache.calcite.sql.validate.SqlValidator;
21+
import org.apache.calcite.sql2rel.RelFieldTrimmer;
2022
import org.apache.calcite.sql2rel.SqlRexConvertletTable;
2123
import org.apache.calcite.sql2rel.SqlToRelConverter;
24+
import org.apache.calcite.tools.RelBuilder;
2225
import org.checkerframework.checker.nullness.qual.Nullable;
26+
import org.opensearch.sql.calcite.utils.OpenSearchRelFieldTrimmer;
2327

24-
public class PplSqlToRelConverter extends SqlToRelConverter {
25-
public PplSqlToRelConverter(
28+
public class OpenSearchSqlToRelConverter extends SqlToRelConverter {
29+
protected final RelBuilder relBuilder;
30+
31+
public OpenSearchSqlToRelConverter(
2632
RelOptTable.ViewExpander viewExpander,
2733
@Nullable SqlValidator validator,
2834
Prepare.CatalogReader catalogReader,
2935
RelOptCluster cluster,
3036
SqlRexConvertletTable convertletTable,
3137
Config config) {
3238
super(viewExpander, validator, catalogReader, cluster, convertletTable, config);
39+
this.relBuilder =
40+
config
41+
.getRelBuilderFactory()
42+
.create(
43+
cluster,
44+
validator != null ? validator.getCatalogReader().unwrap(RelOptSchema.class) : null)
45+
.transform(config.getRelBuilderConfigTransform());
46+
}
47+
48+
@Override
49+
protected RelFieldTrimmer newFieldTrimmer() {
50+
return new OpenSearchRelFieldTrimmer(validator, this.relBuilder);
3351
}
3452

3553
/**

core/src/main/java/org/opensearch/sql/executor/QueryService.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@
4545
import org.opensearch.sql.calcite.validate.OpenSearchSparkSqlDialect;
4646
import org.opensearch.sql.calcite.validate.PplConvertletTable;
4747
import org.opensearch.sql.calcite.validate.ValidationUtils;
48-
import org.opensearch.sql.calcite.validate.converters.PplRelToSqlNodeConverter;
49-
import org.opensearch.sql.calcite.validate.converters.PplSqlToRelConverter;
48+
import org.opensearch.sql.calcite.validate.converters.OpenSearchRelToSqlConverter;
49+
import org.opensearch.sql.calcite.validate.converters.OpenSearchSqlToRelConverter;
5050
import org.opensearch.sql.calcite.validate.shuttles.PplRelToSqlRelShuttle;
5151
import org.opensearch.sql.calcite.validate.shuttles.SkipRelValidationShuttle;
5252
import org.opensearch.sql.calcite.validate.shuttles.SqlRewriteShuttle;
@@ -300,7 +300,7 @@ private RelNode validate(RelNode relNode, CalcitePlanContext context) {
300300
RelNode sqlRelNode = relNode.accept(new PplRelToSqlRelShuttle(context.rexBuilder, true));
301301

302302
// Convert RelNode to SqlNode for validation
303-
RelToSqlConverter rel2sql = new PplRelToSqlNodeConverter(OpenSearchSparkSqlDialect.DEFAULT);
303+
RelToSqlConverter rel2sql = new OpenSearchRelToSqlConverter(OpenSearchSparkSqlDialect.DEFAULT);
304304
SqlImplementor.Result result = rel2sql.visitRoot(sqlRelNode);
305305
SqlNode root = result.asStatement();
306306

@@ -328,7 +328,7 @@ private RelNode validate(RelNode relNode, CalcitePlanContext context) {
328328
// See SqlToRelConverter.java#convertSelectImpl
329329
.withHintStrategyTable(context.relBuilder.getCluster().getHintStrategies());
330330
SqlToRelConverter sql2rel =
331-
new PplSqlToRelConverter(
331+
new OpenSearchSqlToRelConverter(
332332
context.config.getViewExpander(),
333333
validator,
334334
validator.getCatalogReader().unwrap(CalciteCatalogReader.class),

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLAbstractTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
import org.opensearch.sql.calcite.CalciteRelNodeVisitor;
4747
import org.opensearch.sql.calcite.SysLimit;
4848
import org.opensearch.sql.calcite.validate.OpenSearchSparkSqlDialect;
49-
import org.opensearch.sql.calcite.validate.converters.PplRelToSqlNodeConverter;
49+
import org.opensearch.sql.calcite.validate.converters.OpenSearchRelToSqlConverter;
5050
import org.opensearch.sql.common.setting.Settings;
5151
import org.opensearch.sql.datasource.DataSourceService;
5252
import org.opensearch.sql.exception.ExpressionEvaluationException;
@@ -66,7 +66,7 @@ public CalcitePPLAbstractTest(CalciteAssert.SchemaSpec... schemaSpecs) {
6666
this.config = config(schemaSpecs);
6767
this.dataSourceService = mock(DataSourceService.class);
6868
this.planTransformer = new CalciteRelNodeVisitor(dataSourceService);
69-
this.converter = new PplRelToSqlNodeConverter(OpenSearchSparkSqlDialect.DEFAULT);
69+
this.converter = new OpenSearchRelToSqlConverter(OpenSearchSparkSqlDialect.DEFAULT);
7070
this.settings = mock(Settings.class);
7171
}
7272

0 commit comments

Comments
 (0)