Skip to content

[Opt](agg) enable_single_distinct_column_opt do not effective in not group by #12935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
HappenLee merged 1 commit into from
Sep 24, 2022
Merged

[Opt](agg) enable_single_distinct_column_opt do not effective in not group by #12935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ public static AggregateInfo create(
// 1: if aggExprs don't have distinct or have multi distinct , create aggregate info for
// one stage aggregation.
// 2: if aggExprs have one distinct , create aggregate info for two stage aggregation
boolean isUsingSetForDistinct = estimateIfUsingSetForDistinct(distinctAggExprs);
boolean isUsingSetForDistinct = estimateIfUsingSetForDistinct(distinctAggExprs,
!groupingExprs.isEmpty());
if (distinctAggExprs.isEmpty() || isUsingSetForDistinct) {
// It is used to map new aggr expr to old expr to help create an external
// reference to the aggregation node tuple
Expand Down Expand Up @@ -249,22 +250,25 @@ public static AggregateInfo create(


// note(wb): in some cases, using hashset for distinct is better
public static boolean isSetUsingSetForDistinct(List<FunctionCallExpr> distinctAggExprs) {
public static boolean isSetUsingSetForDistinct(List<FunctionCallExpr> distinctAggExprs,
boolean haveGrouping) {
boolean isSetUsingSetForDistinct = false;
// for vectorized execution, we force it to using hash set to execution
if (distinctAggExprs.size() == 1
&& distinctAggExprs.get(0).getFnParams().isDistinct()
&& VectorizedUtil.isVectorized()
&& haveGrouping
&& ConnectContext.get().getSessionVariable().enableSingleDistinctColumnOpt()) {
isSetUsingSetForDistinct = true;
}
return isSetUsingSetForDistinct;
}

public static boolean estimateIfUsingSetForDistinct(List<FunctionCallExpr> distinctAggExprs)
public static boolean estimateIfUsingSetForDistinct(List<FunctionCallExpr> distinctAggExprs,
boolean haveGrouping)
throws AnalysisException {
return estimateIfContainsMultiDistinct(distinctAggExprs)
|| isSetUsingSetForDistinct(distinctAggExprs);
|| isSetUsingSetForDistinct(distinctAggExprs, haveGrouping);
}

/**
Expand Down Expand Up @@ -367,7 +371,7 @@ private void createDistinctAggInfo(
}
}

this.isUsingSetForDistinct = estimateIfUsingSetForDistinct(distinctAggExprs);
this.isUsingSetForDistinct = estimateIfUsingSetForDistinct(distinctAggExprs, !groupingExprs.isEmpty());

// add DISTINCT parameters to grouping exprs
if (!isUsingSetForDistinct) {
Expand Down
6 changes: 3 additions & 3 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ private void analyzeAggregation(Analyzer analyzer) throws AnalysisException {
// ii) Other DISTINCT aggregates are present.
ExprSubstitutionMap countAllMap = createCountAllMap(aggExprs, analyzer);
final ExprSubstitutionMap multiCountOrSumDistinctMap =
createSumOrCountMultiDistinctSMap(aggExprs, analyzer);
createSumOrCountMultiDistinctSMap(aggExprs, groupByClause != null, analyzer);
countAllMap = ExprSubstitutionMap.compose(multiCountOrSumDistinctMap, countAllMap, analyzer);
List<Expr> substitutedAggs =
Expr.substituteList(aggExprs, countAllMap, analyzer, false);
Expand Down Expand Up @@ -1175,15 +1175,15 @@ private void analyzeAggregation(Analyzer analyzer) throws AnalysisException {
* assumes that select list and having clause have been analyzed.
*/
private ExprSubstitutionMap createSumOrCountMultiDistinctSMap(
ArrayList<FunctionCallExpr> aggExprs, Analyzer analyzer) throws AnalysisException {
ArrayList<FunctionCallExpr> aggExprs, boolean haveGrouping, Analyzer analyzer) throws AnalysisException {
final List<FunctionCallExpr> distinctExprs = Lists.newArrayList();
for (FunctionCallExpr aggExpr : aggExprs) {
if (aggExpr.isDistinct()) {
distinctExprs.add(aggExpr);
}
}
final ExprSubstitutionMap result = new ExprSubstitutionMap();
final boolean isUsingSetForDistinct = AggregateInfo.estimateIfUsingSetForDistinct(distinctExprs);
final boolean isUsingSetForDistinct = AggregateInfo.estimateIfUsingSetForDistinct(distinctExprs, haveGrouping);
if (!isUsingSetForDistinct) {
return result;
}
Expand Down