diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitor.java b/legacy/src/main/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitor.java index 5e89b3b8aef..3441eb5e8db 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitor.java @@ -53,6 +53,8 @@ import org.opensearch.sql.legacy.antlr.parser.OpenSearchLegacySqlParser.SubqueryTableItemContext; import org.opensearch.sql.legacy.antlr.parser.OpenSearchLegacySqlParser.TableNamePatternContext; import org.opensearch.sql.legacy.antlr.parser.OpenSearchLegacySqlParserBaseVisitor; +import org.opensearch.sql.legacy.exception.SqlParseException; +import org.opensearch.sql.legacy.utils.Util; /** ANTLR parse tree visitor to drive the analysis process. */ public class AntlrSqlParseTreeVisitor @@ -264,6 +266,42 @@ public T visitFunctionNameBase(FunctionNameBaseContext ctx) { return visitor.visitFunctionName(ctx.getText()); } + @Override + public T visitGroupByItem(OpenSearchLegacySqlParser.GroupByItemContext ctx) { + ParserRuleContext fromClause = ctx.getParent(); + + boolean hasJoin = detectJoinInFromClause(fromClause); + + if (hasJoin) { + String errorMessage = + Util.JOIN_AGGREGATION_ERROR_PREFIX + + Util.DOC_REDIRECT_MESSAGE + + Util.getJoinAggregationDocumentationUrl(AntlrSqlParseTreeVisitor.class); + throw new RuntimeException(errorMessage, new SqlParseException(errorMessage)); + } + return super.visitGroupByItem(ctx); + } + + boolean detectJoinInFromClause(ParserRuleContext fromClause) { + return fromClause.accept( + new OpenSearchLegacySqlParserBaseVisitor() { + @Override + public Boolean visitTableSourceBase(TableSourceBaseContext ctx) { + return !ctx.joinPart().isEmpty(); + } + + @Override + protected Boolean defaultResult() { + return false; + } + + @Override + protected Boolean aggregateResult(Boolean aggregate, Boolean nextResult) { + return aggregate || nextResult; + } + }); + } + @Override public T visitBinaryComparisonPredicate(BinaryComparisonPredicateContext ctx) { if (isNamedArgument(ctx)) { // Essentially named argument is assign instead of comparison diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/parser/SqlParser.java b/legacy/src/main/java/org/opensearch/sql/legacy/parser/SqlParser.java index c380ded1762..9473afce5f4 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/parser/SqlParser.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/parser/SqlParser.java @@ -48,6 +48,7 @@ import org.opensearch.sql.legacy.domain.hints.HintFactory; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.query.multi.MultiQuerySelect; +import org.opensearch.sql.legacy.utils.Util; /** * OpenSearch sql support @@ -365,6 +366,15 @@ public JoinSelect parseJoinSelect(SQLQueryExpr sqlExpr) throws SqlParseException MySqlSelectQueryBlock query = (MySqlSelectQueryBlock) sqlExpr.getSubQuery().getQuery(); + // Check for JOIN + GROUP BY combination and throw error + if (query.getGroupBy() != null && !query.getGroupBy().getItems().isEmpty()) { + String errorMessage = + Util.JOIN_AGGREGATION_ERROR_PREFIX + + Util.DOC_REDIRECT_MESSAGE + + Util.getJoinAggregationDocumentationUrl(SqlParser.class); + throw new SqlParseException(errorMessage); + } + List joinedFrom = findJoinedFrom(query.getFrom()); if (joinedFrom.size() != 2) { throw new RuntimeException("currently supports only 2 tables join"); @@ -399,7 +409,6 @@ public JoinSelect parseJoinSelect(SQLQueryExpr sqlExpr) throws SqlParseException updateJoinLimit(query.getLimit(), joinSelect); - // todo: throw error feature not supported: no group bys on joins ? return joinSelect; } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/utils/Util.java b/legacy/src/main/java/org/opensearch/sql/legacy/utils/Util.java index 500c64ed944..9bdbb91ebd1 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/utils/Util.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/utils/Util.java @@ -41,6 +41,15 @@ public class Util { public static final String NESTED_JOIN_TYPE = "NestedJoinType"; + public static final String JOIN_AGGREGATION_ERROR_PREFIX = + "JOIN queries do not support aggregations on the joined result."; + + public static final String DOC_REDIRECT_MESSAGE = " For more information, see "; + + public static final String OPENSEARCH_DOC_BASE_URL = "https://docs.opensearch.org/"; + + public static final String LIMITATION_DOC_PATH = "/search-plugins/sql/limitation/"; + public static String joiner(List lists, String oper) { if (lists.size() == 0) { @@ -280,4 +289,33 @@ public static SQLExpr toSqlExpr(String sql) { } return expr; } + + /** + * Gets the OpenSearch major.minor version for documentation links. Converts "x.y.z" format to + * "x.y". + * + * @param clazz The class to get package implementation version from + * @return The major.minor version string, or "latest" if version cannot be determined + */ + public static String getDocumentationVersion(Class clazz) { + String version = clazz.getPackage().getImplementationVersion(); + if (version == null) { + return "latest"; + } + String[] parts = version.split("\\."); + if (parts.length >= 2) { + return parts[0] + "." + parts[1]; + } + return "latest"; + } + + /** + * Builds a complete OpenSearch documentation URL for JOIN aggregation limitation. + * + * @param clazz The class to get package implementation version from + * @return Complete documentation URL with version + */ + public static String getJoinAggregationDocumentationUrl(Class clazz) { + return OPENSEARCH_DOC_BASE_URL + getDocumentationVersion(clazz) + LIMITATION_DOC_PATH; + } } diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitorTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitorTest.java index 2baaa919804..4f82f9e0190 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitorTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/antlr/visitor/AntlrSqlParseTreeVisitorTest.java @@ -20,6 +20,7 @@ import org.opensearch.sql.legacy.antlr.SqlAnalysisConfig; import org.opensearch.sql.legacy.antlr.semantic.scope.SemanticContext; import org.opensearch.sql.legacy.antlr.semantic.types.Type; +import org.opensearch.sql.legacy.antlr.semantic.types.base.OpenSearchIndex; import org.opensearch.sql.legacy.antlr.semantic.types.special.Product; import org.opensearch.sql.legacy.antlr.semantic.visitor.TypeChecker; import org.opensearch.sql.legacy.exception.SqlFeatureNotImplementedException; @@ -31,6 +32,11 @@ public class AntlrSqlParseTreeVisitorTest { new TypeChecker(new SemanticContext()) { @Override public Type visitIndexName(String indexName) { + // Special case: for JOIN tests (both implicit and explicit JOIN + GROUP BY validation) + // Return proper OpenSearchIndex to enable JOIN semantic analysis for "testIndex" + if ("testIndex".equals(indexName)) { + return new OpenSearchIndex(indexName, OpenSearchIndex.IndexType.INDEX); + } return null; // avoid querying mapping on null LocalClusterState } @@ -101,6 +107,13 @@ public void visitFunctionAsAggregatorShouldThrowException() { visit("SELECT max(abs(age)) FROM test"); } + @Test + public void visitExplicitJoinWithGroupByShouldThrowException() { + exceptionRule.expect(RuntimeException.class); + exceptionRule.expectMessage("JOIN queries do not support aggregations on the joined result."); + visit("SELECT COUNT(*) FROM testIndex t1 JOIN testIndex t2 ON t1.id = t2.id GROUP BY t1.field"); + } + @Test public void visitUnsupportedOperatorShouldThrowException() { exceptionRule.expect(SqlFeatureNotImplementedException.class);