HIVE-29641: Upgrade Calcite to 1.42.0

[rubenada]

What changes were proposed in this pull request?

Upgrade Calcite to 1.42.0.

Why are the changes needed?

Upgrade to latest Calcite version.

Does this PR introduce any user-facing change?

Plan changes, see details below.

How was this patch tested?

Running all existing tests, making test adjustments where needed, see details below.

Details

pom changes

• parent pom.xml: change Calcite version (from 1.33.0 to 1.42.0)
• ql/pom.xml: Add httpclient5/httpcore5 as runtime dependencies to fix NoClassDefFoundError: org/apache/hc/core5/util/Timeout when running certain tests (see more info on the comment of this file)
• ql/pom.xml: Add org.jooq:joou-java-6 to fix NoClassDefFoundError: org/joou/UShort (seen on Jenkins init-metastore); this is due to "[CALCITE-1466] Support for UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT in type system" (1.41)
• druid-handler/pom.xml: remove Calcite shade include and relocation to avoid Error in ASM processing class org/apache/hive/druid/org/apache/calcite/runtime/SqlFunctions.class: Index 65536 out of bounds for length 334 during shading / javadoc; the problem originates due to invalid byte code on Calcite: "[CALCITE-6393] Byte code of SqlFunctions is invalid" (already fixed on Calcite side for 1.43.0, so this workaround could be removed on a subsequent upgrade to said version).

Code adjustments due to Calcite breaking changes

• "[CALCITE-5557] Add SAFE_CAST function (enabled in BigQuery library)" (1.35)
  Use non-deprecated version of RexBuilder#makeAbstractCast in HiveSqlSumAggFunction, RexNodeConverter, ASTConverter
• "ddb4200 Refactor: Add fields AggregateCall.rexList and RelBuilder.AggCall.preOperands" (1.35)
  AggCall construction adjustments in HiveAggregate and HiveSqlSumAggFunction
