From f9f937f6ee596e7d355d78a89234b57786075d6c Mon Sep 17 00:00:00 2001 From: feiniaofeiafei Date: Fri, 15 Mar 2024 15:28:33 +0800 Subject: [PATCH 1/2] [Fix](nereids) remove duplicate expr in grouping set --- .../doris/nereids/rules/analysis/NormalizeRepeat.java | 11 +++++++++++ .../doris/nereids/trees/plans/algebra/Repeat.java | 3 --- .../nereids/trees/plans/logical/LogicalRepeat.java | 4 ++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java index 3c8c2b369990a9..2ccdba9d5e9fa7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java @@ -78,12 +78,23 @@ public Rule build() { return RuleType.NORMALIZE_REPEAT.build( logicalRepeat(any()).when(LogicalRepeat::canBindVirtualSlot).then(repeat -> { checkRepeatLegality(repeat); + repeat = removeDuplicateColumns(repeat); // add virtual slot, LogicalAggregate and LogicalProject for normalize return normalizeRepeat(repeat); }) ); } + private LogicalRepeat removeDuplicateColumns(LogicalRepeat repeat) { + List> groupingSets = repeat.getGroupingSets(); + ImmutableList.Builder> builder = ImmutableList.builder(); + for (List sets : groupingSets) { + List newList = ImmutableList.copyOf(ImmutableSet.copyOf(sets)); + builder.add(newList); + } + return repeat.withGroupSets(builder.build()); + } + private void checkRepeatLegality(LogicalRepeat repeat) { checkIfAggFuncSlotInGroupingSets(repeat); checkGroupingSetsSize(repeat); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java index 388cb4264310a7..810106a55ebad0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java @@ -175,9 +175,6 @@ default List> getGroupingSetsIndexesInOutput() { if (index == null) { throw new AnalysisException("Can not find grouping set expression in output: " + expression); } - if (groupingSetIndex.contains(index)) { - throw new AnalysisException("expression duplicate in grouping set: " + expression); - } groupingSetIndex.add(index); } groupingSetsIndex.add(groupingSetIndex); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java index 61fc8584d40163..8fee9add02b95a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java @@ -154,6 +154,10 @@ public Plan withGroupExprLogicalPropChildren(Optional groupExpr children.get(0)); } + public LogicalRepeat withGroupSets(List> groupingSets) { + return new LogicalRepeat<>(groupingSets, outputExpressions, child()); + } + public LogicalRepeat withGroupSetsAndOutput(List> groupingSets, List outputExpressionList) { return new LogicalRepeat<>(groupingSets, outputExpressionList, child()); From a80d54811d4dd6b00e42001b58b232d0714e0935 Mon Sep 17 00:00:00 2001 From: feiniaofeiafei Date: Fri, 15 Mar 2024 16:06:26 +0800 Subject: [PATCH 2/2] [Fix](nereids) remove duplicate expr in grouping set --- .../remove_duplicate_expr_in_grouping_set.out | 19 ++++++++++ ...move_duplicate_expr_in_grouping_set.groovy | 37 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 regression-test/data/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.out create mode 100644 regression-test/suites/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.groovy diff --git a/regression-test/data/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.out b/regression-test/data/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.out new file mode 100644 index 00000000000000..5956255d7bd136 --- /dev/null +++ b/regression-test/data/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.out @@ -0,0 +1,19 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !test_col -- +\N 6 +1 20 +2 8 +3 20 +4 2 +5 26 +7 1 + +-- !test_expr -- +\N 6 +2 20 +3 8 +4 20 +5 2 +6 26 +8 1 + diff --git a/regression-test/suites/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.groovy b/regression-test/suites/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.groovy new file mode 100644 index 00000000000000..b9a10fe21d2b4e --- /dev/null +++ b/regression-test/suites/nereids_rules_p0/grouping_sets/remove_duplicate_expr_in_grouping_set.groovy @@ -0,0 +1,37 @@ +// 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. +suite("remove_duplicate_expr_in_grouping_set") { + sql "SET enable_nereids_planner=true" + sql "SET enable_fallback_to_original_planner=false" + sql """ + DROP TABLE IF EXISTS mal_test1 + """ + + sql """ + create table mal_test1(pk int, a int, b int) distributed by hash(pk) buckets 10 + properties('replication_num' = '1'); + """ + + sql """ + insert into mal_test1 values(2,1,3),(1,1,2),(3,5,6),(6,null,6),(4,5,6),(2,1,4),(2,3,5),(1,1,4) + ,(3,5,6),(3,5,null),(6,7,1),(2,1,7),(2,4,2),(2,3,9),(1,3,6),(3,5,8),(3,2,8); + """ + sql "sync" + qt_test_col "select a, sum(b) from mal_test1 group by grouping sets((a,a)) order by 1,2" + qt_test_expr "select a+1,sum(b) from mal_test1 group by grouping sets((a+1,a+1)) order by 1,2" + +}