This is an automated email from the ASF dual-hosted git repository.
xuzifu666 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 fcb07463d0 [CALCITE-7485] FIRST_VALUE/LAST_VALUE should only be
defined for window aggregates
fcb07463d0 is described below
commit fcb07463d05ac072f86634fdb53c70b113bb1b75
Author: Yu Xu <[email protected]>
AuthorDate: Wed May 6 14:33:10 2026 +0800
[CALCITE-7485] FIRST_VALUE/LAST_VALUE should only be defined for window
aggregates
---
.../org/apache/calcite/sql/SqlAggFunction.java | 5 +++
.../apache/calcite/sql/validate/SqlValidator.java | 6 ++++
.../calcite/sql/validate/SqlValidatorImpl.java | 5 ++-
.../org/apache/calcite/test/RelOptRulesTest.java | 3 +-
.../org/apache/calcite/test/SqlValidatorTest.java | 36 +++++++++++++++++++++-
.../org/apache/calcite/test/RelOptRulesTest.xml | 9 +++---
.../org/apache/calcite/test/SqlOperatorTest.java | 28 +++++++++++------
7 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlAggFunction.java
b/core/src/main/java/org/apache/calcite/sql/SqlAggFunction.java
index 59cb7522e6..7ba19c48de 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlAggFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlAggFunction.java
@@ -27,6 +27,7 @@
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.util.Optionality;
+import org.apache.calcite.util.Static;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -135,6 +136,10 @@ protected SqlAggFunction(
SqlValidator validator,
SqlValidatorScope scope,
SqlValidatorScope operandScope) {
+ if (requiresOver() && !validator.isInWindow()) {
+ throw validator.newValidationError(call,
+ Static.RESOURCE.absentOverClause());
+ }
super.validateCall(call, validator, scope, operandScope);
validator.validateAggregateParams(call, null, null, null, scope);
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java
index b113dec80f..3846ed5daf 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java
@@ -295,6 +295,12 @@ void validateWindow(
SqlValidatorScope scope,
@Nullable SqlCall call);
+ /** Returns whether the validator is currently validating within a window
+ * expression. */
+ default boolean isInWindow() {
+ return false;
+ }
+
/**
* Validates a MATCH_RECOGNIZE clause.
*
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 6881bae035..63f20d5941 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
@@ -6210,6 +6210,10 @@ public void setOriginal(SqlNode expr, SqlNode original) {
return new FieldNamespace(this, field.getType());
}
+ @Override public boolean isInWindow() {
+ return inWindow;
+ }
+
@Override public void validateWindow(
SqlNode windowOrId,
SqlValidatorScope scope,
@@ -6832,7 +6836,6 @@ private SqlNode navigationInDefine(SqlNode node, String
alpha) {
break;
case 2:
assert op.allowsNullTreatment();
- assert op.requiresOver();
assert op.requiresGroupOrder() == Optionality.FORBIDDEN;
break;
default:
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index e527029749..0f5959fb57 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -10067,8 +10067,7 @@ private RelOptFixture spatial(String sql) {
*/
@Test void testReduceAggregateFunctionsByGroup() {
final String sql = "select sal, max(sal) as sal_max, min(sal) as
sal_min,\n"
- + "avg(sal) sal_avg, any_value(sal) as sal_val, first_value(sal) as
sal_first,\n"
- + "last_value(sal) as sal_last\n"
+ + "avg(sal) sal_avg, any_value(sal) as sal_val\n"
+ "from emp group by sal, deptno";
sql(sql).withRule(CoreRules.AGGREGATE_REDUCE_FUNCTIONS,
CoreRules.PROJECT_MERGE).check();
}
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 20602e8d75..36ef279b53 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -3374,7 +3374,10 @@ void testWinPartClause() {
* <a href="https://issues.apache.org/jira/browse/CALCITE-820";>[CALCITE-820]
* Validate that window functions have OVER clause</a>, and
* <a
href="https://issues.apache.org/jira/browse/CALCITE-1340";>[CALCITE-1340]
- * Window aggregates give invalid errors</a>. */
+ * Window aggregates give invalid errors</a> and
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7485";>[CALCITE-7485]
+ * FIRST_VALUE/LAST_VALUE should only be defined for window aggregates</a>.
+ * */
@Test void testWindowFunctionsWithoutOver() {
winSql("select sum(empno)\n"
+ "from emp\n"
@@ -3397,6 +3400,37 @@ void testWinPartClause() {
winSql("select ^nth_value(sal, 2)^\n"
+ "from emp")
.fails("OVER clause is necessary for window functions");
+
+ winSql("select ^first_value(sal)^\n"
+ + "from emp")
+ .fails("OVER clause is necessary for window functions");
+
+ winSql("select ^last_value(sal)^\n"
+ + "from emp")
+ .fails("OVER clause is necessary for window functions");
+
+ // With alias, first_value and last_value without OVER should also fail
+ winSql("select ^first_value(sal)^ as sal_first\n"
+ + "from emp")
+ .fails("OVER clause is necessary for window functions");
+
+ winSql("select ^last_value(sal)^ as sal_last\n"
+ + "from emp")
+ .fails("OVER clause is necessary for window functions");
+
+ // In GROUP BY context, first_value and last_value without OVER should fail
+ winSql("select sal, ^first_value(sal)^ as sal_first\n"
+ + "from emp group by sal, deptno")
+ .fails("OVER clause is necessary for window functions");
+
+ // first_value and last_value with OVER clause should succeed
+ winSql("select first_value(sal) over (order by empno)\n"
+ + "from emp")
+ .ok();
+
+ winSql("select last_value(sal) over (order by empno)\n"
+ + "from emp")
+ .ok();
}
@Test void testOverInPartitionBy() {
diff --git
a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index cf66384c12..fc60b88649 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -15968,21 +15968,20 @@ LogicalProject(EXPR$0=[1], A=[$1])
<TestCase name="testReduceAggregateFunctionsByGroup">
<Resource name="sql">
<![CDATA[select sal, max(sal) as sal_max, min(sal) as sal_min,
-avg(sal) sal_avg, any_value(sal) as sal_val, first_value(sal) as sal_first,
-last_value(sal) as sal_last
+avg(sal) sal_avg, any_value(sal) as sal_val
from emp group by sal, deptno]]>
</Resource>
<Resource name="planBefore">
<![CDATA[
-LogicalProject(SAL=[$0], SAL_MAX=[$2], SAL_MIN=[$3], SAL_AVG=[$4],
SAL_VAL=[$5], SAL_FIRST=[$6], SAL_LAST=[$7])
- LogicalAggregate(group=[{0, 1}], SAL_MAX=[MAX($0)], SAL_MIN=[MIN($0)],
SAL_AVG=[AVG($0)], SAL_VAL=[ANY_VALUE($0)], SAL_FIRST=[FIRST_VALUE($0)],
SAL_LAST=[LAST_VALUE($0)])
+LogicalProject(SAL=[$0], SAL_MAX=[$2], SAL_MIN=[$3], SAL_AVG=[$4],
SAL_VAL=[$5])
+ LogicalAggregate(group=[{0, 1}], SAL_MAX=[MAX($0)], SAL_MIN=[MIN($0)],
SAL_AVG=[AVG($0)], SAL_VAL=[ANY_VALUE($0)])
LogicalProject(SAL=[$5], DEPTNO=[$7])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
<Resource name="planAfter">
<![CDATA[
-LogicalProject(SAL=[$0], SAL_MAX=[$0], SAL_MIN=[$0], SAL_AVG=[$0],
SAL_VAL=[$0], SAL_FIRST=[$0], SAL_LAST=[$0])
+LogicalProject(SAL=[$0], SAL_MAX=[$0], SAL_MIN=[$0], SAL_AVG=[$0],
SAL_VAL=[$0])
LogicalAggregate(group=[{0, 1}])
LogicalProject(SAL=[$5], DEPTNO=[$7])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index a6d2eb6f0d..cf2792c5a6 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -15836,30 +15836,38 @@ void testTimestampDiff(boolean coercionEnabled) {
+ "between 0 and 1", false);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7485";>[CALCITE-7485]
+ * FIRST_VALUE/LAST_VALUE should only be defined for window aggregates</a>.
+ */
@Test void testPercentileContBigQueryFunc() {
final SqlOperatorFixture f = fixture()
.setFor(SqlLibraryOperators.PERCENTILE_CONT2,
SqlOperatorFixture.VmName.EXPAND)
.withLibrary(SqlLibrary.BIG_QUERY);
- f.checkType("percentile_cont(1, .5)",
+ f.checkType("percentile_cont(1, .5) over ()",
"DOUBLE NOT NULL");
- f.checkType("percentile_cont(.5, .5 RESPECT NULLS)", "DOUBLE NOT NULL");
- f.checkType("percentile_cont(1, .5 IGNORE NULLS)", "DOUBLE NOT NULL");
- f.checkType("percentile_cont(2+3, .5 IGNORE NULLS)", "DOUBLE NOT NULL");
- f.checkFails("^percentile_cont(1, 1.5)^",
+ f.checkType("percentile_cont(.5, .5 RESPECT NULLS) over ()", "DOUBLE NOT
NULL");
+ f.checkType("percentile_cont(1, .5 IGNORE NULLS) over ()", "DOUBLE NOT
NULL");
+ f.checkType("percentile_cont(2+3, .5 IGNORE NULLS) over ()", "DOUBLE NOT
NULL");
+ f.checkFails("^percentile_cont(1, 1.5)^ over ()",
"Argument to function 'PERCENTILE_CONT' must be a numeric literal "
+ "between 0 and 1", false);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7485";>[CALCITE-7485]
+ * FIRST_VALUE/LAST_VALUE should only be defined for window aggregates</a>.
+ */
@Test void testPercentileDiscBigQueryFunc() {
final SqlOperatorFixture f = fixture()
.setFor(SqlLibraryOperators.PERCENTILE_DISC2,
SqlOperatorFixture.VmName.EXPAND)
.withLibrary(SqlLibrary.BIG_QUERY);
- f.checkType("percentile_disc(1, .5)",
+ f.checkType("percentile_disc(1, .5) over ()",
"INTEGER NOT NULL");
- f.checkType("percentile_disc(1, .5 RESPECT NULLS)", "INTEGER NOT NULL");
- f.checkType("percentile_disc(0.75, .5 IGNORE NULLS)", "DECIMAL(3, 2) NOT
NULL");
- f.checkType("percentile_disc(2+3, .5 IGNORE NULLS)", "INTEGER NOT NULL");
- f.checkFails("^percentile_disc(1, 1.5)^",
+ f.checkType("percentile_disc(1, .5 RESPECT NULLS) over ()", "INTEGER NOT
NULL");
+ f.checkType("percentile_disc(0.75, .5 IGNORE NULLS) over ()", "DECIMAL(3,
2) NOT NULL");
+ f.checkType("percentile_disc(2+3, .5 IGNORE NULLS) over ()", "INTEGER NOT
NULL");
+ f.checkFails("^percentile_disc(1, 1.5)^ over ()",
"Argument to function 'PERCENTILE_DISC' must be a numeric literal "
+ "between 0 and 1", false);
}
