[
https://issues.apache.org/jira/browse/HIVE-25758?focusedWorklogId=719996&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-719996
]
ASF GitHub Bot logged work on HIVE-25758:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 03/Feb/22 10:17
Start Date: 03/Feb/22 10:17
Worklog Time Spent: 10m
Work Description: asolimando commented on a change in pull request #2966:
URL: https://github.com/apache/hive/pull/2966#discussion_r798411704
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinPushTransitivePredicatesRule.java
##########
@@ -144,27 +146,70 @@ public void onMatch(RelOptRuleCall call) {
}
// We need to filter i) those that have been pushed already as stored in
the join,
- // and ii) those that were already in the subtree rooted at child
- ImmutableList<RexNode> toPush =
HiveCalciteUtil.getPredsNotPushedAlready(predicatesToExclude,
- child, valids);
- return toPush;
+ // ii) those that were already in the subtree rooted at child,
+ // iii) predicates that are not safe for transitive inference.
+ //
+ // There is no formal definition of safety for predicate inference, only
an empirical one.
Review comment:
The change does not reinforce the dependency on the registry. If you
check the call, we are filtering candidate predicates coming from
`HiveRelMdPredicates`, which is not linked to the registry at all. The
predicates in the registry are used to add another filtering step (to remove
"duplicates"), but the two steps are totally independent, they are just
performed within the same method.
This said, I agree that keeping a global state is a problem, and should be
avoided. The current implementation is also faulty under some respects.
The motivation for storing predicates in the registry is given in
[HIVE-12478](https://issues.apache.org/jira/browse/HIVE-12478?focusedCommentId=15047639&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15047639).
Since predicates can be modified (pushed past project/setOp/joins/aggregate,
they can be simplified or merged), we can try to push them transitively as it
happens in this OOM cases.
But, as I was saying, the current implementation is faulty, since even the
registry is not enough, because most of the other rules they don't know about
it, they will alter the predicates, but they won't register them into the
registry. Making all those rules aware of the registry would increase the
coupling, hence not an option IMO.
The registry main use was to prevent firing the same rule against the same
rel multiple times, but even this use is not complete, since rels are modified
along the way, and we don't update the registry accordingly. For instance, a
join condition is updated, the new join has a fresh rel-id, the registry will
try to re-apply `HiveJoinPushTransitivePredicatesRule`, and if the predicates
were modified too, this will also escape the duplicate check based on the
`registry.getPushedPredicates(join)`, and we can have a loop).
On the other hand, I don't see how we can prevent such loops without storing
a global state. But for this global state to work, each and every rule must be
aware of it, which is totally not doable IMO, which is also the line in Calcite.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 719996)
Time Spent: 50m (was: 40m)
> OOM due to recursive application of CBO rules
> ---------------------------------------------
>
> Key: HIVE-25758
> URL: https://issues.apache.org/jira/browse/HIVE-25758
> Project: Hive
> Issue Type: Bug
> Components: CBO, Query Planning
> Affects Versions: 4.0.0
> Reporter: Alessandro Solimando
> Assignee: Alessandro Solimando
> Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
>
> Reproducing query is as follows:
> {code:java}
> create table test1 (act_nbr string);
> create table test2 (month int);
> create table test3 (mth int, con_usd double);
> EXPLAIN
> SELECT c.month,
> d.con_usd
> FROM
> (SELECT
> cast(regexp_replace(substr(add_months(from_unixtime(unix_timestamp(),
> 'yyyy-MM-dd'), -1), 1, 7), '-', '') AS int) AS month
> FROM test1
> UNION ALL
> SELECT month
> FROM test2
> WHERE month = 202110) c
> JOIN test3 d ON c.month = d.mth; {code}
>
> Different plans are generated during the first CBO steps, last being:
> {noformat}
> 2021-12-01T08:28:08,598 DEBUG [a18191bb-3a2b-4193-9abf-4e37dd1996bb main]
> parse.CalcitePlanner: Plan after decorre
> lation:
> HiveProject(month=[$0], con_usd=[$2])
> HiveJoin(condition=[=($0, $1)], joinType=[inner], algorithm=[none],
> cost=[not available])
> HiveProject(month=[$0])
> HiveUnion(all=[true])
>
> HiveProject(month=[CAST(regexp_replace(substr(add_months(FROM_UNIXTIME(UNIX_TIMESTAMP,
> _UTF-16LE'yyyy-MM-d
> d':VARCHAR(2147483647) CHARACTER SET "UTF-16LE"), -1), 1, 7),
> _UTF-16LE'-':VARCHAR(2147483647) CHARACTER SET "UTF-
> 16LE", _UTF-16LE'':VARCHAR(2147483647) CHARACTER SET "UTF-16LE")):INTEGER])
> HiveTableScan(table=[[default, test1]], table:alias=[test1])
> HiveProject(month=[$0])
> HiveFilter(condition=[=($0, CAST(202110):INTEGER)])
> HiveTableScan(table=[[default, test2]], table:alias=[test2])
> HiveTableScan(table=[[default, test3]], table:alias=[d]){noformat}
>
> Then, the HEP planner will keep expanding the filter expression with
> redundant expressions, such as the following, where the identical CAST
> expression is present multiple times:
>
> {noformat}
> rel#118:HiveFilter.HIVE.[].any(input=HepRelVertex#39,condition=IN(CAST(regexp_replace(substr(add_months(FROM_UNIXTIME(UNIX_TIMESTAMP,
> _UTF-16LE'yyyy-MM-dd':VARCHAR(2147483647) CHARACTER SET "UTF-16LE"), -1), 1,
> 7), _UTF-16LE'-':VARCHAR(2147483647) CHARACTER SET "UTF-16LE",
> _UTF-16LE'':VARCHAR(2147483647) CHARACTER SET "UTF-16LE")):INTEGER,
> CAST(regexp_replace(substr(add_months(FROM_UNIXTIME(UNIX_TIMESTAMP,
> _UTF-16LE'yyyy-MM-dd':VARCHAR(2147483647) CHARACTER SET "UTF-16LE"), -1), 1,
> 7), _UTF-16LE'-':VARCHAR(2147483647) CHARACTER SET "UTF-16LE",
> _UTF-16LE'':VARCHAR(2147483647) CHARACTER SET "UTF-16LE")):INTEGER,
> 202110)){noformat}
>
> The problem seems to come from a bad interaction of at least
> _HiveFilterProjectTransposeRule_ and
> {_}HiveJoinPushTransitivePredicatesRule{_}, possibly more.
> Most probably then UNION part can be removed and the reproducer be simplified
> even further.
>
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
