From bfcb128981bb69fcaab0b850567af76722f3f4cf Mon Sep 17 00:00:00 2001 From: starocean999 <12095047@qq.com> Date: Fri, 19 Jul 2024 14:50:36 +0800 Subject: [PATCH 1/2] [enhancement](nereids)optimized aggregate node's error msg --- .../doris/nereids/rules/analysis/BindExpression.java | 9 +++++++++ .../suites/nereids_p0/aggregate/agg_error_msg.groovy | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java index 8a74d71917c15b..1af67c27a53d5a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java @@ -110,6 +110,7 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -622,6 +623,14 @@ private Plan bindAggregate(MatchingContext> ctx) { SimpleExprAnalyzer aggOutputAnalyzer = buildSimpleExprAnalyzer( agg, cascadesContext, agg.children(), true, true); List boundAggOutput = aggOutputAnalyzer.analyzeToList(agg.getOutputExpressions()); + if (boundAggOutput.size() == 1 && boundAggOutput.get(0) instanceof BoundStar) { + // select * from t group by 1 + // the aggregate node's output is BoundStar and need to be replaced with real slots + BoundStar boundStar = (BoundStar) boundAggOutput.get(0); + List boundProjections = new ArrayList<>(); + boundProjections.addAll(boundStar.getSlots()); + boundAggOutput = boundProjections; + } Supplier aggOutputScopeWithoutAggFun = buildAggOutputScopeWithoutAggFun(boundAggOutput, cascadesContext); List boundGroupBy = bindGroupBy( agg, agg.getGroupByExpressions(), boundAggOutput, aggOutputScopeWithoutAggFun, cascadesContext); diff --git a/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy b/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy index 0b807be9d3add2..3f889769acfebf 100644 --- a/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy +++ b/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy @@ -47,4 +47,9 @@ suite("agg_error_msg") { sql """SELECT col_int_undef_signed2 col_alias1, col_int_undef_signed * (SELECT MAX (col_int_undef_signed) FROM table_20_undef_partitions2_keys3_properties4_distributed_by58 where table_20_undef_partitions2_keys3_properties4_distributed_by53.pk = pk) AS col_alias2 FROM table_20_undef_partitions2_keys3_properties4_distributed_by53 GROUP BY GROUPING SETS ((col_int_undef_signed2),()) ;""" exception "pk, col_int_undef_signed not in aggregate's output"; } + + test { + sql """SELECT * from table_20_undef_partitions2_keys3_properties4_distributed_by58 group by 1;""" + exception "col_int_undef_signed, col_int_undef_signed2 not in aggregate's output"; + } } From 752543310efdd5c516d146ebcac18d7802b94149 Mon Sep 17 00:00:00 2001 From: starocean999 <12095047@qq.com> Date: Mon, 22 Jul 2024 12:35:45 +0800 Subject: [PATCH 2/2] modified based on comments --- .../rules/analysis/BindExpression.java | 21 ++++++++++--------- .../nereids_p0/aggregate/agg_error_msg.groovy | 10 +++++++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java index 1af67c27a53d5a..15f8dabc8f7da4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java @@ -623,18 +623,19 @@ private Plan bindAggregate(MatchingContext> ctx) { SimpleExprAnalyzer aggOutputAnalyzer = buildSimpleExprAnalyzer( agg, cascadesContext, agg.children(), true, true); List boundAggOutput = aggOutputAnalyzer.analyzeToList(agg.getOutputExpressions()); - if (boundAggOutput.size() == 1 && boundAggOutput.get(0) instanceof BoundStar) { - // select * from t group by 1 - // the aggregate node's output is BoundStar and need to be replaced with real slots - BoundStar boundStar = (BoundStar) boundAggOutput.get(0); - List boundProjections = new ArrayList<>(); - boundProjections.addAll(boundStar.getSlots()); - boundAggOutput = boundProjections; + List boundProjections = new ArrayList<>(boundAggOutput.size()); + for (NamedExpression output : boundAggOutput) { + if (output instanceof BoundStar) { + boundProjections.addAll(((BoundStar) output).getSlots()); + } else { + boundProjections.add(output); + } } - Supplier aggOutputScopeWithoutAggFun = buildAggOutputScopeWithoutAggFun(boundAggOutput, cascadesContext); + Supplier aggOutputScopeWithoutAggFun = + buildAggOutputScopeWithoutAggFun(boundProjections, cascadesContext); List boundGroupBy = bindGroupBy( - agg, agg.getGroupByExpressions(), boundAggOutput, aggOutputScopeWithoutAggFun, cascadesContext); - return agg.withGroupByAndOutput(boundGroupBy, boundAggOutput); + agg, agg.getGroupByExpressions(), boundProjections, aggOutputScopeWithoutAggFun, cascadesContext); + return agg.withGroupByAndOutput(boundGroupBy, boundProjections); } private Plan bindRepeat(MatchingContext> ctx) { diff --git a/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy b/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy index 3f889769acfebf..a644a26ade7330 100644 --- a/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy +++ b/regression-test/suites/nereids_p0/aggregate/agg_error_msg.groovy @@ -52,4 +52,14 @@ suite("agg_error_msg") { sql """SELECT * from table_20_undef_partitions2_keys3_properties4_distributed_by58 group by 1;""" exception "col_int_undef_signed, col_int_undef_signed2 not in aggregate's output"; } + + test { + sql """SELECT *, pk from table_20_undef_partitions2_keys3_properties4_distributed_by58 group by 1;""" + exception "col_int_undef_signed, col_int_undef_signed2 not in aggregate's output"; + } + + test { + sql """SELECT *, * from table_20_undef_partitions2_keys3_properties4_distributed_by58 group by 1;""" + exception "col_int_undef_signed, col_int_undef_signed2 not in aggregate's output"; + } }