[Enhancement] Add error handling for known limitation of sql JOIN (#4344)

Author: RyanL1997

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<T extends Reducible>
@@ -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<Boolean>() {
+          @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

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<From> 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;
   }
 

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<KVValue> 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;
+  }
 }

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);