• "[CALCITE-3679] Allow lambda expressions in SQL queries" (1.37)
  Add visitLambda and visitLambdaRef in RexVisitor implementation (HiveCalciteUtil#ConstantFinder)
• "[CALCITE-6961] Support LogicalRepeatUnion in RelShuttle" + "[CALCITE-6959] Support LogicalAsofJoin in RelShuttle" (1.40)
  Add visit(LogicalRepeatUnion) and visit(LogicalAsofJoin) to HiveRelShuttleImpl
• "[CALCITE-5387] Type-mismatch on nullability in JoinPushTransitivePredicatesRule RelRule" (1.40)
  Adjust RexPermuteInputsShuttle constructor in HiveJoinConstraintsRule
• "[CALCITE-7029] Support DPhyp to handle various join types" (1.41)
  Add visitNodeAndFieldIndex in RexVisitor implementation (HiveCalciteUtil#ConstantFinder)
• "[CALCITE-7351] Make getMaxNumericScale() and getMaxNumericPrecision() final" (1.42)
  Adjust HiveTypeSystemImpl

Other code changes

• Hive.java: Update Calcite default charset system property (use UTF16 instead of the default ISO-8859-1), otherwise we get "charset pollution" on char literals in explained plans (e.g. _UTF-16LE'1' instead of simply '1') due to the modification in SqlImplementor#toSql(RexLiteral) via "[CALCITE-6006] RelToSqlConverter loses charset information" (introduced in 1.36)
  As a consequence of defining Calcite default charset system property, now RexLiteral#appendAsJava (which already contained the literal charset vs default charset verification, added to SqlImplementor in CALCITE-6006 1.36), literals printed via this method (i.e. used to print the charset) don't do it any more, e.g. _UTF-16LE'ten' => 'ten', which aligns them with the ones printed via SqlImplementor#toSql(RexLiteral)
• Due to "7fc3e1b Refactor: Add RelNode.stripped" (included in Calcite 1.35), which added a new check in HepRelVertex constructor (inner rel cannot be another HepRelVertex), we basically cannot apply a HepProgram on a tree containing already HepVertex. This is violated by HiveHepExtractRelNodeRule, so all its callers now use an existing auxiliary method for this unwrapping (HiveCalciteUtil.stripHepVertices).
  This was also violated by materialization rules in HiveMaterializedViewRule, which used its own private static class HiveHepExtractRelNodeRule in a HepProgram as unionRewritingPullProgram. As a workaround, it is proposed to extend the MaterializedView rules (as HiveMaterializedViewRule), override the rewriteQuery
  (where the UnionRewritingPullProgram is called), and unwrap the HepRelVertex tree ourselves (using HiveCalciteUtil.stripHepVertices) before calling the super.rewriteQuery. This Calcite issue is being tracked via CALCITE-7641, (already fixed on Calcite side, so this workaround could be removed on a subsequent upgrade to Calcite 1.43.0).
• Add new case CHAR_LENGTH in SqlFunctionConverter#buildAST, since a new SqlKind.CHAR_LENGTH has been added (previously length functions had SqlKind.OTHER_FUNCTION) via "[CALCITE-6182] Add LENGTH/LEN function (enabled in Snowflake library)" (in Calcite 1.37). Problem observed in several tests, where length(x) function not being evaluated (returning 'length'literal instead of the actual value), e.g. TestJdbcDriver2#testExprCol: Illegal conversion to int from column 2 [length]
• Due to "[CALCITE-6044] RelMetadataQuery should regard single-row relational expressions as unique" (1.37), RelMdUniqueKeys can return now a set containing a single, empty set in cases like an empty Values or Values with 1 tuple; so we need to consider this circumstance when checking the uniqueKeys of Aggregate's input in HiveRelFieldTrimmer#generateNewGroupset, otherwise it can convert a HiveAggregate(group=[{0}], COUNT)
  into HiveAggregate(group=[{}], COUNT) which is NOT the same semantics when the input is an empty Values (empty resultset vs 0). Issue seen on TestMiniLlapLocalCliDriver with empty_result.q which was incorrectly generating a Project+Agg+EmptyValues plan instead of simply EmptyValues before fixing HiveRelFieldTrimmer#generateNewGroupset:

select count(a1) from t1 where 1=0 group by a1 order by a1;
< HiveProject(_c0=[$0])
<   HiveAggregate(group=[{}], agg#0=[count($0)])
<     HiveValues(tuples=[[]])
---
> HiveValues(tuples=[[]])

• The upgrade unveiled a bug in HiveRelFieldTrimmer#rewriteGBConstantKeys: the condition on its javadoc "none of the relnode above aggregate refers to these group keys" should not be checked via fieldsUsed.contains(aggregate.getGroupSet()) but rather fieldsUsed.intersects(aggregate.getGroupSet()).
  Also took the opportunity here to get rid of the unnecessary check aggregate.getIndicatorCount() > 0 which is always false (this method is deprecated and always returns zero)
• Disable simplifyValues in HiveRelBuilder config. Due to "[CALCITE-5717] RelBuilder.project of literals on a single-row Aggregate should create a Values" (introduced 1.35), cases like TestMiniLlapLocalCliDriver with file explainuser_1.q led to: java.lang.UnsupportedOperationException: Values with non-empty tuples are not supported. at org.apache.hadoop.hive.ql.optimizer.calcite.translator.ASTConverter.convert(ASTConverter.java:264). The reason is that RelBuilder#project_ contains the simplification (only applicable if config.simplifyValues is true): "If the expressions are all literals, and the input is a Values with N rows [...], replace with a Values with same tuple N times"; and with CALCITE-5717 that simplification was extended to not only "Values with N rows" but also "Aggregates with 1 row", and this case would lead to creating a non-empty HiveValues, which is not supported in our case. The easiest solution to prevent that from happening is disabling simplifyValues in HiveRelBuilder config.
• Due to a new assertion introduced in 1.41 via "[CALCITE-7070] FILTER_REDUCE_EXPRESSIONS crashes on expression BETWEEN ( NULL) AND X", now Calcite can throw an AssertionError in case of a wrong binary comparison predicate with null value java.lang.AssertionError: Comparison with NULL in pulledUpPredicates, so in order to avoid that, a few classes needed to be adjusted. Firstly HiveFunctionHelper#getExpression must not create such expressions (seen NULL = NULL in TestMiniLlapLocalCliDriver with subquery_null_agg.q); secondly avoid incorrect comparison ($snapshotIdInputRef <= NULL seen in TestIcebergCliDriver with mv_iceberg_orc.q) in HiveAugmentSnapshotMaterializationRule, use snapshotId -1 instead, and then switch it back in HivePushdownSnapshotFilterRule.
• Override HiveTypeSystemImpl#deriveDecimalDivideType because "[CALCITE-6464] Type inference for DECIMAL division seems incorrect" (introduced in 1.38) changed the default behavior to match MS-SQL-Server-style algorithm, which can cause a drop in the scale computation, hence a precision loss in certain cases. We override this method to keep the "old behavior" (pre-CALCITE-6464). Note: an alternative could be not overriding it (and keep the new Calcite default MS-SQL-style decimal-divide semantics), but that would lead to "regressions" (precision loss) and would require test adjustments, so not considered in the current patch.

Test plan adjustments

• Perf Imp 1 ) materialized_view_rewrite_4.q.out
  Plan improvement. On test query "EXAMPLE 32" MV rewriting was NOT applied so far (full join+aggregate plan, Stage-1 Tez with 2 maps + 2 reducers); but now it is: direct scan of mv1_n3 (Stage-0 Fetch only)

• Perf Reg 1 ) materialized_view_partitioned_2.q
  REGRESSION, most efficient alternative (partition_mv_3) is no longer used.
  Reason: mv partition_mv_3 PARTITIONED ON (key) is created as SELECT value, key FROM src_txn_2 where key > 220 and key < 230 The regression queries are ... where key > 224 and key < 226; and ... where key > 223 and key < 225; so partition_mv_3 should be used.
  However, in Calcite's SubstitutionVisitor#splitFilter, instead of returning the appropriate filter, it now returns null.
  The cause is that this line RexNode r = canonizeNode(rexBuilder, simplify.simplifyUnknownAsFalse(x2)); leads to an extra IS NOT NULL predicate on r, which makes it not equivalent to condition2 on the following line.
  The reason is an existing bug on a Calcite simplification: CALCITE-7635. This bug was so far "hidden", because this simplification was only applied for comparisons of the form ref op literal, and in our query they are literal op ref. But Calcite 1.35 introduced an improvement (CALCITE-5780) to support all comparisons formats, which enabled the problematic simplification that leads to the bug CALCITE-7635. This bug has already been fixed on Calcite side, so this shall be corrected with the eventual upgrade to Calcite 1.43.0.

• 0a ) Many plan changes of literals without charset, due to Calcite system property default charset definitions (see change on Hive.java above), after "CALCITE-6006: RelToSqlConverter loses charset information" (introduced in 1.36). Example:

