Tim Armstrong has posted comments on this change. Change subject: IMPALA-5788: Fix agg node crash when grouping by nondeterministic exprs ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/7714/2/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: Line 1160: // partition index nit: period at end of sentence for consistency. PS2, Line 1163: hashTable hash_table. Although I don't think we really need this temporary variable in the first place. Line 1407: // We might be dealing with a rebuilt spilled partition, where all partitions are Maybe add something to the start of this to explain the broader purpose (otherwise it sounds like the code is only dealing with this corner case). Maybe something like "Remove references to the destroyed hash table from 'hash_tbls_'. Line 1409: // with it as well nit: end with period for consistency. Line 1426: // right partition nit: end with period for consistency. http://gerrit.cloudera.org:8080/#/c/7714/2/testdata/workloads/functional-query/queries/QueryTest/spilling.test File testdata/workloads/functional-query/queries/QueryTest/spilling.test: Line 352: group by 1,2,3,4,5, random() nit: add spaces after commas for consistency (this was from my original sloppy formatting). Line 354: ---- RUNTIME_PROFILE It would be nice to have a RESULTS section to verify that the query isn't returning anything crazy, but I can't think of a good way to both have this non-deterministic behaviour and return consistent results. -- To view, visit http://gerrit.cloudera.org:8080/7714 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibdb09239577b3f0a19d710b0d148e882b0b73e23 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
