[
https://issues.apache.org/jira/browse/HIVE-30031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113579#comment-18113579
]
Konstantin Bereznyakov commented on HIVE-30031:
-----------------------------------------------
Precision note on the fallback default: Hive 4.0.0 shipped
{{hive.cbo.fallback.strategy=CONSERVATIVE}} (HiveConf at rel/release-4.0.0,
which also still accepted the {{TEST}} value). HIVE-27831 changed the default
to NEVER in 4.1.0. So "NEVER, the default" in the table applies to 4.1.0 and
later, including master. On 4.0.x the default masks the shape 3 failure:
{{recompile_without_cbo}} recompiles without CBO and returns the legacy row, as
described in the CONSERVATIVE paragraph. Shapes 1 and 2 on that line are
unaffected by the setting.
> numeric = boolean comparison: inconsistencies between CBO and non-CBO
> ---------------------------------------------------------------------
>
> Key: HIVE-30031
> URL: https://issues.apache.org/jira/browse/HIVE-30031
> Project: Hive
> Issue Type: Bug
> Components: CBO, Query Planning
> Affects Versions: 4.0.0
> Reporter: Konstantin Bereznyakov
> Priority: Major
> Attachments: bool_int_shape1.q, bool_int_shape2.master.q.out,
> bool_int_shape2.q, bool_int_shape3.q,
> bool_int_shape3_conservative.master.q.out
>
>
> h3. Summary
> For {{numeric_col = TRUE}}, {{hive.cbo.enable}} changes the outcome of every
> WHERE-clause shape below (rows, or success versus failure), and under CBO the
> same term is accepted in one shape and fails in two others. The legacy
> planner is consistent (TRUE is compared as 1). Root cause: Hive's Rex
> translation finds no common comparison type for (bigint, boolean) and emits
> {{=(BIGINT, BOOLEAN)}} without a cast; Calcite's simplification rules assume
> a well-typed tree. Verified with the attached q files on master 9019223a86
> (Calcite 1.42.0) and on a 4.0.0 build (Calcite 1.25.0).
> h3. Reproduction
> {code:sql}
> create table bool_int_cmp (code bigint, ts bigint);
> insert into bool_int_cmp values (200, 10), (404, 20), (0, 30), (1, 40);
> set hive.fetch.task.conversion=none;
> select code from bool_int_cmp where code = true; /* shape 1 */
> select code from bool_int_cmp where ts > 0 and code = true; /* shape 2:
> unrelated conjunct */
> select code from bool_int_cmp where code > -1 and code = true; /* shape 3:
> same-column range */
> {code}
> The row {{code=1}} discriminates the candidates: TRUE-equals-1 returns only
> 1; nonzero-is-true returns 200, 404, 1.
> h3. Observed ({{hive.cbo.fallback.strategy=NEVER}}, the default)
> || shape || cbo=true, master (1.42.0) || cbo=true, 4.0.0 (1.25.0) ||
> cbo=false, both ||
> | 1 {{code = true}} | AssertionError "Cannot add expression of different type
> to set" in {{HiveProjectFilterPullUpConstantsRule}} | folds {{code}} to
> {{1L}}, returns 1 | {{(code = true)}}, returns 1 |
> | 2 {{ts > 0 and code = true}} | {{((ts > 0L) and UDFToBoolean(code))}},
> returns 200, 404, 1 | same | {{((ts > 0L) and (code = true))}}, returns 1 |
> | 3 {{code > -1 and code = true}} | ClassCastException Boolean to BigDecimal
> in {{RexSimplify.residue}} (line 2173) | same ({{residue}} line 1692) |
> {{((code > -1L) and (code = true))}}, returns 1 |
> With {{CONSERVATIVE}} (same files): shape 3 returns 1 on both builds because
> {{ReCompileWithoutCBOPlugin}} recompiles without CBO (the CLI prints
> "[ReExecDriver] ReCompileWithoutCBOPlugin triggered re-compile"); shape 2 is
> unchanged, there is no exception to fall back from; shape 1 on master still
> fails, because the AssertionError is an {{Error}} and skips the {{catch
> (Exception e)}} in {{CalcitePlanner.genOPTree}} (CalcitePlanner.java:665), so
> no fallback strategy can intercept it. The fallback setting thus decides the
> semantics of shape 3 while shape 2 keeps CBO's reading.
> {code}
> java.lang.AssertionError: Cannot add expression of different type to set:
> set type is RecordType(BIGINT code) NOT NULL
> expression type is RecordType(BIGINT NOT NULL code) NOT NULL
> expression is HiveProject(code=[1:BIGINT])
> HiveFilter(condition=[=($0, true)])
> at
> org.apache.calcite.plan.RelOptUtil.verifyTypeEquivalence(RelOptUtil.java:436)
> at
> org.apache.calcite.plan.hep.HepRuleCall.transformTo(HepRuleCall.java:58)
> at
> org.apache.hadoop.hive.ql.optimizer.calcite.rules.HiveProjectFilterPullUpConstantsRule.onMatch(HiveProjectFilterPullUpConstantsRule.java:94)
> {code}
> {code}
> java.lang.ClassCastException: class java.lang.Boolean cannot be cast to class
> java.math.BigDecimal
> at java.base/java.math.BigDecimal.compareTo(BigDecimal.java:309)
> at
> org.apache.hive.com.google.common.collect.Range.compareOrThrow(Range.java:679)
> at
> org.apache.hive.com.google.common.collect.ImmutableRangeSet.encloses(ImmutableRangeSet.java:165)
> at org.apache.calcite.rex.RexSimplify.residue(RexSimplify.java:2173)
> at
> org.apache.calcite.rex.RexSimplify.simplifyUsingPredicates(RexSimplify.java:2096)
> at
> org.apache.calcite.rex.RexSimplify.simplifyComparison(RexSimplify.java:781)
> at
> org.apache.calcite.rel.rules.ReduceExpressionsRule.reduceExpressions(ReduceExpressionsRule.java:729)
> {code}
> h3. Analysis
> * *Root.* {{HiveFunctionHelper.convertInputs}} (type-check path) and
> {{RexNodeConverter}} (ExprNodeDesc path) take the cast target from
> {{FunctionRegistry.getCommonClassForComparison}}. For (bigint, boolean) every
> branch falls through, boolean being implicitly convertible to no numeric
> type, and the method returns null, so the operands pass through unconverted.
> Per the CALCITE-2745 discussion, Rex is strictly typed and the SQL-to-Rel
> layer must insert the casts; here that layer is Hive.
> * *Shape 1 (master only).* {{HiveProjectFilterPullUpConstantsRule}} replaces
> the projected {{$0}} with the filter constant through
> {{rexBuilder.makeCast(fieldType, constant, true)}} ({{RexReplacer.visit}}).
> For a BOOLEAN constant and an exact-numeric field the result is the
> non-nullable literal {{1:BIGINT}}, the row type changes from BIGINT to BIGINT
> NOT NULL, and {{RelOptUtil.verifyTypeEquivalence}} throws an explicit
> AssertionError (not a JVM assert). 4.0.0 folds the same query to {{1L}} and
> returns 1.
> * *Shape 2.* {{RexSimplify.simplifyAnd2ForUnknownAsFalse}} (1.42.0 lines
> 1887-1899, same in 1.25.0) strips {{= TRUE}} from an AND term whenever one
> operand is always true, without checking that the other operand is BOOLEAN
> ({{simplifyComparison}}, line 677, does check). The plan becomes
> {{HiveFilter(condition=[AND(>($1, 0), $0)])}} with a bare BIGINT operand,
> which Hive's operator-tree translation wraps as {{UDFToBoolean(code)}}:
> nonzero is true. Excluding the ReduceExpressions rules with
> {{hive.cbo.rule.exclusion.regex}} removes the rewrite, and the query then
> fails like shape 1.
> * *Shape 3.* {{RexSimplify.simplifyUsingPredicates}} derives a
> {{Range<BigDecimal>}} from {{code > -1}}; {{residue}} compares the Boolean
> literal against it and throws. Same class as CALCITE-2745 (open since 2018),
> sibling site; present in 1.25.0 and 1.42.0.
> * *Legacy planner.* {{GenericUDFBaseCompare.initialize}} gets the same null
> and takes the "for now, we always convert to double" fallback (HIVE-2248,
> 2011), so TRUE is compared as 1.0. Consistent, but undocumented.
> * *Documentation.* The Relational Operators table lists {{A = B}} for all
> primitive types; the Allowed Implicit Conversions matrix has no boolean to
> numeric or numeric to boolean cell. The comparison is documented as legal and
> its meaning as undefined.
> h3. Proposed fix
> Hive already rejects a fixed list of unsafe comparisons by default through
> {{hive.strict.checks.type.safety}} (bigint versus string, char, varchar,
> double; decimal versus string, char, varchar), checked once per comparison in
> {{TypeCheckProcFactory.validateUDF}}, which both planners share; with the
> flag off it warns and proceeds.
> * (a) Add the boolean versus numeric-group pairs to that check
> ({{TypeInfoUtils.LOSSY_TYPE_CONVERSIONS}}, or a sibling set with an accurate
> message). Default behavior becomes a clean SemanticException on both
> planners; no new configuration.
> * (b) With the flag off, cast the boolean operand to the numeric operand's
> type in {{HiveFunctionHelper.convertInputs}} / {{RexNodeConverter}}, so CBO
> gives the legacy TRUE-equals-1 result (Spark's {{BooleanEquality}} coercion
> does the same and attributes it to Hive).
> Either half alone removes both planner failures for this operand pair;
> together, all shapes agree on both planners under both flag values.
> Independently of the semantics, {{HiveProjectFilterPullUpConstantsRule}}
> should not accept a replacement whose nullability differs from the field
> (skip it, or cast preserving nullability). The Calcite-side gaps are noted on
> CALCITE-2745.
> h3. Attachments
> bool_int_shape1.q, bool_int_shape2.q, bool_int_shape3.q (each runs the shape
> with cbo on, then off); bool_int_shape2.master.q.out (the nonzero-is-true
> plan and rows next to the legacy plan);
> bool_int_shape3_conservative.master.q.out (the recompile console line and
> legacy predicate). Further outputs, including the 4.0.0 build's, available on
> request.
> h3. Related
> HIVE-2248 (double fallback in the comparison UDFs; HIVE-1638 introduced
> them); HIVE-13861, HIVE-20913, HIVE-29447 (nullability in constant pull-up
> and simplification); CALCITE-2745 (RexSimplify ClassCastException on
> mixed-type comparison operands).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
