This is an automated email from the ASF dual-hosted git repository.
tanner 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 14ade3a720 [CALCITE-6337] Distinguish naked measure support between
inside and outside aggregation
14ade3a720 is described below
commit 14ade3a72032986731239068963e4716dc7fe6af
Author: Barry Kelly <[email protected]>
AuthorDate: Mon Nov 20 15:59:46 2023 +0000
[CALCITE-6337] Distinguish naked measure support between inside and outside
aggregation
Split nakedMeasures flag into two flags:
- nakedMeasuresInsideAggregatingQuery : allowed in group-by queries
- nakedMeasuresOutsideAggregatingQuery : allowed in non-group-by queries
The existing nakedMeasures flag is marked as deprecated. Its functionality
is equivalent to setting the above two flags to the same value.
---
.../apache/calcite/sql/validate/AggChecker.java | 2 +-
.../apache/calcite/sql/validate/SqlValidator.java | 34 +++++++++++++++++---
.../calcite/sql/validate/SqlValidatorImpl.java | 10 +++---
.../org/apache/calcite/test/SqlValidatorTest.java | 37 +++++++++++++++++-----
4 files changed, 64 insertions(+), 19 deletions(-)
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 9a93718c38..43ccc3453b 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 @@ class AggChecker extends SqlBasicVisitor<Void> {
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 80d1734d9c..2f27837b76 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 @@ public interface SqlValidator {
*/
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 9572cf4ebd..a0e970fadb 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 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
}
}
- // 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 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
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 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
}
}
- // 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 11c2edf7a7..192b65fef5 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 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
.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 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
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 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
.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 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
+ "MEASURE<INTEGER NOT NULL> 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"
