[IMPALA-7899] RuntimeFilterGenerator uses Preconditions for normal conditions

• Type: Bug
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: Impala 3.0
• Component/s: Frontend
• Epic Color: ghx-label-1
• Assignee: Unassigned
• Reporter: Paul Rogers
• Created: 27/Nov/18 01:44
• Updated: 05/Oct/19 21:13

Description

The RuntimeFilterGenerator.computeTargetExpr() checks if a value can be cast to a given type. It does so by calling Expr.castTo(), which uses the following technique to report that the cast is not valid:

      Preconditions.checkArgument(targetType.equals(type),
          "targetType=" + targetType + " type=" + type);

This condition occurs in PlannerTest. So, if I have a breakpoint set to catch Preconditions failures, that breakpoint is hit on every run. I have to remember, "oh, yeah, it is OK that we are violating this invariant."

In general, it is bad practice to use things like assert, Preconditions or unchecked exceptions to report "expected" conditions: it muddies the water about what is an invariant or is not.

Instead, use a checked, purpose-built exception, such as SqlCastException, to report such errors, and include comments that the exception will be thrown for expected illegal casts.

Or, better, restructure the method so that there is one version that is expected to succeed (the current one), and another that returns null if the cast is not legal. Have the enforcing version call the returning-null version.