This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-1.2-lts
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 887dcf17aca3cb25b0aa32349998babcd9f41126
Author: minghong <engle...@gmail.com>
AuthorDate: Thu Sep 14 09:54:09 2023 +0800
[fix](planner) having clause analyze bug #24288
---
.../java/org/apache/doris/analysis/SelectStmt.java | 78 +++++++++++-----------
.../suites/query_p0/having/having.groovy | 49 ++++++++++++++
2 files changed, 89 insertions(+), 38 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
index 16bb1ecf2fd..4b5836074f0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
@@ -109,7 +109,7 @@ public class SelectStmt extends QueryStmt {
// having clause which has been analyzed
// For example: select k1, sum(k2) a from t group by k1 having a>1;
// this parameter: sum(t.k2) > 1
- private Expr havingClauseAfterAnaylzed;
+ private Expr havingClauseAfterAnalyzed;
// END: Members that need to be reset()
// ///////////////////////////////////////
@@ -167,8 +167,8 @@ public class SelectStmt extends QueryStmt {
whereClause = (other.whereClause != null) ? other.whereClause.clone()
: null;
groupByClause = (other.groupByClause != null) ?
other.groupByClause.clone() : null;
havingClause = (other.havingClause != null) ?
other.havingClause.clone() : null;
- havingClauseAfterAnaylzed =
- other.havingClauseAfterAnaylzed != null ?
other.havingClauseAfterAnaylzed.clone() : null;
+ havingClauseAfterAnalyzed =
+ other.havingClauseAfterAnalyzed != null ?
other.havingClauseAfterAnalyzed.clone() : null;
colLabels = Lists.newArrayList(other.colLabels);
aggInfo = (other.aggInfo != null) ? other.aggInfo.clone() : null;
@@ -193,7 +193,7 @@ public class SelectStmt extends QueryStmt {
if (havingClause != null) {
havingClause.reset();
}
- havingClauseAfterAnaylzed = null;
+ havingClauseAfterAnalyzed = null;
havingPred = null;
aggInfo = null;
analyticInfo = null;
@@ -240,7 +240,7 @@ public class SelectStmt extends QueryStmt {
}
public Expr getHavingClauseAfterAnaylzed() {
- return havingClauseAfterAnaylzed;
+ return havingClauseAfterAnalyzed;
}
public List<TableRef> getTableRefs() {
@@ -639,8 +639,8 @@ public class SelectStmt extends QueryStmt {
if (whereClause != null) {
whereClause.getIds(result, null);
}
- if (havingClauseAfterAnaylzed != null) {
- havingClauseAfterAnaylzed.getIds(result, null);
+ if (havingClauseAfterAnalyzed != null) {
+ havingClauseAfterAnalyzed.getIds(result, null);
}
return result;
}
@@ -1055,48 +1055,48 @@ public class SelectStmt extends QueryStmt {
// according to case3, column name do not exist,
keep alias name inside alias map
}
}
- havingClauseAfterAnaylzed =
havingClause.substitute(excludeAliasSMap, analyzer, false);
+ havingClauseAfterAnalyzed =
havingClause.substitute(excludeAliasSMap, analyzer, false);
} else {
// If user set force using alias, then having clauses
prefer using alias rather than column name
- havingClauseAfterAnaylzed =
havingClause.substitute(aliasSMap, analyzer, false);
+ havingClauseAfterAnalyzed =
havingClause.substitute(aliasSMap, analyzer, false);
}
} else {
// according to mysql
// if there is no group by clause, the having clause should
use alias
- havingClauseAfterAnaylzed = havingClause.substitute(aliasSMap,
analyzer, false);
+ havingClauseAfterAnalyzed = havingClause.substitute(aliasSMap,
analyzer, false);
}
- havingClauseAfterAnaylzed =
rewriteQueryExprByMvColumnExpr(havingClauseAfterAnaylzed, analyzer);
- havingClauseAfterAnaylzed.checkReturnsBool("HAVING clause", true);
+ havingClauseAfterAnalyzed =
rewriteQueryExprByMvColumnExpr(havingClauseAfterAnalyzed, analyzer);
+ havingClauseAfterAnalyzed.checkReturnsBool("HAVING clause", true);
if (groupingInfo != null) {
-
groupingInfo.substituteGroupingFn(Arrays.asList(havingClauseAfterAnaylzed),
analyzer);
+
groupingInfo.substituteGroupingFn(Arrays.asList(havingClauseAfterAnalyzed),
analyzer);
}
// can't contain analytic exprs
- Expr analyticExpr =
havingClauseAfterAnaylzed.findFirstOf(AnalyticExpr.class);
+ Expr analyticExpr =
havingClauseAfterAnalyzed.findFirstOf(AnalyticExpr.class);
if (analyticExpr != null) {
throw new AnalysisException(
"HAVING clause must not contain analytic expressions: "
+ analyticExpr.toSql());
}
- if (isContainInBitmap(havingClauseAfterAnaylzed)) {
+ if (isContainInBitmap(havingClauseAfterAnalyzed)) {
throw new AnalysisException(
- "HAVING clause dose not support in bitmap syntax: " +
havingClauseAfterAnaylzed.toSql());
+ "HAVING clause dose not support in bitmap syntax: " +
havingClauseAfterAnalyzed.toSql());
}
}
if (groupByClause == null && !selectList.isDistinct()
&& !TreeNode.contains(resultExprs, Expr.isAggregatePredicate())
- && (havingClauseAfterAnaylzed == null ||
!havingClauseAfterAnaylzed.contains(
+ && (havingClauseAfterAnalyzed == null ||
!havingClauseAfterAnalyzed.contains(
Expr.isAggregatePredicate()))
&& (sortInfo == null ||
!TreeNode.contains(sortInfo.getOrderingExprs(),
Expr.isAggregatePredicate()))) {
// We're not computing aggregates but we still need to register
the HAVING
// clause which could, e.g., contain a constant expression
evaluating to false.
- if (havingClauseAfterAnaylzed != null) {
- if (havingClauseAfterAnaylzed.contains(Subquery.class)) {
+ if (havingClauseAfterAnalyzed != null) {
+ if (havingClauseAfterAnalyzed.contains(Subquery.class)) {
throw new AnalysisException("Only constant expr could be
supported in having clause "
+ "when no aggregation in stmt");
}
- analyzer.registerConjuncts(havingClauseAfterAnaylzed, true);
+ analyzer.registerConjuncts(havingClauseAfterAnalyzed, true);
}
return;
}
@@ -1117,7 +1117,7 @@ public class SelectStmt extends QueryStmt {
if (selectList.isDistinct()
&& (groupByClause != null
|| TreeNode.contains(resultExprs, Expr.isAggregatePredicate())
- || (havingClauseAfterAnaylzed != null &&
havingClauseAfterAnaylzed.contains(
+ || (havingClauseAfterAnalyzed != null &&
havingClauseAfterAnalyzed.contains(
Expr.isAggregatePredicate())))) {
throw new AnalysisException("cannot combine SELECT DISTINCT with
aggregate functions or GROUP BY");
}
@@ -1148,8 +1148,8 @@ public class SelectStmt extends QueryStmt {
// of this statement.
ArrayList<FunctionCallExpr> aggExprs = Lists.newArrayList();
TreeNode.collect(resultExprs, Expr.isAggregatePredicate(), aggExprs);
- if (havingClauseAfterAnaylzed != null) {
- havingClauseAfterAnaylzed.collect(Expr.isAggregatePredicate(),
aggExprs);
+ if (havingClauseAfterAnalyzed != null) {
+ havingClauseAfterAnalyzed.collect(Expr.isAggregatePredicate(),
aggExprs);
}
if (sortInfo != null) {
// TODO: Avoid evaluating aggs in ignored order-bys
@@ -1175,9 +1175,9 @@ public class SelectStmt extends QueryStmt {
// the resultExprs and havingClause must substitute in the same way as
aggExprs
// then resultExprs and havingClause can be substitute correctly using
combinedSmap
resultExprs = Expr.substituteList(resultExprs, countAllMap, analyzer,
false);
- if (havingClauseAfterAnaylzed != null) {
- havingClauseAfterAnaylzed =
- havingClauseAfterAnaylzed.substitute(countAllMap,
analyzer, false);
+ if (havingClauseAfterAnalyzed != null) {
+ havingClauseAfterAnalyzed =
+ havingClauseAfterAnalyzed.substitute(countAllMap,
analyzer, false);
}
if (sortInfo != null) {
// the ordering exprs must substitute in the same way as
resultExprs
@@ -1297,10 +1297,10 @@ public class SelectStmt extends QueryStmt {
LOG.debug("resultexprs: " + Expr.debugString(resultExprs));
}
- if (havingClauseAfterAnaylzed != null) {
+ if (havingClauseAfterAnalyzed != null) {
// forbidden correlated subquery in having clause
List<Subquery> subqueryInHaving = Lists.newArrayList();
- havingClauseAfterAnaylzed.collect(Subquery.class,
subqueryInHaving);
+ havingClauseAfterAnalyzed.collect(Subquery.class,
subqueryInHaving);
for (Subquery subquery : subqueryInHaving) {
if (subquery.isCorrelatedPredicate(getTableRefIds())) {
throw new AnalysisException("The correlated having clause
is not supported");
@@ -1325,8 +1325,8 @@ public class SelectStmt extends QueryStmt {
if (LOG.isDebugEnabled()) {
LOG.debug("post-agg selectListExprs: " +
Expr.debugString(resultExprs));
}
- if (havingClauseAfterAnaylzed != null) {
- havingPred = havingClauseAfterAnaylzed.substitute(combinedSmap,
analyzer, false);
+ if (havingClauseAfterAnalyzed != null) {
+ havingPred = havingClauseAfterAnalyzed.substitute(combinedSmap,
analyzer, false);
analyzer.registerConjuncts(havingPred, true,
finalAggInfo.getOutputTupleId().asList());
if (LOG.isDebugEnabled()) {
LOG.debug("post-agg havingPred: " + havingPred.debugString());
@@ -1576,10 +1576,12 @@ public class SelectStmt extends QueryStmt {
whereClause.collect(Subquery.class, subqueryExprs);
}
- if (havingClause != null) {
- havingClause = rewriter.rewrite(havingClause, analyzer);
- havingClauseAfterAnaylzed.collect(Subquery.class, subqueryExprs);
+
+ if (havingClauseAfterAnalyzed != null) {
+ havingClauseAfterAnalyzed =
rewriter.rewrite(havingClauseAfterAnalyzed, analyzer);
+ havingClauseAfterAnalyzed.collect(Subquery.class, subqueryExprs);
}
+
for (Subquery subquery : subqueryExprs) {
subquery.getStatement().rewriteExprs(rewriter);
}
@@ -1672,9 +1674,9 @@ public class SelectStmt extends QueryStmt {
}
if (havingClause != null) {
- registerExprId(havingClauseAfterAnaylzed);
- exprMap.put(havingClauseAfterAnaylzed.getId().toString(),
havingClauseAfterAnaylzed);
- havingClauseAfterAnaylzed.collect(Subquery.class, subqueryExprs);
+ registerExprId(havingClauseAfterAnalyzed);
+ exprMap.put(havingClauseAfterAnalyzed.getId().toString(),
havingClauseAfterAnalyzed);
+ havingClauseAfterAnalyzed.collect(Subquery.class, subqueryExprs);
}
for (Subquery subquery : subqueryExprs) {
registerExprId(subquery);
@@ -1779,8 +1781,8 @@ public class SelectStmt extends QueryStmt {
whereClause.collect(Subquery.class, subqueryExprs);
}
if (havingClause != null) {
- havingClause =
rewrittenExprMap.get(havingClauseAfterAnaylzed.getId().toString());
- havingClauseAfterAnaylzed.collect(Subquery.class, subqueryExprs);
+ havingClause =
rewrittenExprMap.get(havingClauseAfterAnalyzed.getId().toString());
+ havingClauseAfterAnalyzed.collect(Subquery.class, subqueryExprs);
}
for (Subquery subquery : subqueryExprs) {
diff --git a/regression-test/suites/query_p0/having/having.groovy
b/regression-test/suites/query_p0/having/having.groovy
new file mode 100644
index 00000000000..2cb0755d542
--- /dev/null
+++ b/regression-test/suites/query_p0/having/having.groovy
@@ -0,0 +1,49 @@
+// 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.
+
+// The cases is copied from https://github.com/trinodb/trino/tree/master
+//
/testing/trino-product-tests/src/main/resources/sql-tests/testcases/aggregate
+// and modified by Doris.
+
+suite("having") {
+ sql "set enable_nereids_planner=false;"
+ sql """CREATE TABLE `supplier` (
+ `s_suppkey` int(11) NOT NULL,
+ `s_name` varchar(25) NOT NULL,
+ `s_address` varchar(40) NOT NULL,
+ `s_nationkey` int(11) NOT NULL,
+ `s_phone` varchar(15) NOT NULL,
+ `s_acctbal` DECIMAL(15, 2) NOT NULL,
+ `s_comment` varchar(101) NOT NULL
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`s_suppkey`)
+ COMMENT 'OLAP'
+ DISTRIBUTED BY HASH(`s_suppkey`) BUCKETS 12
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1",
+ "is_being_synced" = "false",
+ "storage_format" = "V2",
+ "light_schema_change" = "true",
+ "disable_auto_compaction" = "false",
+ "enable_single_replica_compaction" = "false"
+ ); """
+ sql """explain
+ select count(*)
+ from supplier s
+ group by s_nationkey,s_suppkey
+ having s_nationkey=1 or s_suppkey=1;"""
+}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
