Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}

Expand Down
38 changes: 38 additions & 0 deletions legacy/src/main/java/org/opensearch/sql/legacy/utils/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
}

Expand Down Expand Up @@ -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);
Expand Down
Loading