Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15463 )
Change subject: IMPALA-9530: query option to limit preagg memory ...................................................................... Patch Set 2: (3 comments) mostly minor comments, overall LGTM http://gerrit.cloudera.org:8080/#/c/15463/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/15463/2/be/src/service/query-options.cc@923 PS2, Line 923: preaggregation bytes nit: 'preaggregation bytes limit' http://gerrit.cloudera.org:8080/#/c/15463/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java File fe/src/main/java/org/apache/impala/planner/AggregationNode.java: http://gerrit.cloudera.org:8080/#/c/15463/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@581 PS2, Line 581: useStreamingPreagg_ IIUC, the preagg_bytes_limit won't be applied if DISABLE_STREAMING_PREAGGREGATIONS=true, right? should that be documented somewhere? http://gerrit.cloudera.org:8080/#/c/15463/2/testdata/workloads/tpch/queries/tpch-passthrough-aggregations.test File testdata/workloads/tpch/queries/tpch-passthrough-aggregations.test: http://gerrit.cloudera.org:8080/#/c/15463/2/testdata/workloads/tpch/queries/tpch-passthrough-aggregations.test@99 PS2, Line 99: nit: whitespace -- To view, visit http://gerrit.cloudera.org:8080/15463 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I87f7a5c68da93d068e304ef01afbcbb0d56807d9 Gerrit-Change-Number: 15463 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 18 Mar 2020 18:42:28 +0000 Gerrit-HasComments: Yes
