Dan Hecht has posted comments on this change.

Change subject: IMPALA-5788: Fix agg node crash when grouping by 
nondeterministic exprs
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7714/3/be/src/exec/partitioned-aggregation-node.cc
File be/src/exec/partitioned-aggregation-node.cc:

Line 1160:   // partition index.
> Just to add to this, we would also have to make the same effort for hash jo
i agree it's best to not adding a check to the fast path. We can go forward 
with this fix, but please do add that comment referencing this jira to the 
header file.

Why isn't this a problem in the join?  Is it because we repartition into the 
stream first, and then build hashtables over a single stream in the join case? 
whereas in agg we're repartitioning and building the table in the same pass?

Ultimately I think we should eventually reject these queries during analysis 
(e.g. IMPALA-4605), but that's obviously out of scope for now.


http://gerrit.cloudera.org:8080/#/c/7714/3/be/src/exec/partitioned-aggregation-node.h
File be/src/exec/partitioned-aggregation-node.h:

PS3, Line 338: all pointers in this vector will point to a single
             :   /// in-memory partition
> You are absolutely right. This is very specific to our bug fix. would it ma
I think adding a reference to the JIRA makes sense in this case since it's so 
specific to that.


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to