From f70b8e2cdb1502b59ea5ccb59e60a8a372ca8f7f Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 13:43:07 -0700 Subject: [PATCH 1/6] Extract getOnlyForCalciteException Signed-off-by: Tomoyuki Morita --- .../org/opensearch/sql/analysis/Analyzer.java | 27 +++++++------------ .../sql/analysis/ExpressionAnalyzer.java | 11 +++----- .../sql/calcite/utils/CalciteUtils.java | 19 +++++++++++++ .../opensearch/sql/ppl/parser/AstBuilder.java | 11 +++----- 4 files changed, 36 insertions(+), 32 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/calcite/utils/CalciteUtils.java diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 805c17b6af8..ef681ef2009 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -10,7 +10,7 @@ import static org.opensearch.sql.ast.tree.Sort.NullOrder.NULL_LAST; import static org.opensearch.sql.ast.tree.Sort.SortOrder.ASC; import static org.opensearch.sql.ast.tree.Sort.SortOrder.DESC; -import static org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED; +import static org.opensearch.sql.calcite.utils.CalciteUtils.getOnlyForCalciteException; import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; import static org.opensearch.sql.data.type.ExprCoreType.TIME; @@ -181,8 +181,7 @@ public LogicalPlan visitSubqueryAlias(SubqueryAlias node, AnalysisContext contex STRUCT); return child; } else { - throw new UnsupportedOperationException( - "Subsearch is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Subsearch"); } } @@ -672,14 +671,12 @@ public LogicalPlan visitML(ML node, AnalysisContext context) { @Override public LogicalPlan visitBin(Bin node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Bin command is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Bin"); } @Override public LogicalPlan visitExpand(Expand expand, AnalysisContext context) { - throw new UnsupportedOperationException( - "Expand is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Expand"); } /** Build {@link LogicalTrendline} for Trendline command. */ @@ -733,14 +730,12 @@ public LogicalPlan visitTrendline(Trendline node, AnalysisContext context) { @Override public LogicalPlan visitFlatten(Flatten node, AnalysisContext context) { - throw new UnsupportedOperationException( - "FLATTEN is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("FLATTEN"); } @Override public LogicalPlan visitReverse(Reverse node, AnalysisContext context) { - throw new UnsupportedOperationException( - "REVERSE is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("REVERSE"); } @Override @@ -763,22 +758,20 @@ public LogicalPlan visitCloseCursor(CloseCursor closeCursor, AnalysisContext con @Override public LogicalPlan visitJoin(Join node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Join is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Join"); } @Override public LogicalPlan visitLookup(Lookup node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Lookup is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Lookup"); } @Override public LogicalPlan visitAppendCol(AppendCol node, AnalysisContext context) { - throw new UnsupportedOperationException( - "AppendCol is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("AppendCol"); } + private LogicalSort buildSort( LogicalPlan child, AnalysisContext context, Integer count, List sortFields) { ExpressionReferenceOptimizer optimizer = diff --git a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java index f276ee774e3..d002e3672d0 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java @@ -7,7 +7,7 @@ import static org.opensearch.sql.ast.dsl.AstDSL.and; import static org.opensearch.sql.ast.dsl.AstDSL.compare; -import static org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED; +import static org.opensearch.sql.calcite.utils.CalciteUtils.getOnlyForCalciteException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -435,20 +435,17 @@ public Expression visitArgument(Argument node, AnalysisContext context) { @Override public Expression visitScalarSubquery(ScalarSubquery node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Subsearch is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Subsearch"); } @Override public Expression visitExistsSubquery(ExistsSubquery node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Subsearch is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Subsearch"); } @Override public Expression visitInSubquery(InSubquery node, AnalysisContext context) { - throw new UnsupportedOperationException( - "Subsearch is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + throw getOnlyForCalciteException("Subsearch"); } /** diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteUtils.java b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteUtils.java new file mode 100644 index 00000000000..e995d7efd52 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/CalciteUtils.java @@ -0,0 +1,19 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite.utils; + +import static org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED; + +import lombok.experimental.UtilityClass; + +@UtilityClass +public class CalciteUtils { + + public static UnsupportedOperationException getOnlyForCalciteException(String feature) { + return new UnsupportedOperationException( + feature + " is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true"); + } +} diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index beb84b783e9..6c1f906c345 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -9,6 +9,7 @@ import static java.util.Collections.emptyMap; import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; +import static org.opensearch.sql.calcite.utils.CalciteUtils.getOnlyForCalciteException; import static org.opensearch.sql.lang.PPLLangSpec.PPL_SPEC; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BinCommandContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DedupCommandContext; @@ -294,10 +295,7 @@ public UnresolvedPlan visitTableCommand(TableCommandContext ctx) { : new Argument("exclude", new Literal(false, DataType.BOOLEAN))); return buildProjectCommand(ctx.fieldsCommandBody(), arguments); } - throw new UnsupportedOperationException( - "Table command is supported only when " - + Key.CALCITE_ENGINE_ENABLED.getKeyValue() - + "=true"); + throw getOnlyForCalciteException("Table command"); } private UnresolvedPlan buildProjectCommand( @@ -308,10 +306,7 @@ private UnresolvedPlan buildProjectCommand( if (settings != null && Boolean.FALSE.equals(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED))) { if (hasEnhancedFieldFeatures(bodyCtx, fields)) { - throw new UnsupportedOperationException( - "Enhanced fields features are supported only when " - + Key.CALCITE_ENGINE_ENABLED.getKeyValue() - + "=true"); + throw getOnlyForCalciteException("Enhanced fields features"); } } From 7fd6bb0c52c243f186cc29bbbc6afe5d255a43ef Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 13:45:45 -0700 Subject: [PATCH 2/6] Minor fix Signed-off-by: Tomoyuki Morita --- .../src/main/java/org/opensearch/sql/analysis/Analyzer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index ef681ef2009..dd42ad54547 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -730,12 +730,12 @@ public LogicalPlan visitTrendline(Trendline node, AnalysisContext context) { @Override public LogicalPlan visitFlatten(Flatten node, AnalysisContext context) { - throw getOnlyForCalciteException("FLATTEN"); + throw getOnlyForCalciteException("Flatten"); } @Override public LogicalPlan visitReverse(Reverse node, AnalysisContext context) { - throw getOnlyForCalciteException("REVERSE"); + throw getOnlyForCalciteException("Reverse"); } @Override @@ -768,7 +768,7 @@ public LogicalPlan visitLookup(Lookup node, AnalysisContext context) { @Override public LogicalPlan visitAppendCol(AppendCol node, AnalysisContext context) { - throw getOnlyForCalciteException("AppendCol"); + throw getOnlyForCalciteException("Appendcol"); } From aac293ceb47fed798f88de9f137bafa2c589e3c6 Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 13:48:40 -0700 Subject: [PATCH 3/6] reformat Signed-off-by: Tomoyuki Morita --- core/src/main/java/org/opensearch/sql/analysis/Analyzer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index dd42ad54547..e25b38531aa 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -771,7 +771,6 @@ public LogicalPlan visitAppendCol(AppendCol node, AnalysisContext context) { throw getOnlyForCalciteException("Appendcol"); } - private LogicalSort buildSort( LogicalPlan child, AnalysisContext context, Integer count, List sortFields) { ExpressionReferenceOptimizer optimizer = From 9c49d84c689379446ea09796299e25787358e8ec Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 15:10:29 -0700 Subject: [PATCH 4/6] Refactoring Signed-off-by: Tomoyuki Morita --- .../opensearch/sql/ppl/FieldsCommandIT.java | 51 +++++-------------- .../opensearch/sql/ppl/parser/AstBuilder.java | 2 +- 2 files changed, 14 insertions(+), 39 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java index f83fe83db3b..a5fee710409 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java @@ -139,45 +139,20 @@ public void testMergedObjectFields() throws IOException { } @Test - public void testEnhancedFieldFeaturesBlockedWhenCalciteDisabled() { - // Test wildcards are blocked - Exception e1 = - assertThrows( - Exception.class, - () -> executeQuery(String.format("source=%s | fields *", TEST_INDEX_ACCOUNT))); - verifyErrorMessageContains( - e1, "Enhanced fields features are supported only when" + " plugins.calcite.enabled=true"); - - Exception e2 = - assertThrows( - Exception.class, - () -> executeQuery(String.format("source=%s | fields account_*", TEST_INDEX_ACCOUNT))); - verifyErrorMessageContains( - e2, "Enhanced fields features are supported only when" + " plugins.calcite.enabled=true"); - - // Test space-delimited fields are blocked - Exception e3 = - assertThrows( - Exception.class, - () -> - executeQuery( - String.format( - "source=%s | fields account_number balance firstname", - TEST_INDEX_ACCOUNT))); - verifyErrorMessageContains( - e3, "Enhanced fields features are supported only when" + " plugins.calcite.enabled=true"); + public void testEnhancedFieldsWhenCalciteDisabled() { + verifyQueryThrowsCalciteError("source=%s | fields *"); + verifyQueryThrowsCalciteError("source=%s | fields account_*"); + verifyQueryThrowsCalciteError("source=%s | fields account_number balance firstname"); + verifyQueryThrowsCalciteError("source=%s | fields account_number balance, firstname"); + } - // Test mixed delimiters are blocked - Exception e4 = - assertThrows( - Exception.class, - () -> - executeQuery( - String.format( - "source=%s | fields account_number balance, firstname", - TEST_INDEX_ACCOUNT))); - verifyErrorMessageContains( - e4, "Enhanced fields features are supported only when" + " plugins.calcite.enabled=true"); + private void verifyQueryThrowsCalciteError(String query) { + String expectedErrorMessage = + "Enhanced fields feature is supported only when plugins.calcite.enabled=true"; + assertThrows( + expectedErrorMessage, + UnsupportedOperationException.class, + () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); } @Ignore( diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index 6c1f906c345..1984077b87f 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -306,7 +306,7 @@ private UnresolvedPlan buildProjectCommand( if (settings != null && Boolean.FALSE.equals(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED))) { if (hasEnhancedFieldFeatures(bodyCtx, fields)) { - throw getOnlyForCalciteException("Enhanced fields features"); + throw getOnlyForCalciteException("Enhanced fields feature"); } } From de4af1e8ec639239212a26b4b9423c80abd5b0fb Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 15:22:17 -0700 Subject: [PATCH 5/6] Fix test Signed-off-by: Tomoyuki Morita --- .../test/java/org/opensearch/sql/ppl/FieldsCommandIT.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java index a5fee710409..ff4bdc4f158 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java @@ -147,12 +147,8 @@ public void testEnhancedFieldsWhenCalciteDisabled() { } private void verifyQueryThrowsCalciteError(String query) { - String expectedErrorMessage = - "Enhanced fields feature is supported only when plugins.calcite.enabled=true"; - assertThrows( - expectedErrorMessage, - UnsupportedOperationException.class, - () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); + Exception e = assertThrows(Exception.class, () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); + verifyErrorMessageContains(e, "Enhanced fields feature is supported only when plugins.calcite.enabled=true"); } @Ignore( From 333fb85590293b4ca6ae937caa996bf337052646 Mon Sep 17 00:00:00 2001 From: Tomoyuki Morita Date: Wed, 27 Aug 2025 23:26:14 -0700 Subject: [PATCH 6/6] reformat Signed-off-by: Tomoyuki Morita --- .../test/java/org/opensearch/sql/ppl/FieldsCommandIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java index ff4bdc4f158..3f078fe6512 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java @@ -147,8 +147,10 @@ public void testEnhancedFieldsWhenCalciteDisabled() { } private void verifyQueryThrowsCalciteError(String query) { - Exception e = assertThrows(Exception.class, () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); - verifyErrorMessageContains(e, "Enhanced fields feature is supported only when plugins.calcite.enabled=true"); + Exception e = + assertThrows(Exception.class, () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); + verifyErrorMessageContains( + e, "Enhanced fields feature is supported only when plugins.calcite.enabled=true"); } @Ignore(