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 233e5aeaf12..49754a49df4 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; @@ -182,8 +182,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"); } } @@ -673,14 +672,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. */ @@ -732,14 +729,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 @@ -762,20 +757,17 @@ 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( 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/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..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 @@ -139,45 +139,18 @@ 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))); + private void verifyQueryThrowsCalciteError(String query) { + Exception e = + assertThrows(Exception.class, () -> executeQuery(String.format(query, TEST_INDEX_ACCOUNT))); verifyErrorMessageContains( - e4, "Enhanced fields features are supported only when" + " plugins.calcite.enabled=true"); + e, "Enhanced fields feature is supported only when plugins.calcite.enabled=true"); } @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 8ce07421956..f638d7f2f65 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; @@ -293,10 +294,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( @@ -307,10 +305,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 feature"); } }