<     HiveFilter(condition=[IN($15, '2000Q1', '2000Q2', '2000Q3')])
---
>     HiveFilter(condition=[IN($15, _UTF-16LE'2000Q1', _UTF-16LE'2000Q2', _UTF-16LE'2000Q3')])

• 0b ) Many plan differences due to Double format change, because of RexLiteral using Doubles instead of BigDecimal: "[CALCITE-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity" (included in 1.38). Example

<     WHERE `key` < 2.000E2 AND `ds` = '2008-04-08'
---
>     WHERE `key` < 200 AND `ds` = '2008-04-08'

• 1 ) New IS NOT NULL simplifications involving CAST, like in: dynamic_partition_pruning.q

<                   filterExpr: ((UDFToDouble(hour) = 11.0D) and hr is not null) (type: boolean)
---
>                   filterExpr: ((UDFToDouble(hour) = 11.0D) and CAST( UDFToInteger((hr / 2.0D)) AS STRING) is not null) (type: boolean)

Reason: "[CALCITE-5769] Optimizing 'CAST(e AS t) IS NOT NULL' to 'e IS NOT NULL'" (1.35) (only for "lossless" casts)

• 2 ) New IS NOT NULL simplifications, like in: merge_iceberg_copy_on_write_unpartitioned.q

<                   predicate: ((_col5 <> _col8) or (_col5 <= 100) or (_col5 is null or ((_col5 = _col8) and (_col5 > 100))) is null) (type: boolean)
---
>                   predicate: ((((_col5 <> _col8) or (_col5 <= 100)) and _col5 is not null) or (_col5 is null or ((_col5 = _col8) and (_col5 > 100))) is null) (type: boolean)

Reason: "[CALCITE-5639] RexSimplify should remove IS NOT NULL check when LIKE comparison is present" (actually more generic than just LIKE) (1.35)

• 3 ) Some IS [NOT] NULL simplifications have been removed, like in: explainuser_1

