Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16777 )
Change subject: KUDU-3108: fix invalid memory accesses in merge iterator ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/16777/7/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/16777/7/src/kudu/common/generic_iterators.cc@565 PS7, Line 565: // The min-heaps as described in the algorithm above. > No ordered set anymore? Done http://gerrit.cloudera.org:8080/#/c/16777/7/src/kudu/common/generic_iterators.cc@677 PS7, Line 677: bool pulled_new_block = false; > Shouldn't be necessary: if you don't return from Advance, pulled_new_block You're right, but it seems like tidy-bot struggles to make sense of the fact that we're checking for an OK condition twice, and is therefore incorrectly flagging that the we can read a garbage value. http://gerrit.cloudera.org:8080/#/c/16777/7/src/kudu/common/generic_iterators.cc@884 PS7, Line 884: // Since we're advancing iterators of the same row starting with hot_.top(), > This is an interesting, but is it relevant to understanding MaterializeOneR I found it unclear without this tidbit that 'state' was always must be equivalent to hot_.top(). Otherwise, it wasn't clear to me why we we could simply pop() the hot heap in AdvanceAndReheap() when we're examining 's'. -- To view, visit http://gerrit.cloudera.org:8080/16777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ec1cd3fd67ec4ea92a55b5b0ce555123748824d Gerrit-Change-Number: 16777 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Lieber-Dembo <a...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Sun, 29 Nov 2020 05:14:12 +0000 Gerrit-HasComments: Yes
