[ 
https://issues.apache.org/jira/browse/KUDU-3782?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18089151#comment-18089151
 ] 

ASF subversion and git services commented on KUDU-3782:
-------------------------------------------------------

Commit 9abc5a5e3c3d3c3a9f1807817fbdbecbd9125129 in kudu's branch 
refs/heads/master from Gabriella Lotz
[ https://gitbox.apache.org/repos/asf?p=kudu.git;h=9abc5a5e3 ]

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]>


> TServer aborts (SIGABRT / std::length_error) on COUNT(*) scans with predicates
> ------------------------------------------------------------------------------
>
>                 Key: KUDU-3782
>                 URL: https://issues.apache.org/jira/browse/KUDU-3782
>             Project: Kudu
>          Issue Type: Bug
>          Components: tserver
>            Reporter: Gabriella Lotz
>            Assignee: Gabriella Lotz
>            Priority: Critical
>
> h3. Summary
> When a tablet server serves a counting scan
> {noformat}
> COUNT(*) or COUNT(<constant>), {noformat}
> that also carries column predicates, MaterializingIterator::Init (in 
> src/kudu/common/generic_iterators.cc) computes the number of projected 
> columns minus the number of predicates using mismatched signed/unsigned 
> types. When the projected column count is smaller than the predicate count, 
> the subtraction underflows to a very large value, which is passed to 
> std::vector::reserve, throwing an uncaught std::length_error and aborting the 
> process.
>  
> This happens because the projection for a counting scan is tiny, while during 
> initialization the rowset iterator (CFileSet::Iterator::OptimizePKPredicates 
> → LiftPrimaryKeyBounds) re-adds predicates derived from the rowset's primary 
> key bounds on key columns that are not in the projection. The guarding 
> DCHECK_GE is compiled out in release builds, so release builds reach the 
> underflow. It is intermittent because it depends on the rowset's on-disk key 
> layout.
> Trigger: a 
> {code:java}
> COUNT(*)/COUNT(<const>){code}
> query with predicates on a non-contiguous subset of a composite primary key, 
> against a rowset whose primary key bounds share a long constant prefix.
> h3. Fix (in MaterializingIterator::Init)
> Remove the invalid DCHECK_GE, compute the reservation so it cannot underflow, 
> and skip predicates whose column is not in the projection (these are 
> rowset-derived and already enforced by the key range) instead of failing the 
> scan. A regression test covering a projection smaller than the predicate set 
> passes with this change, and the existing generic_iterators-test suite 
> continues to pass.
> h3. Workaround
> Rewrite affected counting queries so the scan projects real columns, for 
> example by wrapping the count in a sub-query that selects actual rows. A 
> non-empty projection avoids the code path entirely. Only COUNT(...) queries 
> that include a WHERE clause can trigger this.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to