<                               predicate:(UDFToDouble(_col0) / _col1) is not null
---
>                               predicate:(_col1 is not null and UDFToDouble(_col0) is not null)

or

<                       predicate:(UDFToDouble(key) + 1.0D) is not null
---
>                       predicate:UDFToDouble(key) is not null

Reason: Some IS [NOT] NULL simplifications have been removed from RexSimplify for "non-safe" operations, e.g. division (if it could "hide" a runtime division by zero) or operations involving cast (only "lossless" casts can be simplified)
"[CALCITE-7145] RexSimplify should not simplify IS NULL(10/0)" (1.42)
"[CALCITE-7296] RexSimplify should not simplify IS (NOT) NULL(CAST(10/0 AS BIGINT))" (1.42)
"[CALCITE-7295] RexSimplify should simplify a division with a NULL argument" (1.42) — added a new x/NULL → NULL rule, but only when the surrounding context is "safe".
"[CALCITE-7032] Simplify 'NULL>ALL (ARRAY[1,2,NULL])' to 'NULL'" (1.41)

• 4 ) Simplifications around IS NOT TRUE, like: subquery_ANY

<                   expressions: _col1 (type: int), _col0 (type: string), (_col2 <> 0L) (type: boolean), (_col2 <= _col3) (type: boolean)
---
>                   expressions: _col1 (type: int), _col0 (type: string), (_col2 <> 0L) (type: boolean), (_col2 > _col3) is not true (type: boolean)

Reason: "[CALCITE-5798] Improve simplification of '(x < y) IS NOT TRUE' when x and y are not nullable" (1.35)

• 5 ) Fixed padding on fixed-length string literals inside OR/IN/SEARCH, like in: cbo_query12

<             HiveFilter(condition=[IN($12, 'Books                                             ', 'Jewelry                                           ', 'Sports                                            ')])
---
>             HiveFilter(condition=[IN($12, 'Books', 'Jewelry', 'Sports')])

Reason: "[CALCITE-4590] Incorrect query result with fixed-length string" (1.39)
NOTE: This fix only impacts predicates with OR/IN/SEARCH, and aligns these plans with single value condition ones (which already contained the padding, e.g. ql/src/test/results/clientpositive/perf/tpcds30tb/tez/cbo_query33.q.out)

• 6 ) Array construction adjustment, like in: insert_into_dynamic_partitions

< FROM TABLE(INLINE(ARRAY ((1, '101'), (2, '202'), (3, '303'), (4, '404'), (5, '505'))))
---
> FROM TABLE(INLINE(ARRAY[(1, '101'), (2, '202'), (3, '303'), (4, '404'), (5, '505')]))

Reason: "[CALCITE-6417] Map value constructor and Array value constructor unparsed incorrectly for HiveSqlDialect" (1.38)

• 7 ) New return type on MINUS in JSON, like in: query2.q TestExplainCBOFormattedCliDriver

<               ],
<               "type": {
<                 "type": "INTEGER",
<                 "nullable": true
<               }
---
>               ]

Reason: "[CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) for SqlKind.MINUS" (1.37)

• 8 ) New IS [NOT] DISTINCT FROM simplifications, like: orc_predicate_pushdown

<             predicate: ((t = -1Y) and (s like 'bob%')) (type: boolean)
---
>             predicate: ((t IS NOT DISTINCT FROM -1Y) and (s like 'bob%')) (type: boolean)

Reason: "[CALCITE-7160] Simplify AND/OR with DISTINCT predicates to SEARCH" (1.41)

• 9 ) Small fix on on multi_count_distinct_null.q

<     HiveProject($f0=[CASE(=($2, 1), 1, null:INTEGER)], $f1=[CASE(AND(=($2, 2), IS NOT NULL($1)), 1, null:INTEGER)])
---
>     HiveProject($f0=[CASE(=($2, 1), 1, null:INTEGER)], $f1=[null:INTEGER])

Reason: "[CALCITE-6317] Incorrect constant replacement when group keys are NULL" (1.37)

• 10 ) IS [NOT] TRUE/FALSE adjustments, like in: sharedwork

