[IMPALA-7604] In AggregationNode.computeStats, handle cardinality overflow better

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: Impala 2.12.0
• Fix Version/s: Impala 3.4.0
• Component/s: Frontend
• Target Version: Impala 3.4.0
• Epic Color: ghx-label-3
• Assignee: Tim Armstrong
• Reporter: Paul Rogers
• Created: 21/Sep/18 00:27
• Updated: 29/Aug/19 12:44
• Resolved: 28/Aug/19 22:52

Description

Consider the cardinality overflow logic in AggregationNode.computeStats(). Current code:

    // if we ended up with an overflow, the estimate is certain to be wrong
    if (cardinality_ < 0) cardinality_ = -1;

This code has a number of issues.

• The check is done after looping over all conjuncts. It could be that, as a result, the number overflowed twice. The check should be done after each multiplication.
• Since we know that the number overflowed, a better estimate of the total count is Long.MAX_VALUE.
• The code later checks for the -1 value and, if found, uses the cardinality of the first child. This is a worse estimate than using the max value, since the first child might have a low cardinality (it could be the later children that caused the overflow.)
• If we really do expect overflow, then we are dealing with very large numbers. Being accurate to the row is not needed. Better to use a double which can handle the large values.

Since overflow probably seldom occurs, this is not an urgent issue. Though, if overflow does occur, the query is huge, and having at least some estimate of the hugeness is better than none. Also, seems that this code probably evolved; this newbie is looking at it fresh and seeing that the accumulated fixes could be tidied up.