This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 3af0745c8f [Bug](function) fix aggFnParams set not correct (#12006)
3af0745c8f is described below
commit 3af0745c8feec5ee5805211c444494b330f9e899
Author: Pxl <[email protected]>
AuthorDate: Fri Aug 26 14:29:56 2022 +0800
[Bug](function) fix aggFnParams set not correct (#12006)
---
.../aggregate_function_window_funnel.cpp | 7 +--
.../apache/doris/analysis/AggregateParamsList.java | 61 ----------------------
.../apache/doris/analysis/FunctionCallExpr.java | 30 ++++++++---
.../org/apache/doris/analysis/FunctionParams.java | 9 ++++
.../data/query_p0/join/sql/issue_7288.out | 4 ++
.../suites/query_p0/join/ddl/left_table.sql | 12 +++++
.../suites/query_p0/join/ddl/right_table.sql | 12 +++++
regression-test/suites/query_p0/join/load.groovy | 4 +-
.../suites/query_p0/join/sql/issue_7288.sql | 1 +
9 files changed, 68 insertions(+), 72 deletions(-)
diff --git
a/be/src/vec/aggregate_functions/aggregate_function_window_funnel.cpp
b/be/src/vec/aggregate_functions/aggregate_function_window_funnel.cpp
index cbdb8a85d5..ea323f1a6b 100644
--- a/be/src/vec/aggregate_functions/aggregate_function_window_funnel.cpp
+++ b/be/src/vec/aggregate_functions/aggregate_function_window_funnel.cpp
@@ -17,10 +17,7 @@
#include "vec/aggregate_functions/aggregate_function_window_funnel.h"
-#include "common/logging.h"
#include "vec/aggregate_functions/aggregate_function_simple_factory.h"
-#include "vec/aggregate_functions/factory_helpers.h"
-#include "vec/aggregate_functions/helpers.h"
namespace doris::vectorized {
@@ -28,6 +25,10 @@ AggregateFunctionPtr
create_aggregate_function_window_funnel(const std::string&
const DataTypes&
argument_types,
const Array&
parameters,
const bool
result_is_nullable) {
+ if (argument_types.size() < 3) {
+ LOG(WARNING) << "window_funnel's argument less than 3.";
+ return nullptr;
+ }
if (WhichDataType(argument_types[2]).is_date_time_v2()) {
return std::make_shared<
AggregateFunctionWindowFunnel<DateV2Value<DateTimeV2ValueType>, UInt64>>(
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateParamsList.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateParamsList.java
deleted file mode 100644
index 12ff07d99a..0000000000
---
a/fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateParamsList.java
+++ /dev/null
@@ -1,61 +0,0 @@
-// 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.doris.analysis;
-
-import java.util.List;
-
-/**
- * Return value of the grammar production that parses aggregate function
- * parameters.
- */
-class AggregateParamsList {
- private final boolean isStar;
- private final boolean isDistinct;
- private List<Expr> exprs;
-
- // c'tor for non-star params
- public AggregateParamsList(boolean isDistinct, List<Expr> exprs) {
- super();
- isStar = false;
- this.isDistinct = isDistinct;
- this.exprs = exprs;
- }
-
- // c'tor for <agg>(*)
- private AggregateParamsList() {
- super();
- isStar = true;
- isDistinct = false;
- }
-
- public static AggregateParamsList createStarParam() {
- return new AggregateParamsList();
- }
-
- public boolean isStar() {
- return isStar;
- }
-
- public boolean isDistinct() {
- return isDistinct;
- }
-
- public List<Expr> exprs() {
- return exprs;
- }
-}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
index 363eda3bd0..2b7c6c837e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
@@ -57,6 +57,7 @@ import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
import java.text.StringCharacterIterator;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
@@ -212,17 +213,13 @@ public class FunctionCallExpr extends Expr {
fnName = other.fnName;
orderByElements = other.orderByElements;
isAnalyticFnCall = other.isAnalyticFnCall;
- // aggOp = other.aggOp;
+ // aggOp = other.aggOp;
// fnParams = other.fnParams;
// Clone the params in a way that keeps the children_ and the
params.exprs()
// in sync. The children have already been cloned in the super c'tor.
- if (other.fnParams.isStar()) {
- Preconditions.checkState(children.isEmpty());
- fnParams = FunctionParams.createStarParam();
- } else {
- fnParams = new FunctionParams(other.fnParams.isDistinct(),
children);
- }
+ fnParams = other.fnParams.clone(children);
aggFnParams = other.aggFnParams;
+
this.isMergeAggFn = other.isMergeAggFn;
fn = other.fn;
this.isTableFnCall = other.isTableFnCall;
@@ -257,6 +254,25 @@ public class FunctionCallExpr extends Expr {
return isMergeAggFn;
}
+ @Override
+ protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer)
+ throws AnalysisException {
+ if (aggFnParams != null && aggFnParams.exprs() != null) {
+ ArrayList<Expr> newParams = new ArrayList<Expr>();
+ for (Expr expr : aggFnParams.exprs()) {
+ Expr substExpr = smap.get(expr);
+ if (substExpr != null) {
+ newParams.add(substExpr.clone());
+ } else {
+ newParams.add(expr);
+ }
+ }
+ aggFnParams = aggFnParams
+ .clone(newParams);
+ }
+ return super.substituteImpl(smap, analyzer);
+ }
+
@Override
public Expr clone() {
return new FunctionCallExpr(this);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java
index 3b77ec52b6..3968a736a1 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java
@@ -24,6 +24,7 @@ import org.apache.doris.common.io.Writable;
import org.apache.doris.thrift.TAggregateExpr;
import org.apache.doris.thrift.TTypeDesc;
+import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import java.io.DataInput;
@@ -65,6 +66,14 @@ public class FunctionParams implements Writable {
return new FunctionParams();
}
+ public FunctionParams clone(List<Expr> children) {
+ if (isStar()) {
+ Preconditions.checkState(children.isEmpty());
+ return FunctionParams.createStarParam();
+ }
+ return new FunctionParams(isDistinct(), children);
+ }
+
public TAggregateExpr createTAggregateExpr(boolean isMergeAggFn) {
List<TTypeDesc> paramTypes = new ArrayList<TTypeDesc>();
if (exprs != null) {
diff --git a/regression-test/data/query_p0/join/sql/issue_7288.out
b/regression-test/data/query_p0/join/sql/issue_7288.out
new file mode 100644
index 0000000000..6d81669a89
--- /dev/null
+++ b/regression-test/data/query_p0/join/sql/issue_7288.out
@@ -0,0 +1,4 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !issue_7288 --
+1 \N
+
diff --git a/regression-test/suites/query_p0/join/ddl/left_table.sql
b/regression-test/suites/query_p0/join/ddl/left_table.sql
new file mode 100644
index 0000000000..81da559ddc
--- /dev/null
+++ b/regression-test/suites/query_p0/join/ddl/left_table.sql
@@ -0,0 +1,12 @@
+CREATE TABLE left_table (
+k1 int(11) NULL COMMENT "",
+no varchar(50) NOT NULL COMMENT ""
+) ENGINE=OLAP
+DUPLICATE KEY(k1)
+COMMENT "OLAP"
+DISTRIBUTED BY HASH(k1) BUCKETS 2
+PROPERTIES (
+"replication_allocation" = "tag.location.default: 1",
+"in_memory" = "false",
+"storage_format" = "V2"
+);
diff --git a/regression-test/suites/query_p0/join/ddl/right_table.sql
b/regression-test/suites/query_p0/join/ddl/right_table.sql
new file mode 100644
index 0000000000..1a7f0e63cd
--- /dev/null
+++ b/regression-test/suites/query_p0/join/ddl/right_table.sql
@@ -0,0 +1,12 @@
+CREATE TABLE right_table (
+k1 int(11) NULL COMMENT "",
+no varchar(50) NOT NULL COMMENT ""
+) ENGINE=OLAP
+DUPLICATE KEY(k1)
+COMMENT "OLAP"
+DISTRIBUTED BY HASH(k1) BUCKETS 2
+PROPERTIES (
+"replication_allocation" = "tag.location.default: 1",
+"in_memory" = "false",
+"storage_format" = "V2"
+);
diff --git a/regression-test/suites/query_p0/join/load.groovy
b/regression-test/suites/query_p0/join/load.groovy
index 81d44d9339..9f12ae40f1 100644
--- a/regression-test/suites/query_p0/join/load.groovy
+++ b/regression-test/suites/query_p0/join/load.groovy
@@ -20,7 +20,7 @@
// and modified by Doris.
suite("load") {
- def tables=["test_join", "full_join_table", "test_bucket_shuffle_join",
"table_1", "table_2"]
+ def tables=["test_join", "full_join_table", "test_bucket_shuffle_join",
"table_1", "table_2", "left_table", "right_table"]
for (String table in tables) {
sql """ DROP TABLE IF EXISTS $table """
@@ -40,4 +40,6 @@ suite("load") {
sql """ INSERT INTO full_join_table(x) VALUES (NULL) """;
sql """ INSERT INTO table_2 VALUES ('H220427011909850160918','2022-04-27
16:00:33'),('T220427400109910160949','2022-04-27
16:00:54'),('T220427400123770120058','2022-04-27
16:00:56'),('T220427400126530112854','2022-04-27
16:00:34'),('T220427400127160144672','2022-04-27
16:00:10'),('T220427400127900184511','2022-04-27
16:00:34'),('T220427400129940120380','2022-04-27
16:00:23'),('T220427400139720192986','2022-04-27
16:00:34'),('T220427400140260152375','2022-04-27 16:00:02'),('T220427400 [...]
+
+ sql """ INSERT INTO left_table values(1, "test") """;
}
diff --git a/regression-test/suites/query_p0/join/sql/issue_7288.sql
b/regression-test/suites/query_p0/join/sql/issue_7288.sql
new file mode 100644
index 0000000000..461a4cf94e
--- /dev/null
+++ b/regression-test/suites/query_p0/join/sql/issue_7288.sql
@@ -0,0 +1 @@
+ select l.k1, group_concat(r.no) from left_table l left join right_table r on
l.k1=r.k1 group by l.k1;
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
