Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/24439 )
Change subject: KUDU-3782 avoid unsigned underflow in MaterializingIterator::Init ...................................................................... Patch Set 2: (4 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)); > +1 Done 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() > +1 Ah yes, thank you! I removed the subtraction altogether and now reserve num_columns, which is a safe upper bound for non_predicate_column_indexes_ and avoids both the cast and the underflow. http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators.cc@1193 PS1, Line 1193: const ColumnPredicate& pred = col_p > Could there be a case where the number of columns in the projection is less This scenario is fine for two reasons. First, there is no COUNT, so the scan returns a real column (email), meaning the columns side is not zero. Second, and more importantly, tenant_id and user_id are the first two key columns with no gap, so both filters collapse into a single key range. Nothing is left stranded on a non returned column, so there is no underflow. The crash needs a COUNT plus a gap in the key filter, so a filter gets stranded on a column the scan is not returning. http://gerrit.cloudera.org:8080/#/c/24439/1/src/kudu/common/generic_iterators.cc@1222 PS1, Line 1222: std::iota(non_predicate_column_indexes_.begin(), > actually there is a TODO mentioning this scenario in tablet-pushdown-test.c I have added the VLOG in the skip branch. For the removal: Fully supporting filters on columns that are not in the projection is larger than this crash fix, so I've filed KUDU-3784 to track it and added a TODO in the skip branch. For this patch, skipping is safe on the server scan path because TabletServiceImpl adds every predicate column to the projection via ScanSpec::GetMissingColumns(), so the only out-of-projection predicates here are the redundant ones lifted from the rowset's primary key bounds, which the key range already enforces. -- 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: 2 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 09:31:32 +0000 Gerrit-HasComments: Yes
