Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 )
Change subject: IMPALA-9127: explicit probe state machine in hash join ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/14688/5/be/src/exec/partitioned-hash-join-node.h File be/src/exec/partitioned-hash-join-node.h: http://gerrit.cloudera.org:8080/#/c/14688/5/be/src/exec/partitioned-hash-join-node.h@204 PS5, Line 204: beein typo http://gerrit.cloudera.org:8080/#/c/14688/5/be/src/exec/partitioned-hash-join-node.cc File be/src/exec/partitioned-hash-join-node.cc: http://gerrit.cloudera.org:8080/#/c/14688/5/be/src/exec/partitioned-hash-join-node.cc@522 PS5, Line 522: case ProbeState::PROBING_IN_BATCH: { optional: I think that it would be nice to move this and the next case into different functions, e.g. ProcessCurrentProbeBatch() and NextProbeBatch() (and probably rename NextProbeRowBatch() to get NextProbeRowBatchFromChild() or something similar). The functions could be ordered like the old logic to reduce the diff. -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 19 Nov 2019 16:24:42 +0000 Gerrit-HasComments: Yes