< WHERE (`t17`.`$f2` IS NULL OR `t17`.`c` IS NULL OR `t17`.`c` = 0) AND (`t17`.`p_type` IS NULL OR `t17`.`c` IS NULL OR `t17`.`c` = 0 OR (`t17`.`ck` < `t17`.`c` IS NULL OR NOT `t17`.`ck` < `t17`.`c`) OR `t17`.`$f2` IS NOT NULL) AND (`t17`.`c` IS NULL OR `t17`.`c` = 0 OR `t17`.`p_type` IS NOT NULL OR `t17`.`$f2` IS NOT NULL)
---
> WHERE (`t17`.`$f2` IS NULL OR `t17`.`c` IS NULL OR `t17`.`c` = 0) AND (`t17`.`p_type` IS NULL OR `t17`.`c` IS NULL OR `t17`.`c` = 0 OR `t17`.`ck` < `t17`.`c` IS NOT TRUE OR `t17`.`$f2` IS NOT NULL) AND (`t17`.`c` IS NULL OR `t17`.`c` = 0 OR `t17`.`p_type` IS NOT NULL OR `t17`.`$f2` IS NOT NULL)

Reason: "[CALCITE-7005] Invalid unparse for IS TRUE,IS FALSE,IS NOT TRUE and IS NOT FALSE in Hive/Presto Dialect" (1.40)

• 11 ) Filter predicates in different order, like in: materialized_view_rewrite_ssb.q,materialized_view_rewrite_ssb_2.q

<                   filterExpr: ((s_region = 'AMERICA') and (p_category = 'MFGR#12')) (type: boolean)
---
>                   filterExpr: ((p_category = 'MFGR#12') and (s_region = 'AMERICA')) (type: boolean)

• 12 ) NOT BETWEEN expression replaced by comparisons: udf_between

<           filterExpr: (((UDFToDouble(key) + 100.0D) < 100.0D) or ((UDFToDouble(key) + 100.0D) > 200.0D)) (type: boolean)
---
>           filterExpr: (UDFToDouble(key) + 100.0D) NOT BETWEEN 100.0D AND 200.0D (type: boolean)
...
<             predicate: (((UDFToDouble(key) + 100.0D) < 100.0D) or ((UDFToDouble(key) + 100.0D) > 200.0D)) (type: boolean)
---
>             predicate: (UDFToDouble(key) + 100.0D) NOT BETWEEN 100.0D AND 200.0D (type: boolean)

Reason: "[CALCITE-7194] Simplify comparisons between function calls and literals to SEARCH" (1.41)
The plan difference comes from:

1.33
BETWEEN -> RexSimplify: 100 <= x AND x <= 200 -> HivePointLookupOptimizerRule: x BETWEEN 100, 200 -> SearchTransformer: no-op
NOT BETWEEN -> RexSimplify: x < 100 OR x > 200 -> HivePointLookupOptimizerRule: x NOT BETWEEN 100, 200 -> SearchTransformer: no-op
vs
1.42 (1.41. actually)
BETWEEN -> RexSimplify: Sarg x [100..200] -> HivePointLookupOptimizerRule: no-op -> SearchTransformer: x BETWEEN 100, 200
NOT BETWEEN -> RexSimplify: Sarg x (-inf..100, 200..+inf) -> HivePointLookupOptimizerRule: no-op -> SearchTransformer: x < 100 OR x > 200

It could be back to the original plan if we make SearchTransformer to detect and handle NOT BETWEEN (but that would impact other tests, so tbd separately)

• 13 ) Rowcount adjustments, like: TestExplainCBOFormattedCliDriver query24, 30, 38, 54, 81, 87

<         "rowCount": 2.1331637755562098E35
---
>         "rowCount": 6.998796201537001E10

Reason: "[CALCITE-7083] RelMdDistinctRowCount aggregates implementation problems" (1.41)

• 14 ) Fix in udtf_explode.q

< OPTIMIZED SQL: SELECT `col` AS `a.mycol`, COUNT(*) AS `_c1`
< FROM (SELECT `col`
< FROM TABLE(EXPLODE(ARRAY (1, 2, 3)))
< LIMIT 3) AS `t2`
---
> OPTIMIZED SQL: SELECT `col` AS `a.mycol`, `$f1` AS `_c1`
> FROM (SELECT `col`, COUNT(*) AS `$f1`
> FROM TABLE(EXPLODE(ARRAY[1, 2, 3]))

Reason: "[CALCITE-6748] RelToSqlConverter returns the wrong result when Aggregate is on Sort" (1.39) (and also "[CALCITE-6417] Map value constructor and Array value constructor unparsed incorrectly for HiveSqlDialect" (1.38))

• 15 ) Stats change in: stats_histogram.q

