[Impala-ASF-CR] IMPALA-3524: Don't process spilled partitions with 0 probe rows

["Tim Armstrong (Code Review)" <ge...@cloudera.org>]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3524: Don't process spilled partitions with 0 probe rows
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

I'm happy that the issue I mentioned is fixed. I might play around a bit to see if I can repro it.

While looking at the code, I noticed that the limit wasn't correctly applied on this code path. However, this is a pre-existing bug - IMPALA-4866 - that affects most of the code paths in partitioned-hash-join-node.cc so I'm happy to defer fixing that.

http://gerrit.cloudera.org:8080/#/c/5389/6/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 677:     if (output_unmatched_batch_iter_->AtEnd()) {
> Is there a dcheck we could add to detect the incorrect state?
We could add a DCHECK(!output_unmatched_batch_->needs_deep_copy()) before the Reset() but that is trivially true given we just transferred the resources.


-- 
To view, visit http://gerrit.cloudera.org:8080/5389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I175b32dd9031e51218b38c37693ac3e31dfab47b
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes