Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24439 )
Change subject: KUDU-3782 avoid unsigned underflow in MaterializingIterator::Init ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators-test.cc File src/kudu/common/generic_iterators-test.cc: http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators-test.cc@597 PS1, Line 597: ASSERT_FALSE(dst.selection_vector()->IsRowSelected(0)); > nit: +1 http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators.cc@1177 PS1, Line 1177: num_columns() > This already returns size_t and the casting below can be avoided? +1 http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators.cc@1222 PS1, Line 1222: spec->RemovePredicates(); RemovePredicates() clears all predicates from the spec, including the ones we skipped (the out-of-projection ones). This means no higher layer will evaluate them either. The commit message argues these are already enforced by the rowset's key range. Is it worth adding a VLOG(1) in the continue branch (like the existing VLOG(1) on line 1210) to make it observable when a predicate is skipped? Something like: VLOG(1) << "Skipping out-of-projection predicate " << pred.ToString(); Useful for debugging production queries that return unexpected results. -- To view, visit http://gerrit.cloudera.org:8080/24439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I646ab64c2139b37c7e29152695805bf28ef1dd20 Gerrit-Change-Number: 24439 Gerrit-PatchSet: 1 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 12 Jun 2026 08:06:47 +0000 Gerrit-HasComments: Yes
