diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java index 5d7135ec7e..b818795609 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java @@ -218,4 +218,137 @@ WHERE NOT EXISTS (SELECT 1 FROM catalog.departments WHERE dept_id = age) LogicalTableScan(table=[[catalog, employees]]) """); } + + @Test + public void selectLiteralWithoutFrom() { + // FROM-less SELECT produces a one-row result via LogicalValues so the downstream + // Project evaluates over a single row. + givenQuery("SELECT 1") + .assertPlan( + """ + LogicalSort(sort0=[$0], dir0=[ASC]) + LogicalValues(tuples=[[{ 1 }]]) + """); + } + + @Test + public void selectExpressionWithoutFrom() { + givenQuery("SELECT 1 + 1") + .assertPlan( + """ + LogicalProject(1 + 1=[+(1, 1)]) + LogicalValues(tuples=[[{ 0 }]]) + """); + } + + @Test + public void testHavingMaxCol() { + givenQuery( + """ + SELECT department FROM catalog.employees + GROUP BY department HAVING MAX(age) > 30 + """) + .assertPlan( + """ + LogicalProject(department=[$1]) + LogicalFilter(condition=[>($0, 30)]) + LogicalProject(MAX(age)=[$1], department=[$0]) + LogicalAggregate(group=[{0}], MAX(age)=[MAX($1)]) + LogicalProject(department=[$3], age=[$2]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testScalarFnOverAggregate() { + givenQuery("SELECT ABS(MAX(age)) FROM catalog.employees") + .assertPlan( + """ + LogicalProject(ABS(MAX(age))=[ABS($0)]) + LogicalAggregate(group=[{}], MAX(age)=[MAX($0)]) + LogicalProject(age=[$2]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testArithmeticOnAggregates() { + givenQuery("SELECT MAX(age) + MIN(age) AS range_sum FROM catalog.employees") + .assertPlan( + """ + LogicalProject(range_sum=[+($0, $1)]) + LogicalAggregate(group=[{}], MAX(age)=[MAX($0)], MIN(age)=[MIN($0)]) + LogicalProject(age=[$2]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testHavingCountStar() { + givenQuery( + """ + SELECT department FROM catalog.employees + GROUP BY department HAVING COUNT(*) > 5 + """) + .assertPlan( + """ + LogicalProject(department=[$1]) + LogicalFilter(condition=[>($0, 5)]) + LogicalProject(COUNT(*)=[$1], department=[$0]) + LogicalAggregate(group=[{0}], COUNT(*)=[COUNT()]) + LogicalProject(department=[$3]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testHavingWithAlias() { + givenQuery( + """ + SELECT department, COUNT(*) AS cnt FROM catalog.employees + GROUP BY department HAVING cnt > 1 + """) + .assertPlan( + """ + LogicalProject(department=[$1], cnt=[$0]) + LogicalFilter(condition=[>($0, 1)]) + LogicalProject(COUNT(*)=[$1], department=[$0]) + LogicalAggregate(group=[{0}], COUNT(*)=[COUNT()]) + LogicalProject(department=[$3]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testHavingCompoundAnd() { + givenQuery( + """ + SELECT department FROM catalog.employees + GROUP BY department HAVING MAX(age) > 30 AND MIN(age) < 50 + """) + .assertPlan( + """ + LogicalProject(department=[$2]) + LogicalFilter(condition=[AND(>($0, 30), <($1, 50))]) + LogicalProject(MAX(age)=[$1], MIN(age)=[$2], department=[$0]) + LogicalAggregate(group=[{0}], MAX(age)=[MAX($1)], MIN(age)=[MIN($1)]) + LogicalProject(department=[$3], age=[$2]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testWindowOrderByDefaultsNullsFirst() { + // Window function ORDER BY without explicit NULLS FIRST/LAST defaults to NULLS FIRST, + // matching top-level ORDER BY semantics. + givenQuery( + """ + SELECT name, ROW_NUMBER() OVER (ORDER BY id) AS rn FROM catalog.employees + """) + .assertPlan( + """ + LogicalProject(name=[$1], rn=[ROW_NUMBER() OVER (ORDER BY $0 NULLS FIRST)]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } } diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java index 5f58dea227..f18ac0b047 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java @@ -21,6 +21,7 @@ import org.apache.calcite.rex.RexLambdaRef; import org.apache.calcite.rex.RexNode; import org.apache.calcite.tools.FrameworkConfig; +import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.UnresolvedExpression; import org.opensearch.sql.ast.tree.HighlightConfig; import org.opensearch.sql.calcite.utils.CalciteToolsHelper; @@ -64,6 +65,12 @@ public class CalcitePlanContext { @Getter public Map rexLambdaRefMap; + /** + * Maps AggregateFunction AST nodes to their output field index for HAVING/post-aggregate + * resolution. + */ + @Getter private final Map aggregateOutputIndex = new HashMap<>(); + /** * List of captured variables from outer scope for lambda functions. When a lambda body references * a field that is not a lambda parameter, it gets captured and stored here. The captured diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 10c5d2aa88..7241800b36 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -1748,6 +1748,23 @@ private void visitAggregation( reordered.addAll(aggRexList); } context.relBuilder.project(reordered); + + // Register aggregate output indices for HAVING / post-aggregate resolution. clear() is safe: + // V2 grammar doesn't allow subqueries that nest aggregation scopes above this point. + context.getAggregateOutputIndex().clear(); + int aggStartIdx = metricsFirst ? 0 : aliasedGroupByList.size(); + for (int i = 0; i < aggExprList.size(); i++) { + AggregateFunction aggFunc = extractAggregateFunction(aggExprList.get(i)); + if (aggFunc != null) { + context.getAggregateOutputIndex().put(aggFunc, aggStartIdx + i); + } + } + } + + private static AggregateFunction extractAggregateFunction(UnresolvedExpression expr) { + if (expr instanceof AggregateFunction af) return af; + if (expr instanceof Alias alias) return extractAggregateFunction(alias.getDelegated()); + return null; } private Optional getTimeSpanField(UnresolvedExpression expr) { @@ -4181,12 +4198,17 @@ public RelNode visitMvExpand(MvExpand mvExpand, CalcitePlanContext context) { @Override public RelNode visitValues(Values values, CalcitePlanContext context) { - // Accept SQL SELECT without FROM (dual table), encoded as Values([[]]) — one row, zero columns. List> rows = values.getValues(); - if (rows == null || rows.isEmpty() || (rows.size() == 1 && rows.get(0).isEmpty())) { + if (rows == null || rows.isEmpty()) { + // PPL empty subsearch (e.g., `... | append [ ]`): zero rows, no columns. context.relBuilder.values(context.relBuilder.getTypeFactory().builder().build()); return context.relBuilder.peek(); } + if (rows.size() == 1 && rows.get(0).isEmpty()) { + // SQL FROM-less SELECT (dual table) encoded as Values([[]]): one-row relation for Project. + context.relBuilder.push(LogicalValues.createOneRow(context.relBuilder.getCluster())); + return context.relBuilder.peek(); + } throw new CalciteUnsupportedException("Inline VALUES with literal rows is unsupported"); } diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java index 01592cd30b..830b40d255 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java @@ -107,6 +107,17 @@ public RexNode analyzeJoinCondition(UnresolvedExpression unresolved, CalcitePlan return context.resolveJoinCondition(unresolved, this::analyze); } + @Override + public RexNode visitAggregateFunction(AggregateFunction node, CalcitePlanContext context) { + // Resolve post-aggregate AggregateFunction via registry populated in visitAggregation. + Integer index = context.getAggregateOutputIndex().get(node); + if (index == null) { + throw new IllegalStateException( + "Aggregate function " + node + " not registered (planner bug)"); + } + return context.relBuilder.field(index); + } + @Override public RexNode visitLiteral(Literal node, CalcitePlanContext context) { RexBuilder rexBuilder = context.rexBuilder; @@ -652,9 +663,10 @@ private List translateOrderKeys( field = b.desc(field); } return switch (opt.getNullOrder()) { - case NULL_LAST -> b.nullsLast(field); + // Unspecified NULLS defaults to NULLS FIRST, matching top-level ORDER BY. + case null -> b.nullsFirst(field); case NULL_FIRST -> b.nullsFirst(field); - default -> field; + case NULL_LAST -> b.nullsLast(field); }; }) .toList();