This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 06d6079e49f Stop the single-stage engine from silently ignoring the
HAVING clause (#19554)
06d6079e49f is described below
commit 06d6079e49fbf726c55136eaed54b603cc2d2002
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Thu Sep 17 02:54:48 2026 +0200
Stop the single-stage engine from silently ignoring the HAVING clause
(#19554)
* Stop the single-stage engine from silently ignoring the HAVING clause
The single-stage engine evaluates a HAVING filter only while reducing a
GROUP BY aggregation. Every other shape answered as if the clause were
absent, so the predicate was discarded with no error and no warning:
SELECT city FROM t GROUP BY city HAVING city > 'B' -- every city
SELECT COUNT(*) FROM t HAVING COUNT(*) > 100 -- the count anyway
SELECT city FROM t HAVING city > 'B' -- every row
A third group of shapes failed instead with an internal error naming a
clause the user did not write ("Failed to find SELECT expression: city in
the GROUP-BY clause").
The multi-stage engine answers all of these correctly or rejects them, so
the two engines disagreed on the same query.
Three changes, each where the predicate was lost:
CalciteSqlParser.validateHavingClause() rejects what the engine cannot
evaluate. A HAVING clause imposes grouping semantics: without a GROUP BY
the whole table becomes a single group, so every expression in HAVING, and
in the SELECT list of a query with no GROUP BY, must be an aggregation, a
literal, or functionally dependent on the GROUP BY columns. This is the
rule Calcite applies for the multi-stage engine ("Expression 'x' is not
being grouped"), so both engines now reject the same queries. Validation
lives in validate() rather than in a new QueryRewriter because
QueryRewriterFactory.init() replaces the rewriter list wholesale, so an
operator with a custom list would not get the check.
NonAggregationGroupByToDistinctQueryRewriter leaves a query carrying a
HAVING clause alone. The rewrite drops the GROUP BY list, and DISTINCT has
no reduce step that evaluates a HAVING filter, so the predicate used to
disappear there. Keeping the GROUP BY puts the query back on the reduce
path that does evaluate it, which also fixes the internal errors above.
A GROUP BY with no aggregation anywhere is rejected rather than answered.
Moving the predicate into WHERE would be equivalent for a single-valued
grouping column, but not for a multi-valued one: GROUP BY builds one group
per value while WHERE keeps whole rows, so a row whose values straddle the
predicate would contribute values the predicate excludes. The rewrite runs
before any schema is available, so the two cases cannot be told apart. The
error names the WHERE rewrite and the multi-stage engine.
AggregationDataTableReducer applies the filter to the single group of an
aggregation without GROUP BY. Null awareness is requested unconditionally
rather than from requiresNullAwareKeyEvaluation(): that flag only reports
the query's null-handling option, but this reducer materializes a null
final result either way, because an aggregation over an empty whole-table
group has no value to report. The flag gates the "a null never matches"
early return in PredicateRowMatcher, so without it a null would be unboxed
and throw. The result rewriters run even when the row is filtered away,
since they may replace the DataSchema and the schema must not depend on
the data.
QueryValidationTest's GROOVY-in-HAVING fixture used a shape that is now
rejected; it is rewritten as a legal aggregate query, and compilation is
moved inside the try so a compile failure is reported rather than thrown.
* Reject a HAVING predicate that references an ungrouped column next to an
aggregate
validateHavingClause() used expressionOutsideGroupByList(), which accepts an
expression as soon as it contains an aggregation anywhere. That is too
permissive for HAVING: in
SELECT COUNT(*) FROM t HAVING COUNT(*) > amount
SELECT city FROM t GROUP BY city HAVING COUNT(*) > amount
the predicate contains COUNT(*), so it passed validation, but `amount` still
has no single value per group. The query then failed during broker reduce
with
"Failed to find SELECT expression: amount in the GROUP-BY clause" -- an
internal error naming a clause the user did not write, and in the first case
naming a GROUP BY clause the query does not have.
findUngroupedReference() walks the predicate and stops at an aggregation
instead of short-circuiting on one, so an identifier is accepted only when
it
is a grouping column or an argument of an aggregation. It returns the
offending
expression so the message names the column rather than the whole predicate.
A filtered aggregation is resolved like any other aggregation, and its
FILTER
predicate is evaluated per row while aggregating, so it may reference
columns
that are not grouped and is not descended into.
---------
Co-authored-by: Gonzalo Ortiz <[email protected]>
---
.../broker/requesthandler/QueryValidationTest.java | 8 +-
.../apache/pinot/sql/parsers/CalciteSqlParser.java | 109 ++++++++++++
...nAggregationGroupByToDistinctQueryRewriter.java | 7 +
.../query/reduce/AggregationDataTableReducer.java | 26 ++-
.../apache/pinot/queries/HavingQueriesTest.java | 186 +++++++++++++++++++++
5 files changed, 329 insertions(+), 7 deletions(-)
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
index f3ebe5a647b..9512ebf8e15 100644
---
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
@@ -111,9 +111,10 @@ public class QueryValidationTest {
testValidateGroovyQuery(
"SELECT COUNT(colA) FROM bar GROUP BY
GROOVY('{\"returnType\":\"STRING\",\"isSingleValue\":true}', "
+ "'arg0 + arg1', colA, colB)", true);
+ // HAVING imposes grouping semantics, so both the predicate and the SELECT
list have to be aggregated.
testValidateGroovyQuery(
- "SELECT foo FROM bar HAVING
GROOVY('{\"returnType\":\"STRING\",\"isSingleValue\":true}', 'arg0 + arg1',
colA,"
- + " colB) = 'foobarval'", true);
+ "SELECT COUNT(*) FROM bar HAVING
GROOVY('{\"returnType\":\"STRING\",\"isSingleValue\":true}', "
+ + "'arg0 + arg1', MAX(colA), MIN(colB)) = 'foobarval'", true);
testValidateGroovyQuery("SELECT foo FROM bar", false);
}
@@ -167,9 +168,8 @@ public class QueryValidationTest {
}
private void testValidateGroovyQuery(String query, boolean
queryContainsGroovy) {
- PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
-
try {
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
BaseSingleStageBrokerRequestHandler.validateGroovyScript(pinotQuery,
queryContainsGroovy);
if (queryContainsGroovy) {
fail("Query should have failed since groovy was found in query: " +
pinotQuery);
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index ed16a9d85ad..d50f0f1e8dd 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -247,6 +247,7 @@ public class CalciteSqlParser {
throws SqlCompilationException {
boolean hasGroupByClause = pinotQuery.getGroupByList() != null;
Set<Expression> groupByExprs = hasGroupByClause ? new
HashSet<>(pinotQuery.getGroupByList()) : null;
+ validateHavingClause(pinotQuery, hasGroupByClause, groupByExprs);
int aggregateExprCount = 0;
for (Expression selectExpression : pinotQuery.getSelectList()) {
if (isAggregateExpression(selectExpression)) {
@@ -301,6 +302,114 @@ public class CalciteSqlParser {
}
}
+ /// Rejects a HAVING clause that the single-stage engine would otherwise
drop.
+ ///
+ /// HAVING is evaluated only while reducing a GROUP BY aggregation. Applied
to any other shape the predicate used to
+ /// be discarded silently, so the query answered as if the clause were
absent.
+ ///
+ /// A HAVING clause imposes grouping semantics: without a GROUP BY the whole
table becomes a single group. Every
+ /// expression in HAVING, and in the SELECT list of a query with no GROUP
BY, must therefore be an aggregation, a
+ /// literal, or functionally dependent on the GROUP BY columns -- the rule
the multi-stage engine applies through
+ /// Calcite ("Expression 'x' is not being grouped").
+ ///
+ /// A GROUP BY carrying no aggregation anywhere is rejected on top of that
rule. The engine has no grouping operator
+ /// for such a query:
[org.apache.pinot.sql.parsers.rewriter.NonAggregationGroupByToDistinctQueryRewriter]
turns it
+ /// into a DISTINCT, which has no reduce step that can evaluate a HAVING
filter. Moving the predicate into WHERE
+ /// would be equivalent for a single-valued grouping column, but not for a
multi-valued one -- GROUP BY builds one
+ /// group per value while WHERE keeps whole rows -- and the rewriter has no
schema to tell them apart. The
+ /// multi-stage engine does support this shape.
+ private static void validateHavingClause(PinotQuery pinotQuery, boolean
hasGroupByClause,
+ @Nullable Set<Expression> groupByExprs)
+ throws SqlCompilationException {
+ Expression havingExpression = pinotQuery.getHavingExpression();
+ if (havingExpression == null) {
+ return;
+ }
+ Set<Expression> groupedExprs = hasGroupByClause ? groupByExprs : Set.of();
+ Expression ungrouped = findUngroupedReference(havingExpression,
groupedExprs);
+ if (ungrouped != null) {
+ throw new SqlCompilationException("'" +
RequestUtils.prettyPrint(ungrouped) + "' in HAVING clause must "
+ + (hasGroupByClause ? "be inside an aggregate or functionally
dependent on the columns used in GROUP BY "
+ + "clause." : "be inside an aggregate: with no GROUP BY clause
the whole table is a single group."));
+ }
+ if (!hasGroupByClause) {
+ for (Expression selectExpression : pinotQuery.getSelectList()) {
+ Expression ungroupedSelect = findUngroupedReference(selectExpression,
groupedExprs);
+ if (ungroupedSelect != null) {
+ throw new SqlCompilationException("'" +
RequestUtils.prettyPrint(ungroupedSelect) + "' must be inside an "
+ + "aggregate: with a HAVING clause and no GROUP BY clause the
whole table is a single group.");
+ }
+ }
+ return;
+ }
+ if (!hasAggregation(pinotQuery)) {
+ throw new SqlCompilationException("HAVING is not supported on a GROUP BY
query without an aggregation in the "
+ + "single-stage query engine. Move the predicate to the WHERE
clause, or use the multi-stage query engine.");
+ }
+ }
+
+ /// Returns the first identifier the engine cannot resolve once rows have
been grouped: one that is neither a
+ /// grouping column nor an argument of an aggregation. Returns `null` when
every reference is resolvable.
+ ///
+ /// This deliberately differs from [#expressionOutsideGroupByList], which
accepts an expression as soon as it
+ /// *contains* an aggregation anywhere. That is too permissive for HAVING:
`HAVING COUNT(*) > amount` contains
+ /// COUNT(*), but `amount` still has no single value per group, and the
reducer fails on it at run time with a
+ /// message naming the GROUP BY clause the user did not write.
+ @Nullable
+ private static Expression findUngroupedReference(Expression expr,
Set<Expression> groupByExprs) {
+ if (expr.getType() == ExpressionType.LITERAL ||
groupByExprs.contains(expr)) {
+ return null;
+ }
+ Function function = expr.getFunctionCall();
+ if (function == null) {
+ // An identifier that is not a grouping column.
+ return expr;
+ }
+ if (AggregationFunctionType.isAggregationFunction(function.getOperator()))
{
+ // Arguments are aggregated away, so they do not have to be grouping
columns.
+ return null;
+ }
+ if (function.getOperator().equalsIgnoreCase(SqlKind.FILTER.lowerName)) {
+ // A filtered aggregation, COUNT(*) FILTER (WHERE ...). The engine
resolves it like any other aggregation, and
+ // its predicate is evaluated per row while aggregating, so it may
reference columns that are not grouped.
+ return null;
+ }
+ List<Expression> operands = function.getOperands();
+ if (operands == null) {
+ return null;
+ }
+ // For an alias only the aliased value matters; the alias itself is not a
column reference.
+ List<Expression> toCheck = function.getOperator().equals("as") ?
operands.subList(0, 1) : operands;
+ for (Expression operand : toCheck) {
+ Expression ungrouped = findUngroupedReference(operand, groupByExprs);
+ if (ungrouped != null) {
+ return ungrouped;
+ }
+ }
+ return null;
+ }
+
+ /// Returns `true` if an aggregation appears anywhere the engine would
compute one: the SELECT list, the HAVING
+ /// clause or the ORDER-BY list.
+ private static boolean hasAggregation(PinotQuery pinotQuery) {
+ for (Expression selectExpression : pinotQuery.getSelectList()) {
+ if (isAggregateExpression(selectExpression)) {
+ return true;
+ }
+ }
+ if (pinotQuery.getHavingExpression() != null &&
isAggregateExpression(pinotQuery.getHavingExpression())) {
+ return true;
+ }
+ if (pinotQuery.getOrderByList() != null) {
+ for (Expression orderByExpression : pinotQuery.getOrderByList()) {
+ if (isAggregateExpression(orderByExpression)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
/// Recursively rejects GROUPING() / GROUPING_ID() calls with more than
/// [GroupingSets#MAX_GROUPING_FUNCTION_ARGS] arguments (the packed INT bit
width).
private static void validateGroupingFunctionArgs(Expression expression) {
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
index c318341c2ab..6bbb5d576bd 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
@@ -55,6 +55,13 @@ public class NonAggregationGroupByToDistinctQueryRewriter
implements QueryRewrit
if (pinotQuery.getGroupingSets() != null) {
return pinotQuery;
}
+ // DISTINCT has no reduce step that evaluates a HAVING filter, and the
rewrite drops the GROUP BY list the filter
+ // is validated against, so a query carrying one is left alone.
CalciteSqlParser.validateHavingClause() then
+ // either accepts it -- the aggregation keeps it on the GROUP BY reduce
path, which does evaluate HAVING -- or
+ // rejects it, rather than letting the predicate disappear here.
+ if (pinotQuery.getHavingExpression() != null) {
+ return pinotQuery;
+ }
for (Expression select : pinotQuery.getSelectList()) {
if (CalciteSqlParser.isAggregateExpression(select)) {
return pinotQuery;
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/AggregationDataTableReducer.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/AggregationDataTableReducer.java
index 96f53690f5d..5c8143975f9 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/AggregationDataTableReducer.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/AggregationDataTableReducer.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Map;
import org.apache.pinot.common.datatable.DataTable;
import org.apache.pinot.common.metrics.BrokerMetrics;
+import org.apache.pinot.common.request.context.FilterContext;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
import org.apache.pinot.common.response.broker.ResultTable;
import org.apache.pinot.common.utils.DataSchema;
@@ -211,10 +212,29 @@ public class AggregationDataTableReducer implements
DataTableReducer {
private ResultTable reduceToResultTable(DataSchema dataSchema, Object[]
finalResults) {
PostAggregationHandler postAggregationHandler = new
PostAggregationHandler(_queryContext, dataSchema);
DataSchema resultDataSchema = postAggregationHandler.getResultDataSchema();
- Object[] row = postAggregationHandler.getResult(finalResults);
- RewriterResult resultRewriterResult =
- ResultRewriteUtils.rewriteResult(resultDataSchema,
List.<Object[]>of(row));
+ // An aggregation without GROUP BY produces a single group covering the
whole table, and HAVING filters that
+ // group away or keeps it. The predicate is evaluated on the row before
post-aggregation, the same way
+ // GroupByDataTableReducer does it.
+ //
+ // Null awareness is requested unconditionally rather than from
requiresNullAwareKeyEvaluation(): that flag only
+ // reports the query's null-handling option, but this reducer materializes
a null final result whichever way the
+ // option is set, because an aggregation over an empty whole-table group
has no value to report. The flag only
+ // gates the "a null never matches" early return in PredicateRowMatcher,
so without it a null result would be
+ // unboxed and throw.
+ List<Object[]> matchedRows;
+ FilterContext havingFilter = _queryContext.getHavingFilter();
+ if (havingFilter != null && !new HavingFilterHandler(havingFilter,
postAggregationHandler, true).isMatch(
+ finalResults)) {
+ matchedRows = List.of();
+ } else {
+ matchedRows =
List.<Object[]>of(postAggregationHandler.getResult(finalResults));
+ }
+
+ // The result rewriters may replace the DataSchema
(ParentAggregationResultRewriter drops the internal parent
+ // columns), so they have to run even when HAVING filtered the single row
away. Otherwise the same query would
+ // report a different schema depending on its data.
+ RewriterResult resultRewriterResult =
ResultRewriteUtils.rewriteResult(resultDataSchema, matchedRows);
resultDataSchema = resultRewriterResult.getDataSchema();
List<Object[]> rows = resultRewriterResult.getRows();
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/HavingQueriesTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/HavingQueriesTest.java
new file mode 100644
index 00000000000..81b2f06f03c
--- /dev/null
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/HavingQueriesTest.java
@@ -0,0 +1,186 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.sql.parsers.SqlCompilationException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+
+
+/// End-to-end tests for the HAVING clause in the single-stage engine.
+///
+/// The engine used to evaluate a HAVING filter only when reducing a GROUP BY
aggregation. Every other shape answered
+/// as if the clause were absent, so these queries returned rows that the
predicate excludes. Each expectation here
+/// matches what the multi-stage engine produces for the same query.
+public class HavingQueriesTest {
+ private static final TableConfig TABLE_CONFIG =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ // Columns are read positionally in schema order, which sorts "amount"
before "city".
+ private static final Schema SCHEMA = new
Schema.SchemaBuilder().setSchemaName("testTable")
+ .addMetric("amount", FieldSpec.DataType.INT, 0)
+ .addSingleValueDimension("city", FieldSpec.DataType.STRING)
+ .build();
+
+ /// Same shape but with a nullable metric, for the null-aware predicate path.
+ private static final Schema NULLABLE_SCHEMA = new
Schema.SchemaBuilder().setSchemaName("testTable")
+ .setEnableColumnBasedNullHandling(true)
+ .addMetric("amount", FieldSpec.DataType.INT, 0)
+ .addSingleValueDimension("city", FieldSpec.DataType.STRING)
+ .build();
+
+ private File _baseDir;
+
+ @BeforeClass
+ void createBaseDir()
+ throws IOException {
+ _baseDir = Files.createTempDirectory(getClass().getSimpleName()).toFile();
+ }
+
+ @AfterClass
+ void destroyBaseDir()
+ throws IOException {
+ if (_baseDir != null) {
+ FileUtils.deleteDirectory(_baseDir);
+ }
+ }
+
+ /// Each instance holds the same three rows, so the queried table has Athens
twice over and Madrid twice over:
+ /// Athens has 4 rows summing to 6, Madrid has 2 rows summing to 20.
+ private FluentQueryTest.OnSecondInstance givenTable() {
+ return FluentQueryTest.withBaseDir(_baseDir).givenTable(SCHEMA,
TABLE_CONFIG)
+ .onFirstInstance(new Object[]{1, "Athens"}, new Object[]{2, "Athens"},
new Object[]{10, "Madrid"})
+ .andOnSecondInstance(new Object[]{1, "Athens"}, new Object[]{2,
"Athens"}, new Object[]{10, "Madrid"});
+ }
+
+ /// An aggregation without GROUP BY is a single group over the whole table.
When the predicate rejects that group the
+ /// result is empty; the engine used to return the unfiltered aggregate.
+ @Test
+ public void testHavingOnAggregationWithoutGroupBy() {
+ givenTable().whenQuery("SELECT COUNT(*) FROM testTable HAVING COUNT(*) >
100").thenResultIs(new Object[0][]);
+ givenTable().whenQuery("SELECT COUNT(*) FROM testTable HAVING COUNT(*) >
1")
+ .thenResultIs(new Object[]{6L});
+ givenTable().whenQuery("SELECT SUM(amount) FROM testTable HAVING
SUM(amount) > 100").thenResultIs(new Object[0][]);
+ givenTable().whenQuery("SELECT SUM(amount) FROM testTable HAVING
SUM(amount) > 1")
+ .thenResultIs(new Object[]{26.0});
+ // The predicate may reference an aggregate that is not in the SELECT list.
+ givenTable().whenQuery("SELECT COUNT(*) FROM testTable HAVING SUM(amount)
> 100").thenResultIs(new Object[0][]);
+ }
+
+ /// A GROUP BY whose SELECT list holds no aggregate is rewritten to a
DISTINCT, which has no reduce step that can
+ /// evaluate a HAVING filter, so the predicate used to be dropped. An
aggregate predicate keeps the query on the
+ /// GROUP BY reduce path instead.
+ @Test
+ public void testHavingOnGroupByWithoutAggregateInSelectList() {
+ givenTable().whenQuery("SELECT city FROM testTable GROUP BY city HAVING
COUNT(*) > 2")
+ .thenResultIs(new Object[]{"Athens"});
+ givenTable().whenQuery("SELECT city FROM testTable GROUP BY city HAVING
SUM(amount) > 10")
+ .thenResultIs(new Object[]{"Madrid"});
+ }
+
+ /// The shape that always worked must keep working.
+ @Test
+ public void testHavingOnGroupByWithAggregateInSelectList() {
+ givenTable().whenQuery("SELECT city, COUNT(*) FROM testTable GROUP BY city
HAVING COUNT(*) > 2")
+ .thenResultIs(new Object[]{"Athens", 4L});
+ }
+
+ /// A HAVING clause imposes grouping semantics, so a predicate on a
non-grouped column has no single value to test
+ /// and a bare SELECT column has no single value to report. The multi-stage
engine rejects all of these too.
+ @Test
+ public void testInvalidHavingIsRejected() {
+ assertRejected("SELECT city FROM testTable HAVING city > 'B'");
+ assertRejected("SELECT DISTINCT city FROM testTable HAVING city > 'B'");
+ assertRejected("SELECT city FROM testTable HAVING COUNT(*) > 1");
+ assertRejected("SELECT DISTINCT city FROM testTable HAVING COUNT(*) > 1");
+ assertRejected("SELECT COUNT(*) FROM testTable HAVING amount > 1");
+ assertRejected("SELECT city FROM testTable GROUP BY city HAVING amount >
1");
+ // An aggregate elsewhere in the predicate does not make a bare column
resolvable: the reducer has no single value
+ // per group for it, and used to fail at run time naming a GROUP BY clause
the user never wrote.
+ assertRejected("SELECT COUNT(*) FROM testTable HAVING COUNT(*) > amount");
+ assertRejected("SELECT city FROM testTable GROUP BY city HAVING COUNT(*) >
amount");
+ assertRejected("SELECT city, COUNT(*) FROM testTable GROUP BY city HAVING
COUNT(*) > amount");
+ // A column inside an aggregate stays legal, and so does a grouping column
next to one.
+ givenTable().whenQuery("SELECT city, COUNT(*) FROM testTable GROUP BY city
HAVING SUM(amount) > MIN(amount)")
+ .thenResultIs(new Object[]{"Athens", 4L}, new Object[]{"Madrid", 2L});
+ givenTable().whenQuery("SELECT city, COUNT(*) FROM testTable GROUP BY city
HAVING COUNT(*) > 2 AND city > 'B'")
+ .thenResultIs(new Object[0][]);
+ }
+
+ /// A GROUP BY with no aggregation anywhere has no grouping operator to
filter: the query becomes a DISTINCT, whose
+ /// reduce step cannot evaluate HAVING. Folding the predicate into WHERE
would be equivalent for a single-valued
+ /// grouping column but not for a multi-valued one, and the rewrite happens
before any schema is available, so the
+ /// shape is rejected rather than answered wrongly.
+ @Test
+ public void testHavingOnGroupByWithoutAnyAggregationIsRejected() {
+ assertRejected("SELECT city FROM testTable GROUP BY city HAVING city >
'B'");
+ // The SELECT list being a strict subset of the GROUP BY list reaches the
same reduce path.
+ assertRejected("SELECT city FROM testTable GROUP BY city, amount HAVING
city > 'B'");
+ // An aggregation anywhere -- SELECT list, HAVING or ORDER BY -- keeps the
GROUP BY reduce path and is accepted.
+ givenTable().whenQuery("SELECT city, COUNT(*) FROM testTable GROUP BY city
HAVING city > 'B'")
+ .thenResultIs(new Object[]{"Madrid", 2L});
+ }
+
+ /// The HAVING predicate is matched against the row before post-aggregation,
so its operands are resolved through
+ /// PostAggregationHandler rather than by position in the SELECT list. These
shapes break if that mapping is wrong.
+ @Test
+ public void testHavingOperandsAreMappedThroughPostAggregation() {
+ // Post-aggregation expression in the SELECT list shifts the result
columns away from the aggregate positions.
+ givenTable().whenQuery("SELECT SUM(amount) - COUNT(*) FROM testTable
HAVING SUM(amount) > 1")
+ .thenResultIs(new Object[]{20.0});
+ givenTable().whenQuery("SELECT SUM(amount) - COUNT(*) FROM testTable
HAVING SUM(amount) > 100")
+ .thenResultIs(new Object[0][]);
+ // An aggregate used only by the predicate is not in the SELECT list at
all.
+ givenTable().whenQuery("SELECT COUNT(*) FROM testTable HAVING SUM(amount)
> 1")
+ .thenResultIs(new Object[]{6L});
+ givenTable().whenQuery("SELECT MIN(amount), MAX(amount) FROM testTable
HAVING MIN(amount) < MAX(amount)")
+ .thenResultIs(new Object[]{1.0, 10.0});
+ }
+
+ /// With null handling enabled an aggregation over an all-null column
returns null, and the predicate must treat it
+ /// as non-matching instead of unboxing it. The reduce path materializes
such a null whichever way the
+ /// null-handling option is set, so both modes are covered.
+ @Test
+ public void testHavingOnNullAggregateResult() {
+ for (boolean nullHandling : new boolean[]{false, true}) {
+
FluentQueryTest.withBaseDir(_baseDir).withNullHandling(nullHandling).givenTable(NULLABLE_SCHEMA,
TABLE_CONFIG)
+ .onFirstInstance(new Object[]{null, "Athens"}, new Object[]{null,
"Madrid"})
+ .whenQuery("SELECT MAX(amount) FROM testTable WHERE city = 'Nowhere'
HAVING MAX(amount) > 1")
+ .thenResultIs(new Object[0][]);
+ }
+ }
+
+ private void assertRejected(String query) {
+ SqlCompilationException e =
+ expectThrows(SqlCompilationException.class, () ->
givenTable().whenQuery(query));
+ assertTrue(e.getMessage().contains("HAVING"), "Unexpected message for '" +
query + "': " + e.getMessage());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
