Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3524: Don't process spilled partitions with 0 probe rows ......................................................................
Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/5389/2/be/src/exec/partitioned-hash-join-node.cc File be/src/exec/partitioned-hash-join-node.cc: Line 387: return mem_tracker()->MemLimitExceeded( > Yeah I think maybe just making a counter like "NumHashTableBuildsSkipped" m Done http://gerrit.cloudera.org:8080/#/c/5389/3/be/src/exec/partitioned-hash-join-node.cc File be/src/exec/partitioned-hash-join-node.cc: Line 379: // If there are no probe rows, there's no need to build the hash table, and > The logic for !NeedToProcessUnmatchedBuildRows() is a bit convoluted - we s Done Line 665: // This will only be called for partitions that are added to 'output_build_partitions_' > extra space Done PS3, Line 666: processed unti > OutputAllBuild. Done PS3, Line 667: here > must Done PS3, Line 666: ch adds one partition that is then processed until : // it is done by the loop in GetN > Could condense - I don't think the bit about multiple calls is needed to ex Done Line 680: output_unmatched_batch_iter_.reset( > Could avoid reallocating by assigning *output_unmatched_batch_iter_ = RowBa I don't think that works, due to the const members in RowBatch::Iterator Line 705: return Status::OK(); > Extra blank line here. Done http://gerrit.cloudera.org:8080/#/c/5389/3/be/src/exec/partitioned-hash-join-node.h File be/src/exec/partitioned-hash-join-node.h: PS3, Line 289: OutputUnmatchedBuild > OutputUnmatchedBuild() Done PS3, Line 480: put_unmatched_batch_; > typo Done -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
