[IMPALA-7781] Clean up ad-hoc instance of, casts to use predicates in expr rewriter

• Type: Bug
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: Impala 3.0
• Component/s: Frontend
• Epic Color: ghx-label-3
• Assignee: Unassigned
• Reporter: Paul Rogers
• Created: 29/Oct/18 20:27
• Updated: 02/Nov/18 19:34

Description

The expression rewriter rules have evolved over time, it seems. When originally written, it seem that the standard way to check if an expression is a null literal is to do an instance of as in SimplifyConditionalsRule:

  private Expr simplifyCaseExpr(CaseExpr expr, Analyzer analyzer)
      throws AnalysisException {
    ...
      if (child instanceof NullLiteral) continue;

Since this was written, we added Expr.isNullLiteral() which not only checks if the expression is null, it also tests for the CAST(NULL AS <type>) form created by the constant folding rule.

The result is that rewrites miss optimization cases for expressions such as NULL + 1 which are rewritten to CASE(NULL AS INT). (IMPALA-7769).

Code also does manual casts to Boolean literals in the same function:

      if (whenExpr instanceof BoolLiteral) {
        if (((BoolLiteral) whenExpr).getValue()) {

Which can be replaced with the Expr.IS_TRUE_LITERAL predicate. Same is true for the FALSE check.

Finally, there are places in the rewriter that check isLiteral() when it wants to know a more generic "is near literal". Consider the CAST(NULL...) issue above. The CAST is not a literal, but it acts like one in some cases (such as in the constant folding rule itself, IMPALA-7769.)

In short, a number of minor, obscure errors could be avoided if we made consistent use of the higher-level predicates already available.

Issue Links

• is part of: IMPALA-7747 Clean up the Expression Rewriter