diff --git a/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java b/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java index 9a93718c38b..43ccc3453b1 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java @@ -106,7 +106,7 @@ boolean isMeasureExp(SqlNode e) { return null; } - if (!validator.config().nakedMeasures() + if (!validator.config().nakedMeasuresInAggregateQuery() && isMeasureExp(id)) { SqlNode originalExpr = validator.getOriginal(id); throw validator.newValidationError(originalExpr, 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 80d1734d9c2..2f27837b767 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 @@ -912,15 +912,39 @@ interface Config { */ Config withLenientOperatorLookup(boolean lenient); - /** Returns whether the validator allows measures to be used without the - * AGGREGATE function. Default is true. */ - @Value.Default default boolean nakedMeasures() { + /** Returns whether the validator allows measures to be used without + * AGGREGATE function in a non-aggregate query. Default is true. + */ + @Value.Default default boolean nakedMeasuresInNonAggregateQuery() { return true; } + /** Sets whether the validator allows measures to be used without AGGREGATE + * function in a non-aggregate query. + */ + Config withNakedMeasuresInNonAggregateQuery(boolean value); + + /** Returns whether the validator allows measures to be used without + * AGGREGATE function in an aggregate query. Default is true. + */ + @Value.Default default boolean nakedMeasuresInAggregateQuery() { + return true; + } + + /** Sets whether the validator allows measures to be used without AGGREGATE + * function in an aggregate query. + */ + Config withNakedMeasuresInAggregateQuery(boolean value); + /** Sets whether the validator allows measures to be used without the - * AGGREGATE function. */ - Config withNakedMeasures(boolean nakedMeasures); + * AGGREGATE function inside or outside aggregate queries. + * Deprecated: use the inside / outside variants instead. + */ + @Deprecated // to be removed before 1.38 + default Config withNakedMeasures(boolean nakedMeasures) { + return withNakedMeasuresInAggregateQuery(nakedMeasures) + .withNakedMeasuresInNonAggregateQuery(nakedMeasures); + } /** Returns whether the validator supports implicit type coercion. */ @Value.Default default boolean typeCoercionEnabled() { 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 9572cf4ebd9..a0e970fadbd 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 @@ -2852,7 +2852,7 @@ private void registerQuery( } } - // If this is an aggregating query, the SELECT list and HAVING + // If this is an aggregate query, the SELECT list and HAVING // clause use a different scope, where you can only reference // columns which are in the GROUP BY clause. SqlValidatorScope aggScope = selectScope; @@ -2888,7 +2888,7 @@ private void registerQuery( registerSubQueries(orderScope, orderList); if (!isAggregate(select)) { - // Since this is not an aggregating query, + // Since this is not an aggregate query, // there cannot be any aggregates in the ORDER BY clause. SqlNode agg = aggFinder.findAgg(orderList); if (agg != null) { @@ -4826,10 +4826,10 @@ private void validateExpr(SqlNode expr, SqlValidatorScope scope) { } } - // Unless 'naked measures' are enabled, a non-aggregating query cannot - // reference measure columns. (An aggregating query can use them as + // Unless 'naked measures' are enabled, a non-aggregate query cannot + // reference measure columns. (An aggregate query can use them as // argument to the AGGREGATE function.) - if (!config.nakedMeasures() + if (!config.nakedMeasuresInNonAggregateQuery() && !(scope instanceof AggregatingScope) && scope.isMeasureRef(expr)) { throw newValidationError(expr, 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 11c2edf7a7a..192b65fef50 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -2866,7 +2866,7 @@ void testWinPartClause() { .fails("OVER clause is necessary for window functions"); // With [CALCITE-1340], the validator would see RANK without OVER, - // mistakenly think this is an aggregating query, and wrongly complain + // mistakenly think this is an aggregate query, and wrongly complain // about the PARTITION BY: "Expression 'DEPTNO' is not being grouped" winSql("select cume_dist() over w , ^rank()^\n" + "from emp\n" @@ -3874,8 +3874,19 @@ private void checkNegWindow(String s, String msg) { SqlValidatorFixture f = fixture().withExtendedCatalog() .withOperatorTable(operatorTableFor(SqlLibrary.CALCITE)); - SqlValidatorFixture f2 = - f.withValidatorConfig(c -> c.withNakedMeasures(false)); + + SqlValidatorFixture fNakedInsideOnly = + f.withValidatorConfig(c -> + c.withNakedMeasuresInAggregateQuery(true) + .withNakedMeasuresInNonAggregateQuery(false)); + SqlValidatorFixture fNakedOutsideOnly = + f.withValidatorConfig(c -> + c.withNakedMeasuresInNonAggregateQuery(true) + .withNakedMeasuresInAggregateQuery(false)); + SqlValidatorFixture fNoNakedMeasures = + f.withValidatorConfig(c -> + c.withNakedMeasuresInNonAggregateQuery(false) + .withNakedMeasuresInAggregateQuery(false)); final String measureIllegal = "Measure expressions can only occur within AGGREGATE function"; @@ -3890,28 +3901,36 @@ private void checkNegWindow(String s, String msg) { .ok(); // Same SQL is invalid if naked measures are not enabled - f2.withSql(sql0).fails(measureIllegal); + fNoNakedMeasures.withSql(sql0).fails(measureIllegal); + fNakedOutsideOnly.withSql(sql0).fails(measureIllegal); + fNakedInsideOnly.withSql(sql0).isAggregate(is(true)).ok(); // Similarly, with alias final String sql1b = "select deptno, ^count_plus_100^ as x\n" + "from empm\n" + "group by deptno"; f.withSql(sql1b).isAggregate(is(true)).ok(); - f2.withSql(sql1b).fails(measureIllegal); + fNoNakedMeasures.withSql(sql1b).fails(measureIllegal); + fNakedOutsideOnly.withSql(sql1b).fails(measureIllegal); + fNakedInsideOnly.withSql(sql1b).isAggregate(is(true)).ok(); // Similarly, in an expression final String sql1c = "select deptno, deptno + ^count_plus_100^ * 2 as x\n" + "from empm\n" + "group by deptno"; f.withSql(sql1c).isAggregate(is(true)).ok(); - f2.withSql(sql1c).fails(measureIllegal); + fNoNakedMeasures.withSql(sql1c).fails(measureIllegal); + fNakedOutsideOnly.withSql(sql1c).fails(measureIllegal); + fNakedInsideOnly.withSql(sql1c).isAggregate(is(true)).ok(); // Similarly, for a query that is an aggregate query because of another // aggregate function. final String sql1 = "select count(*), ^count_plus_100^\n" + "from empm"; f.withSql(sql1).isAggregate(is(true)).ok(); - f2.withSql(sql1).fails(measureIllegal); + fNoNakedMeasures.withSql(sql1).fails(measureIllegal); + fNakedInsideOnly.withSql(sql1).isAggregate(is(true)).ok(); + fNakedOutsideOnly.withSql(sql1).fails(measureIllegal); // A measure in a non-aggregate query. // Using a measure should not make it an aggregate query. @@ -3924,7 +3943,9 @@ private void checkNegWindow(String s, String msg) { + "MEASURE NOT NULL COUNT_PLUS_100, " + "VARCHAR(20) NOT NULL ENAME) NOT NULL") .isAggregate(is(false)); - f2.withSql(sql2).fails(measureIllegal2); + fNoNakedMeasures.withSql(sql2).fails(measureIllegal2); + fNakedInsideOnly.withSql(sql2).fails(measureIllegal2); + fNakedOutsideOnly.withSql(sql2).isAggregate(is(false)).ok(); // as above, wrapping the measure in AGGREGATE final String sql3 = "select deptno, aggregate(count_plus_100) as x, ename\n"