<                     predicate: ((e > 0) and ((d < 3.0) or (d > 7.0))) (type: boolean)
<                     Statistics: Num rows: 9 Data size: 1040 Basic stats: COMPLETE Column stats: COMPLETE
---
>                     predicate: (((d < 3.0) or (d > 7.0)) and (e > 0)) (type: boolean)
>                     Statistics: Num rows: 8 Data size: 924 Basic stats: COMPLETE Column stats: COMPLETE
719c719
<                       Statistics: Num rows: 9 Data size: 1040 Basic stats: COMPLETE Column stats: COMPLETE
---
>                       Statistics: Num rows: 8 Data size: 924 Basic stats: COMPLETE Column stats: COMPLETE
722c722
<                         minReductionHashAggr: 0.8888889
---
>                         minReductionHashAggr: 0.875

Reason: condition in different order (see 11), which leads to numRows diff, which in turn leads to minReductionHashAggr diff; all comes from StatsRulesProcFactory.java:367-391 treats AND non-commutatively, order matters due to iteratively rounding

• 16 ) SUBSTRING change, like: jdbc_project_pushdown.q

< WHERE SUBSTRING("title", 0, 4) = 'Les'
---
> WHERE SUBSTRING("title" FROM 0 FOR 4) = 'Les'

Reason: "[CALCITE-5677] SUBSTR signature incorrect for BigQuery" (1.35)
The new emitted SQL is valid in every dialect that Hive is realistically going to push down to (Derby, MySQL, PostgreSQL, MS SQL, Oracle, BigQuery all accept comma-separated SUBSTRING/SUBSTR).
It's actually more dialect-portable than the old form: the old SUBSTRING(x FROM y FOR z) is SQL-92/99 syntax that some older or stricter dialects don't accept, while comma-form is universal. CALCITE-5677 was made specifically to fix the BigQuery breakage.

• 17 ) Adjustment in timestamp.q

< FROM (SELECT TRUE AS `$f0`
112c111
< GROUP BY TRUE) AS `t3`
---
> GROUP BY TRUE

Reason: "[CALCITE-4334] LITERAL_AGG, an aggregate function that returns a constant value" (1.35)

• 18 ) cross_prod_1.q.out
  Equivalent plan: expression "A.value BETWEEN 'val_103' AND 'val_105'" is pushed down and pre-computed before the join. The change comes from a new computation of Strong#isNull for SEARCH, introduced via
  "[CALCITE-7019] Simplify 'NULL IN (20, 10)' to 'NULL'" (1.40)

• 19 ) join46.q.out, mapjoin46.q.out, vectorized_join46.q.out, vectorized_join46_mr.q.out, smb_mapjoin_46.q.out:
  Equivalent plan: expression "key BETWEEN 100 AND 102" is pushed down and pre-computed before the join. The change comes from a new computation of Strong#isNull for SEARCH, introduced via
  "[CALCITE-7019] Simplify 'NULL IN (20, 10)' to 'NULL'" (1.40) (Same as previous point)

• 20 ) vector_aggregate_9.q.out
  RexSimplify.simplifyCast DECIMAL casts are never removed, since they perform bounds checking, via
  "[CALCITE-6685] Support checked arithmetic" (1.39)

• 21 ) vector_coalesce.q.out: equivalent plan, with some IS_NOT_NULL simplification adjustment:
  Before:

IF IsNotNull(cdouble) AND IsNotNull(log2(cint))
THEN cdouble + log2(cint)
ELSE 0.0

After:

IF IsNotNull(cdouble + log2(cint))
THEN cdouble + log2(cint)
ELSE 0.0

Due to some IS [NOT] NULL simplifications have been removed from RexSimplify for "non-safe" operations (see 3).

• 22 ) vector_interval_2.q.out: simplified predicates

str2 >= '1-2' AND str2 > '1-2' ==> str2 > '1-2'
CAST(str1) IS NOT NULL AND (dt + CAST(str1)) = '2002-03-01' ==> (dt + CAST(str1)) = '2002-03-01'
t <> '2002-04-01' AND t = '2002-03-01' ==> t = '2002-03-01'
(dt + I) > T AND T < (dt + I) ==> T < (dt + I)

