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 c2d12ffd186..22a6e60f821 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 @@ -5379,13 +5379,19 @@ protected void rewriteOrderByAll(SqlSelect select) { final SqlNode expr = SqlUtil.stripAs(selectItem); if (expr instanceof SqlIdentifier && ((SqlIdentifier) expr).isStar()) { for (SqlNode column : expandStarForAllRewrite(select, expr)) { - keys.add(applyOrderByAllDirection(column, desc, nulls, pos)); + if (dependsOnInput(column)) { + keys.add(applyOrderByAllDirection(column, desc, nulls, pos)); + } } continue; } - keys.add(applyOrderByAllDirection(expr, desc, nulls, pos)); + if (dependsOnInput(expr)) { + keys.add(applyOrderByAllDirection(expr, desc, nulls, pos)); + } } - select.setOrderBy(new SqlNodeList(keys, pos)); + // An empty but non-null ORDER BY list would make the select unparse with + // redundant parentheses, so remove the clause entirely. + select.setOrderBy(keys.isEmpty() ? null : new SqlNodeList(keys, pos)); } /** Wraps a single ORDER BY ALL key with the optional descending direction @@ -5565,8 +5571,71 @@ protected void validateGroupClause(SqlSelect select) { } } + /** Returns whether an expression depends on the input row, and is therefore + * worth using as an implicit {@code GROUP BY ALL} grouping key or + * {@code ORDER BY ALL} sort key. + * + *
An expression that references no column, and all of whose operators are + * deterministic, has the same value in every row; grouping by it does not + * divide the input into more than one group, and sorting by it does not + * reorder rows. Such expressions -- for example {@code 42}, {@code 1 + 1}, + * {@code upper('x')}, {@code current_date} and {@code ?} -- are therefore + * not made keys. This follows BigQuery, which infers keys only from + * expressions that reference a name in the FROM clause. + * + *
A call to a non-deterministic operator such as {@code RAND()} does vary + * from row to row, and is retained. This is the same rule that + * [CALCITE-7697] + * applies to window keys, one phase later. An unresolved function is + * retained, because its determinism is not yet known. + * + *
A sub-query is always retained, and is not inspected. Its identifiers + * include table names, which are not expressions in this sense, and + * resolving them here would be premature. Retaining is conservative, and is + * required for a correlated sub-query. + * + *
The rule inherits the limits of
+ * {@link SqlOperator#isDeterministic()}: a user-defined function is
+ * deterministic unless it says otherwise, so a call to one with no column
+ * argument is excluded. */
+ private boolean dependsOnInput(SqlNode node) {
+ try {
+ final SqlVisitor The constant is not a grouping key, so no grouping key remains and the
+ * query is a single-group aggregation. It therefore returns one row even
+ * though the input is empty. Before [CALCITE-7675] the constant was a
+ * grouping key, empty input yielded no groups, and the query returned no
+ * rows. BigQuery documents the behavior asserted here: "If the set of
+ * inferred grouping keys is empty after exclusions are applied, all input
+ * rows are considered a single group for aggregation." */
@Test void testGroupByAllOverEmptyInput() {
CalciteAssert.hr()
.query("select 'x', count(*)\n"
+ "from \"hr\".\"emps\"\n"
+ "where false\n"
+ "group by all")
- .returnsCount(0);
+ .returns("EXPR$0=x; EXPR$1=0\n");
}
/** Test case for
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 38f883743ec..d5da805d0c5 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -7390,6 +7390,55 @@ public boolean isBangEqualAllowed() {
.ok();
}
+ /** Tests that {@code ORDER BY ALL} makes a sort key only of a SELECT item
+ * that depends on the input row. See
+ * [CALCITE-7675]. */
+ @Test void testOrderByAllExcludesExpressionsNotDependingOnInput() {
+ // A constant cannot reorder rows, so it is not a sort key. Before
+ // [CALCITE-7675] the synthesized "ORDER BY 42" was read as a sort ordinal
+ // and failed with "Ordinal out of range", in the default conformance.
+ sql("select sal, 42 from emp order by all")
+ .rewritesTo("SELECT `SAL`, 42\n"
+ + "FROM `EMP`\n"
+ + "ORDER BY `SAL`");
+
+ // The dangerous case: an in-range value silently resolved to a different
+ // SELECT item, so this query sorted on DEPTNO twice rather than on the
+ // constant and then DEPTNO.
+ sql("select 2, deptno from emp order by all")
+ .rewritesTo("SELECT 2, `DEPTNO`\n"
+ + "FROM `EMP`\n"
+ + "ORDER BY `DEPTNO`");
+
+ // Constant expressions and deterministic niladic functions are excluded
+ // too; a non-deterministic call is retained.
+ sql("select sal, 1 + 1, current_date from emp order by all")
+ .rewritesTo("SELECT `SAL`, 1 + 1, CURRENT_DATE\n"
+ + "FROM `EMP`\n"
+ + "ORDER BY `SAL`");
+ sql("select sal, rand() from emp order by all")
+ .rewritesTo("SELECT `SAL`, RAND()\n"
+ + "FROM `EMP`\n"
+ + "ORDER BY `SAL`, RAND()");
+
+ // If no sort key remains, no ORDER BY is emitted at all.
+ sql("select 42 from emp order by all")
+ .rewritesTo("SELECT 42\n"
+ + "FROM `EMP`");
+
+ // The trailing direction still applies to every remaining key.
+ sql("select sal, deptno, 42 from emp order by all desc")
+ .rewritesTo("SELECT `SAL`, `DEPTNO`, 42\n"
+ + "FROM `EMP`\n"
+ + "ORDER BY `SAL` DESC, `DEPTNO` DESC");
+
+ // A sort ordinal the user writes is still resolved, and still rejected
+ // when out of range.
+ sql("select sal, deptno from emp order by 2").ok();
+ sql("select sal, deptno from emp order by ^42^")
+ .fails("Ordinal out of range");
+ }
+
@Test void testOrder() {
final SqlConformance conformance = fixture().conformance();
sql("select empno as x from emp order by empno").ok();
@@ -7860,6 +7909,125 @@ public boolean isBangEqualAllowed() {
.ok();
}
+ /** Tests that {@code GROUP BY ALL} makes a grouping key only of a SELECT item
+ * that depends on the input row. See
+ * [CALCITE-7675]. */
+ @Test void testGroupByAllExcludesExpressionsNotDependingOnInput() {
+ // A constant is not a grouping key.
+ sql("select deptno, 42, count(*) from emp group by all")
+ .rewritesTo("SELECT `DEPTNO`, 42, COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY `EMP`.`DEPTNO`");
+
+ // Nor is a constant expression, nor a deterministic niladic function.
+ sql("select deptno, 1 + 1, upper('x'), current_date, count(*)\n"
+ + "from emp group by all")
+ .rewritesTo("SELECT `DEPTNO`, 1 + 1, UPPER('x'), CURRENT_DATE, COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY `EMP`.`DEPTNO`");
+
+ // If no grouping key remains the query is a single-group aggregation.
+ sql("select 42, count(*) from emp group by all")
+ .rewritesTo("SELECT 42, COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY ()");
+
+ // An expression over a column is a grouping key, as always.
+ sql("select deptno + 1, count(*) from emp group by all")
+ .rewritesTo("SELECT `DEPTNO` + 1, COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY `EMP`.`DEPTNO` + 1");
+ sql("select case when sal > 100 then 'high' else 'low' end, count(*)\n"
+ + "from emp group by all")
+ .rewritesTo("SELECT CASE WHEN `SAL` > 100 THEN 'high' ELSE 'low' END, COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY CASE WHEN `EMP`.`SAL` > 100 THEN 'high' ELSE 'low' END");
+
+ // A non-deterministic call varies from row to row, so it is a grouping key.
+ sql("select rand(), count(*) from emp group by all")
+ .rewritesTo("SELECT RAND(), COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY RAND()");
+
+ // Excluding the constant also means the expansion never emits a bare
+ // integer, which a conformance that groups by ordinal would otherwise read
+ // as a select-list position. Before [CALCITE-7675] this failed with
+ // "Ordinal out of range".
+ sql("select deptno, 42, count(*) from emp group by all")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+
+ // An ordinal the user writes is still resolved, and still rejected when out
+ // of range.
+ sql("select deptno, count(*) from emp group by 1")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+ sql("select deptno, count(*) from emp group by ^42^")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .fails("Ordinal out of range");
+
+ // A sub-query is retained without being inspected. Its identifiers include
+ // table names and aliases, which are not expressions; treating one as a
+ // possible niladic function would wrongly reject a query whose alias
+ // happens to name one, under a conformance that requires parentheses.
+ sql("select (select 1 from (values (1)) as pi), count(*) from emp group by all")
+ .withConformance(SqlConformanceEnum.BIG_QUERY)
+ .ok();
+ sql("select (select 1 from (values (1)) as pi), count(*) from emp group by all")
+ .rewritesTo("SELECT (SELECT 1\n"
+ + "FROM (VALUES ROW(1)) AS PI), COUNT(*)\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY (SELECT 1\n"
+ + "FROM (VALUES ROW(1)) AS PI)");
+ }
+
+ /** Tests that the expansion of {@code GROUP BY ALL} can be re-parsed with
+ * the same meaning under a conformance that reads an integer in GROUP BY as
+ * a select-list ordinal.
+ *
+ * The expanded form is what is stored as a view or materialized-table
+ * definition, so it is read back by a validator that cannot know which keys
+ * the user wrote and which the expansion synthesized. Before
+ * [CALCITE-7675]
+ * a constant SELECT item became a grouping key and the definition contained
+ * a bare integer literal, which the re-parse resolved as an ordinal: out of
+ * range it failed to validate, and in range it silently designated a
+ * different SELECT item. */
+ @Test void testGroupByAllExpansionSurvivesReparseAsOrdinal() {
+ // Sergey Nuyanzin's query from the FLIP-606 discussion. This expanded to
+ // "GROUP BY 42", which is not a valid definition: 42 exceeds the size of
+ // the select list.
+ sql("select count(*) as cnt, sum(sal) as sm, 42 as just_a_constant\n"
+ + "from emp group by all")
+ .rewritesTo("SELECT COUNT(*) AS `CNT`, SUM(`SAL`) AS `SM`, 42 AS `JUST_A_CONSTANT`\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY ()");
+
+ // The expansion above, fed back in where GROUP BY ordinals are enabled.
+ sql("select count(*) as cnt, sum(sal) as sm, 42 as just_a_constant\n"
+ + "from emp group by ()")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+
+ // The quieter case: an in-range value. This expanded to
+ // "GROUP BY `EMP`.`DEPTNO`, 2", whose re-parse read 2 as the second
+ // SELECT item -- an aggregate, and so not a legal grouping key.
+ sql("select deptno, count(*) as c, 2 as k from emp group by all")
+ .rewritesTo("SELECT `DEPTNO`, COUNT(*) AS `C`, 2 AS `K`\n"
+ + "FROM `EMP`\n"
+ + "GROUP BY `EMP`.`DEPTNO`");
+
+ sql("select deptno, count(*) as c, 2 as k from emp\n"
+ + "group by emp.deptno")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+
+ // A grouping key the user writes as an ordinal is still resolved as one.
+ sql("select deptno, count(*) from emp group by 1")
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+ }
+
/** Test case for
* [CALCITE-5507]
* HAVING alias failed when aggregate function in condition. */
diff --git a/core/src/test/resources/sql/agg.iq b/core/src/test/resources/sql/agg.iq
index 41f2436a25b..1401e05cfb3 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -4749,4 +4749,43 @@ order by y;
!ok
+# [CALCITE-7675] GROUP BY ALL should infer grouping keys only from expressions
+# that depend on the input. The constant 42 is the same in every row, so it is
+# not a grouping key and the rows are grouped by X alone. Verified against
+# BigQuery, which infers keys only from expressions that reference a name in
+# the FROM clause.
+select x, 42 as c, count(*) as n
+from (values (1, 'a'), (1, 'a'), (2, 'b')) as t(x, y)
+group by all
+order by x;
++---+----+---+
+| X | C | N |
++---+----+---+
+| 1 | 42 | 2 |
+| 2 | 42 | 1 |
++---+----+---+
+(2 rows)
+
+!ok
+
+# [CALCITE-7675] GROUP BY ALL should infer grouping keys only from expressions
+# that depend on the input. When none does, no grouping key remains and the
+# query is a single-group aggregation, GROUP BY (). It
+# therefore returns one row even though the input is empty. BigQuery
+# specifies the same: "if the set of inferred grouping keys is empty after
+# exclusions are applied, all input rows are considered a single group for
+# aggregation".
+select 42 as c, count(*) as n
+from (values (1, 'a')) as t(x, y)
+where x < 0
+group by all;
++----+---+
+| C | N |
++----+---+
+| 42 | 0 |
++----+---+
+(1 row)
+
+!ok
+
# End agg.iq
diff --git a/core/src/test/resources/sql/sort.iq b/core/src/test/resources/sql/sort.iq
index a5320474257..5e293528d43 100644
--- a/core/src/test/resources/sql/sort.iq
+++ b/core/src/test/resources/sql/sort.iq
@@ -583,4 +583,22 @@ order by all;
!ok
+# [CALCITE-7675] GROUP BY ALL should infer grouping keys only from expressions
+# that depend on the input; the same holds for the sort keys of ORDER BY ALL.
+# The constant 2 cannot reorder rows and is not a sort key, so the rows are
+# ordered by Y alone. Before [CALCITE-7675] the synthesized "ORDER BY 2" was
+# read as a sort ordinal and silently ordered by the second SELECT item.
+select 2 as c, y from (values (2, 'b'), (1, 'a'), (1, 'c')) as t(x, y)
+order by all;
++---+---+
+| C | Y |
++---+---+
+| 2 | a |
+| 2 | b |
+| 2 | c |
++---+---+
+(3 rows)
+
+!ok
+
# End sort.iq
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index 59ddce4af54..bccbad1a95b 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -426,6 +426,11 @@ in the order that they appear in the list; for example:
"SELECT x, y FROM t ORDER BY ALL" is equivalent to
"SELECT x, y FROM t ORDER BY x, y"
An optional trailing ASC / DESC and NULLS FIRST / NULLS LAST applies to all keys.
+An expression that references no column, and all of whose operators are
+deterministic, has the same value in every row, and cannot reorder rows; it is
+not a sort key. For example, "SELECT sal, 42 FROM emp ORDER BY ALL" sorts by
+`sal` alone. A call to a non-deterministic operator such as RAND() is a sort
+key. If no sort key remains, no ORDER BY clause is generated.
A `*` in the SELECT clause is expanded to its underlying columns, each of which
becomes a sort key; for example, "SELECT * FROM t ORDER BY ALL" sorts by every
column of `t`.
@@ -468,9 +473,17 @@ GROUP BY DISTINCT removes duplicate grouping sets (for example,
GROUP BY ALL followed by grouping items is equivalent to GROUP BY
(ALL is the default set quantifier).
GROUP BY ALL on its own groups by every expression in the SELECT clause
-that is not an aggregate function; for example,
+that is not an aggregate function and that depends on the input row;
+for example,
"SELECT deptno, SUM(sal) FROM emp GROUP BY ALL" is equivalent to
"SELECT deptno, SUM(sal) FROM emp GROUP BY deptno".
+An expression that references no column, and all of whose operators are
+deterministic, has the same value in every row, and is not a grouping key;
+for example, "SELECT deptno, 42, COUNT(*) FROM emp GROUP BY ALL" groups by
+`deptno` alone. A call to a non-deterministic operator such as RAND() does
+vary from row to row, and is a grouping key. If no grouping key remains, the
+query is a single-group aggregation, equivalent to GROUP BY (), and returns
+one row even if the input is empty.
A `*` in the SELECT clause is expanded to its underlying columns, each of which
becomes a grouping key; for example,
"SELECT *, COUNT(*) FROM emp GROUP BY ALL" groups by every column of `emp`.