Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12205 )
Change subject: KUDU-2645: tablet: Support deduplication of deleted rows in MergeIterator ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc File src/kudu/common/generic_iterators-test.cc: http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc@287 PS8, Line 287: ASSERT_EQ(1, expected_ints_deleted.erase(potential)); > Nit: you can upgrade this to CHECK_EQ; the ContainsKey() check on L286 guar Done http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc@369 PS8, Line 369: int is_deleted_idx = schema.find_first_is_deleted_virtual_column(); > This can be pulled out of the loops. Done http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators.cc@39 PS8, Line 39: #include "kudu/common/common.pb.h" > Any idea what this inclusion is for? Yeah, it's for IS_DELETED -- To view, visit http://gerrit.cloudera.org:8080/12205 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00614b3fa5c6b4e7b620bb78489e24c5ad44daee Gerrit-Change-Number: 12205 Gerrit-PatchSet: 8 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 06 Mar 2019 03:51:45 +0000 Gerrit-HasComments: Yes
