Alexey Serbin has uploaded this change for review. (
http://gerrit.cloudera.org:8080/24488
Change subject: KUDU-3782 avoid unsigned underflow in
MaterializingIterator::Init
......................................................................
KUDU-3782 avoid unsigned underflow in MaterializingIterator::Init
A "counting" scan (e.g. SELECT count(...)) projects no columns, so the
MaterializingIterator's schema can have fewer columns than the number of
predicates in the ScanSpec. This happens because a lower-level iterator,
CFileSet::Iterator::OptimizePKPredicates(), can lift additional predicates
from a rowset's primary key bounds onto key columns that are not part of
the scan projection. It is most pronounced for counting scans whose
projection is empty or tiny.
In that case MaterializingIterator::Init() computed:
non_predicate_column_indexes_.reserve(
num_columns - spec->predicates().size());
where num_columns is int32_t and predicates().size() is size_t. When
num_columns < predicates().size() the subtraction is evaluated in unsigned
arithmetic and wraps to a value near SIZE_MAX, so vector::reserve() throws
std::length_error. The exception is not caught, so the tablet server
aborts with SIGABRT. The preceding DCHECK_GE(num_columns,
predicates().size()) encodes an invariant that does not actually hold and
is compiled out in release builds.
This patch:
- removes the invalid DCHECK_GE;
- reserves an upper bound (num_columns) instead of the difference, which
cannot underflow;
- skips predicates whose column is not in the projection rather than
returning InvalidArgument. Such predicates are derived from the
rowset's primary key bounds and are already enforced by the key range,
so skipping them here is safe.
Also adds a regression test that initializes a MaterializingIterator with
more predicates than projected columns (including a predicate on a column
outside the projection) and verifies that Init() succeeds and that the
in-projection predicate still filters correctly.
Change-Id: I646ab64c2139b37c7e29152695805bf28ef1dd20
Reviewed-on: http://gerrit.cloudera.org:8080/24439
Tested-by: Kudu Jenkins
Reviewed-by: Marton Greber <[email protected]>
Reviewed-by: Zoltan Chovan <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 9abc5a5e3c3d3c3a9f1807817fbdbecbd9125129)
---
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
2 files changed, 76 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/24488/1
--
To view, visit http://gerrit.cloudera.org:8080/24488
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.18.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I646ab64c2139b37c7e29152695805bf28ef1dd20
Gerrit-Change-Number: 24488
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
