wangshuo128 commented on code in PR #9993:
URL: https://github.com/apache/incubator-doris/pull/9993#discussion_r894130900
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -38,49 +38,48 @@
* Encapsulates all the information needed to compute the aggregate functions
of a single
* Select block, including a possible 2nd phase aggregation step for DISTINCT
aggregate
* functions and merge aggregation steps needed for distributed execution.
- *
+ * <p>
* The latter requires a tree structure of AggregateInfo objects which express
the
* original aggregate computations as well as the necessary merging aggregate
* computations.
* TODO: get rid of this by transforming
* SELECT COUNT(DISTINCT a, b, ..) GROUP BY x, y, ...
* into an equivalent query with a inline view:
* SELECT COUNT(*) FROM (SELECT DISTINCT a, b, ..., x, y, ...) GROUP BY x,
y, ...
- *
+ * <p>
* The tree structure looks as follows:
* - for non-distinct aggregation:
- * - aggInfo: contains the original aggregation functions and grouping exprs
- * - aggInfo.mergeAggInfo: contains the merging aggregation functions
(grouping
- * exprs are identical)
+ * - aggInfo: contains the original aggregation functions and grouping exprs
+ * - aggInfo.mergeAggInfo: contains the merging aggregation functions (grouping
+ * exprs are identical)
* - for distinct aggregation (for an explanation of the phases, see
- * SelectStmt.createDistinctAggInfo()):
- * - aggInfo: contains the phase 1 aggregate functions and grouping exprs
- * - aggInfo.2ndPhaseDistinctAggInfo: contains the phase 2 aggregate
functions and
- * grouping exprs
- * - aggInfo.mergeAggInfo: contains the merging aggregate functions for the
phase 1
- * computation (grouping exprs are identical)
- * - aggInfo.2ndPhaseDistinctAggInfo.mergeAggInfo: contains the merging
aggregate
- * functions for the phase 2 computation (grouping exprs are identical)
- *
+ * SelectStmt.createDistinctAggInfo()):
+ * - aggInfo: contains the phase 1 aggregate functions and grouping exprs
+ * - aggInfo.2ndPhaseDistinctAggInfo: contains the phase 2 aggregate functions
and
+ * grouping exprs
+ * - aggInfo.mergeAggInfo: contains the merging aggregate functions for the
phase 1
+ * computation (grouping exprs are identical)
+ * - aggInfo.2ndPhaseDistinctAggInfo.mergeAggInfo: contains the merging
aggregate
+ * functions for the phase 2 computation (grouping exprs are identical)
+ * <p>
* In general, merging aggregate computations are idempotent; in other words,
* aggInfo.mergeAggInfo == aggInfo.mergeAggInfo.mergeAggInfo.
- *
+ * <p>
* TODO: move the merge construction logic from SelectStmt into AggregateInfo
* TODO: Add query tests for aggregation with intermediate tuples with
num_nodes=1.
*/
public final class AggregateInfo extends AggregateInfoBase {
private final static Logger LOG =
LogManager.getLogger(AggregateInfo.class);
public enum AggPhase {
- FIRST,
- FIRST_MERGE,
- SECOND,
- SECOND_MERGE;
+ FIRST, FIRST_MERGE, SECOND, SECOND_MERGE;
Review Comment:
using the original code style?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -414,20 +405,20 @@ public void getRefdSlots(List<SlotId> ids) {
* Substitute all the expressions (grouping expr, aggregate expr) and
update our
* substitution map according to the given substitution map:
* - smap typically maps from tuple t1 to tuple t2 (example: the smap of an
- * inline view maps the virtual table ref t1 into a base table ref t2)
Review Comment:
ditto.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -539,15 +526,14 @@ private Expr createCountDistinctAggExprParam(int
firstIdx, int lastIdx,
* - 'this' is the phase 1 aggregation
* - grouping exprs are those of the original query (param
origGroupingExprs)
* - aggregate exprs for the DISTINCT agg fns: these are aggregating the
grouping
- * slots that were added to the original grouping slots in phase 1;
Review Comment:
ditto.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -733,8 +707,8 @@ public void createSmaps(Analyzer analyzer) {
* - The parameters of the sum function may involve the columns of a
materialized view.
* - The type of this column may happen to be inconsistent with the column
type of the base table.
* - In order to ensure the correctness of the result,
- * the parameter type needs to be changed to the type of the
materialized view column
Review Comment:
ditto.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -277,29 +271,27 @@ public static boolean
estimateIfContainsMultiDistinct(List<FunctionCallExpr> dis
* - aggTupleDesc
* - a complete secondPhaseDistinctAggInfo
* - mergeAggInfo
- *
+ * <p>
* At the moment, we require that all distinct aggregate
* functions be applied to the same set of exprs (ie, we can't do something
* like SELECT COUNT(DISTINCT id), COUNT(DISTINCT address)).
* Aggregation happens in two successive phases:
* - the first phase aggregates by all grouping exprs plus all parameter
exprs
- * of DISTINCT aggregate functions
- *
+ * of DISTINCT aggregate functions
+ * <p>
* Example:
Review Comment:
using the original indents?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java:
##########
@@ -761,10 +735,10 @@ public void updateTypeOfAggregateExprs() {
* Mark slots required for this aggregation as materialized:
* - all grouping output slots as well as grouping exprs
* - for non-distinct aggregation: the aggregate exprs of materialized
aggregate slots;
- * this assumes that the output slots corresponding to aggregate exprs
have already
Review Comment:
ditto.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