Due to more aggressive simplifications:
"[CALCITE-4364] a IN (1,2) AND a = 1 should be simplified to a = 1"
"[CALCITE-5759] SEARCH(1, Sarg[IS NOT NULL]) should be simplified to TRUE"
"[CALCITE-5780] Simplify '1 > x OR 1 <= x OR x IS NULL' to TRUE"

• 23 ) join45.q.out, join47.q.out, mapjoin47.q, smb_mapjoin_47.q
  Simplified: (x >= 100 OR x <= 102) as a tautology on non-null x
  Thus, residual filter predicates: {(((_col2 + _col5) >= 100.0D) or ((_col2 + _col5) <= 102.0D))} is removed, and instead we simply have IS_NOT_NULL(_col2) and IS_NOT_NULL(_col5)
  Thanks to
  "[CALCITE-7194] Simplify comparisons between function calls and literals to SEARCH" (1.41.0)

• 24 ) query53.q.out, query63.q.out
  Equivalent plan: conjunct reordering in AND of IN predicates
  SARG/predicate canonicalisation via "[CALCITE-7194] Simplify comparisons between function calls and literals to SEARCH" (1.41.0).
  Side effect: The row-count change is not caused by the predicate reordering itself (the filter is semantically identical). It comes from a different evaluation path that Hive's statistics estimator (HiveReduceExpressionsWithStatsRule / HiveReduceExpressionsRule) takes when the conjuncts are in a different order. Hive's statistics-based conjunct selectivity evaluation works by applying conjuncts left-to-right and cascadingly, this is inherently order-dependent when the column NDV/histogram data is imprecise (see 15).

• 25 ) iceberg_bucket_map_join_1.q.out
  Simplified predicate (primary difference):

if(decimal_col is not null, (CAST(decimal_col AS STRING) = '...'), false) AND date_col is not null AND decimal_col is not null
=>
(CAST(decimal_col AS STRING) = '...') AND date_col is not null

Secondary changes derived from the simpler predicate:
All row-count estimates collapse to rows=1 (was: target_table TS=20/Filter=10, source_table TS=7/Filter=3); target_table Filter width 260→252.
The two queries with SELECT DISTINCT in the subquery (4 of the 8 explains, across bucket-map-join × vectorized configs) restructure their vertex dependency: Map 1 <- Reducer 3 becomes Reducer 3 <- Map 1, Map 2. With row estimates that small, target_table is shuffled/broadcast into Reducer 3 where the join now happens. The non-DISTINCT queries keep their original shape.
Grouping Num Buckets:7, Grouping Partition Columns annotation drops from target_table's TS on the 4 restructured plans (metadata surface only). Result rows 2022-08-16 and 2022-08-30 swap order in 4 places — same result set, different emission order (final projection now in Reducer 3).

[thomasrebele]

By chance I've stumbled upon the Bug class. It references workarounds for Calcite bugs. The Calcite upgrade fixes some of the bugs making the workarounds unnecessary. I suggest to open a follow-up ticket for removing the workarounds, to avoid making this big PR even bigger.

[rubenada]

By chance I've stumbled upon the Bug class. It references workarounds for Calcite bugs. The Calcite upgrade fixes some of the bugs making the workarounds unnecessary. I suggest to open a follow-up ticket for removing the workarounds, to avoid making this big PR even bigger.

Thanks for pointing this out. I have created a follow-up ticket HIVE-29734 for this purpose.

[zabetak]

I looked into the performance regression around partitioned materialized views (i.e., materialized_view_partitioned_2.q) and from my point of view is not blocking for merging the PR.

First of all, it appears the regression appears only in the context of MV rewrites so the impact is limited. Secondly, I assume that the performance/rewrite bug is already there even before the upgrade. If we modify the query to use ref op literal I guess the rewrite will fail to use the MV for the same reason. Finally, since we are not planning to make an immediate Hive release the performance regression will never reach end-users since it will be fixed by the time we deliver 4.3.0.

For tracking purposes, please raise a JIRA ticket for this bug/regression and mark it as a blocker for the Hive 4.3.0 release adding the appropriate links with HIVE-29641 and potentially the next HIVE ticket tracking the upgrade to calcite 1.43.0.

[rubenada]

