This is an automated email from the ASF dual-hosted git repository.
mihaibudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 7ccb3d1404 [CALCITE-7744] ORDER BY agg(col) on a query where an alias
shadows col produces an invalid plan containing an aggregate call in a Project
7ccb3d1404 is described below
commit 7ccb3d14048be90f37e57f45a8449cc4f5b23f6b
Author: 1fanwang <[email protected]>
AuthorDate: Thu Oct 29 10:40:20 2026 -0700
[CALCITE-7744] ORDER BY agg(col) on a query where an alias shadows col
produces an invalid plan containing an aggregate call in a Project
Signed-off-by: 1fanwang <[email protected]>
---
.../apache/calcite/runtime/CalciteResource.java | 3 +
.../apache/calcite/sql/validate/OrderByScope.java | 23 +++++++
.../calcite/sql/validate/SqlValidatorImpl.java | 28 +++++++--
.../calcite/runtime/CalciteResource.properties | 1 +
.../java/org/apache/calcite/test/JdbcTest.java | 61 ++++++++++++++++++
.../org/apache/calcite/test/SqlValidatorTest.java | 72 ++++++++++++++++++++++
6 files changed, 182 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index 00e7667f80..093fcb4e50 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -409,6 +409,9 @@ ExInst<SqlValidatorException>
naturalOrUsingColumnNotCompatible(String a0,
@BaseMessage("Aggregate expressions cannot be nested")
ExInst<SqlValidatorException> nestedAggIllegal();
+ @BaseMessage("Aggregate expressions cannot be nested: ''{0}'' is a
SELECT-list alias for an aggregate expression")
+ ExInst<SqlValidatorException> orderByAliasNestedAgg(String a0);
+
@BaseMessage("Measure expressions can only occur within AGGREGATE function")
ExInst<SqlValidatorException> measureIllegal();
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
b/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
index b8e58222ad..025cce3b04 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
@@ -18,6 +18,7 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlIdentifier;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
@@ -27,6 +28,8 @@
import java.util.List;
+import static org.apache.calcite.sql.validate.SqlValidatorUtil.flatten;
+
/**
* Represents the name-resolution context for expressions in an ORDER BY
clause.
*
@@ -93,6 +96,26 @@ public class OrderByScope extends DelegatingScope {
@Override public void validateExpr(SqlNode expr) {
SqlNode expanded = validator.expandOrderExpr(select, expr);
+ expanded.accept(
+ new AggVisitor(validator.getOperatorTable(), false, true,
+ false, null, validator.catalogReader.nameMatcher()) {
+ @Override public Void visit(SqlCall call) {
+ switch (call.getKind()) {
+ case FILTER:
+ case WITHIN_DISTINCT:
+ return found(call);
+ default:
+ return super.visit(call);
+ }
+ }
+
+ @Override protected Void found(SqlCall call) {
+ final SqlValidatorUtil.FlatAggregate flat = flatten(call);
+ validator.validateAggregateParams(flat.aggregateCall, flat.filter,
+ flat.distinctList, flat.orderList, OrderByScope.this);
+ return null;
+ }
+ });
// expression needs to be valid in parent scope too
parent.validateExpr(expanded);
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 7f57f0a144..837d90b1d2 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -7951,13 +7951,27 @@ private SqlNode navigationInDefine(SqlNode node, String
alpha) {
}
for (SqlNode param : aggCall.getOperandList()) {
- if (a.findAgg(param) != null) {
+ final SqlCall nestedAgg = a.findAgg(param);
+ if (nestedAgg != null) {
+ if (scope instanceof OrderByScope) {
+ for (SqlNode node
+ : SqlUtil.getAncestry(param, n -> n == nestedAgg, n -> false)) {
+ final SqlNode original = getOriginal(node);
+ if (original instanceof SqlIdentifier
+ && ((SqlIdentifier) original).isSimple()) {
+ throw newValidationError(getOriginal(aggCall),
+ RESOURCE.orderByAliasNestedAgg(((SqlIdentifier)
original).getSimple()));
+ }
+ }
+ }
throw newValidationError(aggCall, RESOURCE.nestedAggIllegal());
}
}
if (filter != null) {
if (a.findAgg(filter) != null) {
- throw newValidationError(filter, RESOURCE.aggregateInFilterIllegal());
+ throw newValidationError(
+ scope instanceof OrderByScope ? getOriginal(filter) : filter,
+ RESOURCE.aggregateInFilterIllegal());
}
}
if (distinctList != null) {
@@ -8818,9 +8832,9 @@ private SqlNode nthSelectItem(int ordinal, final
SqlParserPos pos) {
final SqlNameMatcher nameMatcher = catalogReader.nameMatcher();
RelDataTypeField field = nameMatcher.field(rowType, alias);
if (field != null) {
- return nthSelectItem(
- field.getIndex(),
- id.getParserPosition());
+ final SqlNode expanded = nthSelectItem(field.getIndex(),
id.getParserPosition());
+ setOriginal(expanded, id);
+ return expanded;
}
}
@@ -8850,7 +8864,9 @@ private SqlNode nthSelectItem(int ordinal, final
SqlParserPos pos) {
// operand[1] is a field name, not a column reference, and must not be
qualified.
CallCopyingArgHandler argHandler = new CallCopyingArgHandler(call,
false);
call.getOperator().acceptCall(this, call, true, argHandler);
- return argHandler.result();
+ final SqlNode expanded = argHandler.result();
+ setOriginal(expanded, call);
+ return expanded;
}
}
diff --git
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index 2e6e56fb6b..43c7e1db1b 100644
---
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -143,6 +143,7 @@ NotSelectDistinctExpr=Expression ''{0}'' is not in the
select clause
AggregateIllegalInClause=Aggregate expression is illegal in {0} clause
WindowedAggregateIllegalInClause=Windowed aggregate expression is illegal in
{0} clause
NestedAggIllegal=Aggregate expressions cannot be nested
+OrderByAliasNestedAgg=Aggregate expressions cannot be nested: ''{0}'' is a
SELECT-list alias for an aggregate expression
MeasureIllegal=Measure expressions can only occur within AGGREGATE function
MeasureMustBeInAggregateQuery=Measure expressions can only occur within a
GROUP BY query
AggregateInFilterIllegal=FILTER must not contain aggregate expression
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index c782728ef6..e95a29ed1b 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -3462,6 +3462,67 @@ void testJoinWithTrimmingConfigs(boolean
enableTrimmingByConfig,
+ "store_id=0; grocery_sqft=null\n");
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7744";>[CALCITE-7744]</a>
+ * ORDER BY agg(col) on a query where an alias shadows col produces an
invalid
+ * plan containing an aggregate call in a Project. */
+ @Test void testOrderByAggregateAliasShadowingRejected() {
+ CalciteAssert.that()
+ .with(CalciteAssert.Config.SCOTT)
+ .query("SELECT max(sal) AS sal, deptno, job "
+ + "FROM emp GROUP BY deptno, job ORDER BY max(sal)")
+ .throws_("Aggregate expressions cannot be nested: 'SAL' is a "
+ + "SELECT-list alias for an aggregate expression");
+ }
+
+ static Stream<Arguments> orderByAliasQueries() {
+ return Stream.of(
+ arguments("select deptno in (select deptno from dept) as b from emp
order by b",
+ String.join("", Collections.nCopies(14, "B=true\n"))),
+ arguments("select exists (select 1 from dept d where d.deptno =
e.deptno) as b "
+ + "from emp e order by b",
+ String.join("", Collections.nCopies(14, "B=true\n"))),
+ arguments("select sal > all (select sal from emp) as b from emp order
by b",
+ String.join("", Collections.nCopies(14, "B=false\n"))),
+ arguments("select deptno + 1 as d1, d1 + 2 as measure d3 from emp
order by d3 + 1",
+ String.join("", Collections.nCopies(3, "D1=11; D3=13\n"))
+ + String.join("", Collections.nCopies(5, "D1=21; D3=23\n"))
+ + String.join("", Collections.nCopies(6, "D1=31; D3=33\n"))));
+ }
+
+ @ParameterizedTest
+ @MethodSource("orderByAliasQueries")
+ void testOrderByAliasExecutesValidatedExpressions(String query, String
expected) {
+ CalciteAssert.that()
+ .with(CalciteAssert.Config.SCOTT)
+ .query(query)
+ .returns(expected);
+ }
+
+ @Test void testOrderByAliasExecutesBoundParameter() {
+ CalciteAssert.that()
+ .with(CalciteAssert.Config.SCOTT)
+ .query("select sal + ? as x from emp order by x + 1")
+ .consumesPreparedStatement(p -> p.setInt(1, 1))
+ .returns2("X=801\nX=951\nX=1101\nX=1251\nX=1251\nX=1301\nX=1501\n"
+ + "X=1601\nX=2451\nX=2851\nX=2976\nX=3001\nX=3001\nX=5001\n");
+ }
+
+ @Test void testOrderByAliasExecutesCurrentDate() throws SQLException {
+ try (Connection connection = CalciteAssert.that()
+ .with(CalciteAssert.Config.SCOTT).connect();
+ Statement statement = connection.createStatement();
+ ResultSet resultSet =
+ statement.executeQuery("select current_date as d, empno from emp
order by d")) {
+ assertThat(resultSet.getMetaData().getColumnType(1), is(Types.DATE));
+ int count = 0;
+ while (resultSet.next()) {
+ assertNotNull(resultSet.getDate(1));
+ ++count;
+ }
+ assertThat(count, is(14));
+ }
+ }
+
/** Tests ORDER BY ... DESC. Nulls come first (they come last for ASC). */
@Test void testOrderByDesc() {
CalciteAssert.that()
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index deda4bb7d3..ba5d9b02de 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -85,6 +85,8 @@
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -5070,6 +5072,10 @@ private void checkNegWindow(String s, String msg) {
// various kinds of measure expressions
sql("select deptno, empno + 1 as measure e1 from emp").ok();
sql("select *, empno + 1 as measure e1 from emp").ok();
+ // Guards against validating measure expressions in the SELECT scope when
expanding ORDER BY,
+ // which would fail because measures can reference aliases (e.g. d1)
defined in SELECT.
+ sql("select deptno + 1 as d1, d1 + 2 as measure d3\n"
+ + "from emp order by d3").ok();
// an aggregate function in a measure does not make it an aggregate query
sql("select *, sum(empno) as measure e1 from emp").ok();
@@ -8588,6 +8594,72 @@ void testGroupExpressionEquivalenceParams() {
.fails(ERR_NESTED_AGG);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7744";>[CALCITE-7744]</a>
+ * ORDER BY agg(col) on a query where an alias shadows col produces an
invalid
+ * plan containing an aggregate call in a Project. */
+ @ParameterizedTest
+ @ValueSource(strings = {
+ "^max(sal)^",
+ "^max(sal)^ desc",
+ "sal + ^max(sal)^",
+ "^sum(sal + 1)^",
+ "^max(1 + sal)^",
+ "^max(sal + sal)^",
+ "^max(\"SAL\")^ nulls last"
+ })
+ void testOrderByAggregateAliasShadowingRejected(String orderBy) {
+ final String error = "Aggregate expressions cannot be nested: 'SAL' is a "
+ + "SELECT-list alias for an aggregate expression";
+ sql("select max(sal) as sal, deptno, job from emp\n"
+ + "group by deptno, job order by " + orderBy)
+ .fails(error);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {
+ "^max(sal)^",
+ "^max(2 * sal)^",
+ "case when true then ^sum(sal + 1)^ else 0 end"
+ })
+ void testOrderByAliasRejectsAggregateExpressions(String orderBy) {
+ sql("select max(sal) + 1 as sal, deptno, job from emp\n"
+ + "group by deptno, job order by " + orderBy)
+ .fails("Aggregate expressions cannot be nested: 'SAL' is a "
+ + "SELECT-list alias for an aggregate expression");
+ }
+
+ @Test void testOrderByAggregateAliasFilterRejected() {
+ sql("select max(sal) as sal, deptno, job from emp\n"
+ + "group by deptno, job order by count(*) filter (where ^sal > 1000^)")
+ .fails("FILTER must not contain aggregate expression");
+ sql("select max(sal) as m, deptno, job from emp\n"
+ + "group by deptno, job order by max(sal)").ok();
+ }
+
+ @Test void testOrderByAggregateAliasWithinDistinctRejected() {
+ sql("select max(sal) as m, deptno from emp group by deptno\n"
+ + "order by ^sum(sal)^ within distinct (m)")
+ .fails("WITHIN DISTINCT must not contain aggregate expression");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {
+ "select deptno in (select deptno from dept) as b from emp order by b",
+ "select exists (select 1 from dept d where d.deptno = e.deptno) as b "
+ + "from emp e order by b",
+ "select sal > all (select sal from emp) as b from emp order by b",
+ "select sal + ? as x from emp order by x + 1",
+ "select current_date as d, empno from emp order by d",
+ "select deptno + 1 as d1, d1 + 2 as measure d3 from emp order by d3 + 1",
+ "select (select min(deptno) from dept) as b from emp order by b",
+ "select sal + ? as x from emp order by 1 desc",
+ "select current_timestamp as d, empno from emp order by 1",
+ "select deptno + 1 as d1, d1 + 2 as measure d3 from emp order by d3"
+ })
+ void testOrderByAliasPreservesValidatedExpressions(String query) {
+ sql(query).ok();
+ }
+
@Test void testNestedAggOver() {
sql("select sum(max(empno))\n"
+ " OVER (order by ^deptno^ ROWS 2 PRECEDING)\n"
