impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5788: Fix agg node crash when grouping by nondeterministic exprs
Date Tue, 22 Aug 2017 20:16:54 GMT
Tim Armstrong has posted comments on this change.

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


Patch Set 3:

(1 comment)

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.
> Yes, in case of repartition, the rows do get shuffled around in each iterat
Repartitioning should work ok in that each repartitioning step will reduce the size of partitions
- each row ends up in an arbitrary partition. The end result is pretty much arbitrary though.

I thought this was a reasonable solution. I would be ok with a solution that failed queries
with nondeterministic grouping functions but I couldn't think of another simpler solution
that met the following two constraints:
* Guarantees that a nondeterministic UDF or builtin can't crash Impala
* Doesn't impose overhead on the "fast path".

Runtime checks for whether the row mapped to the right partition would impose some runtime
overhead. We could avoid that by codegen'ing a different version of the probe function for
the case when we're processing a single spilled partition, but then that would add codegen
overhead.


-- 
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 <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message