@zabetak thanks for the feedback; your analysis is correct.
I have filed these for followup:
HIVE-29742 materialized_view_partitioned_2.q chooses the wrong partitioned MV
HIVE-29743 Upgrade Calcite to 1.43.0

[zabetak]

I did a first pass covering mostly changes on .java and pom.xml. I will to do a pass over the .q.out files in the coming week.

[zabetak]

The new plan is equivalent but it seems that the simplification didn't lead to a significant improvement. The simplification here seems to have introduced a new Filter[col0 is not null] higher up so at the end of the day the number of comparisons remains the same.

[zabetak]

Setting calcite properties at this stage is very brittle. The CalciteSystemProperty class is initialized during class loading and remains immutable afterwards. This means that if during the JVM startup we load CalciteSystemProperty.class before Hive.class this settings will never take effect.

I know that this pattern was used before and it may have been my idea to do it as such in the first place but its definitely something that we should fix. I guess a better alternative would be to put these settings in saffron.properties file and package this inside the Hive jar (e.g., under src/main/resources).

This is not a blocker but something that we definitely need to follow-up if not treated here.

[zabetak]

Logged https://issues.apache.org/jira/browse/CALCITE-7659 for having a more convenient way to do this copy.

[zabetak]

nit: We can drop the TODO comments since we already have the Bug reference in the method. By doing this I guess we can get rid of some Sonar warnings.

[zabetak]

nit: we could simplify the constructor to new HiveMaterializedViewProjectFilterRule() since we are always passing in the same param values.

[zabetak]

I am in favor of keeping the existing behavior via overriding for now. However, it makes me wonder when and why we are using the Calcite derivation logic. Hive has its own type derivation rules for division which are applied via org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPDivide#deriveResultDecimalTypeInfo. I get the feeling that this method should use the logic in GenericUDFOPDivide. This is another important follow-up I guess.

[zabetak]

Too intrusive; let's find another way to set these.

[zabetak]

First of all its unfortunate that we inherited yet another dependency from Avatica that we need to bundle in hive-exec but we don't have much of a choice right now. We could try to get rid of it in a subsequent Avatica release by repackaging the client differently.

Secondly, we may need to include the httpcore5 in the shaded jar otherwise queries may break if the dependencies are required at query runtime. Query execution happens in Tez and in a real cluster this has a different classpath than HS2. The shaded jar is part of the Tez classpath but not the HS2/ql dependencies. Tests cannot capture this kind of behavior so to test if everything works fine the easier would be to build the Hive docker image in the packaging module, run it, and test some DDL, DML, SQL, operations to see if everything works fine.

Thirdly, the statement that shade plugin bundles only ql-level dependencies is not true. You can pick which transitive dependencies are included by the appropriate include statements. The most vivid example is Avatica; it is not direct dependency of the ql module but it is included in the shaded jar.

Finally, if we end-up declaring httpcore5 explicitly here then probably we have to use runtime scope if we don't want to accidentally start using it the code.

[zabetak]

Relocation is one thing and inclusion in the shaded jar is another. I suppose they included calcite in the shaded jar because the latter is send to workers (running in different JVMs) with a different classpath. Not sure if the Druid module can still work if you remove calcite from shading.

Druid 0.17.1 is using calcite 1.21.0. I highly doubt that all the used APIs are compatible with 1.42.0. In fact, I am rather skeptical about the compatibility with previous upgrades as well (1.25.0 and 1.33.0).

Out of curiosity, I tried running TestMiniDruidCliDriver today and realized that the Druid module is broken since 2021. I send also an email to https://lists.apache.org/thread/qr0xxkvvg4wrlz4kg0bmktldw96yvpn2 explaining the current status and favoring the complete removal of the module.

All in all, the changes here do not really matter and we don't have to sweat too much about it since it's only a matter of days before getting rid of the entire module.

[zabetak]

Can we replace this code with a call to RexSimplify#simplifyComparisonWithNull?

The fact that we have to call simplification code here in order to avoid an AssertionError later on is worrisome. The choice to throw AssertionError from core calcite APIs was somewhat brutal. If we fail to handle "invalid" comparisons on some other part of the code we may end-up with irrecoverable errors at runtime. Anyways let's hope that its sufficient to update the code here and get done with it.

[sonarqubecloud]

Quality Gate passed

Issues
24 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.6% Duplication on New Code

See analysis details on SonarQube Cloud