You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jesús Camacho Rodríguez <jc...@hortonworks.com> on 2016/02/23 18:27:11 UTC

Review Request 43885: HIVE-13102

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43885/
-----------------------------------------------------------

Review request for hive and Ashutosh Chauhan.


Bugs: HIVE-13102
    https://issues.apache.org/jira/browse/HIVE-13102


Repository: hive-git


Description
-------

HIVE-13102


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java 58a7cff97cf8120b7afdb52af6a8f6cd07b408ea 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexUtil.java 3f6dd6a3738bdcb3c4756136965c54ea6f943f53 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinPushTransitivePredicatesRule.java 994af976f12f8b59c2379f23a33cf8fc94a2efc5 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveReduceExpressionsRule.java 8f15ec7e5f075ceee1ca71b9a389837b9a31167e 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java 36d0b458d4c33707f8e3a5b823e35edf8c5d09d1 
  ql/src/test/queries/clientpositive/constprog3.q PRE-CREATION 
  ql/src/test/queries/clientpositive/infer_join_preds.q PRE-CREATION 
  ql/src/test/results/clientpositive/constprog3.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/fold_case.q.out 90ea0afa49673910be4b28ce5f96e942b42936b0 
  ql/src/test/results/clientpositive/infer_join_preds.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/mergejoin.q.out f2c04e2221c1e7164ffb7cfe49f2fe3e0536e321 
  ql/src/test/results/clientpositive/partition_boolexpr.q.out 5272f33621a45616895f81684e4f620f6b0ec765 
  ql/src/test/results/clientpositive/ppd_udf_col.q.out 2641f5c89a1a8839e8c3e5bfa5f4b6d756ec47fb 
  ql/src/test/results/clientpositive/tez/mergejoin.q.out 14d04317d17684877566ae4aad29a862255ba02c 

Diff: https://reviews.apache.org/r/43885/diff/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 43885: HIVE-13102

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On Feb. 23, 2016, 7:17 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexUtil.java, line 199
> > <https://reviews.apache.org/r/43885/diff/1/?file=1265567#file1265567line199>
> >
> >     Can you add a comment about why we are treating inverted and negated terms in same manner. My assumption was we can fold to false only in presence of negated terms, not inverted terms.

Assume we have the expression a > 5.
Then we can derive the negated term: NOT(a <= 5).
But as the comparison is string based and thus operands order dependent, we should also add the inverted negated term: NOT(5 >= a). Observe that for creating the inverted term we invert the list of operands.

I will add a comment about this to the code to make it clear.


> On Feb. 23, 2016, 7:17 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/ppd_udf_col.q.out, line 117
> > <https://reviews.apache.org/r/43885/diff/1/?file=1265578#file1265578line117>
> >
> >     Seems like we missed opportunity to fold constant here.

This is correct. In fact, we were inferring the constant on the Hive side because we were not folding the expression in CBO. In the following you can see the plans:

CBO plan before patch:
 HiveProject(key=[$0], randum123=[rand()], h4=[CAST(_UTF-16LE'4'):VARCHAR(2147483647) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary"])
  HiveFilter(condition=[AND(=(CAST($0):DOUBLE, CAST(100):DOUBLE), <=(CAST(CAST(_UTF-16LE'4'):VARCHAR(2147483647) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary"):DOUBLE, CAST(3):DOUBLE))])
    HiveTableScan(table=[[default.src]])

CBO plan after patch:
 HiveProject(key=[$0], randum123=[rand()], h4=[CAST(_UTF-16LE'4'):VARCHAR(2147483647) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary"])
  HiveFilter(condition=[false])
    HiveTableScan(table=[[default.src]])

The reason was the code in line 150 in HiveReduceExpressionsRule: it was quiting the method even if a reduction (e.g. into the literal false) was possible. Currently we infer that the predicate is false, thus we do not need to propagate any constant anymore.


- Jesús


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43885/#review120363
-----------------------------------------------------------


On Feb. 23, 2016, 5:27 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43885/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2016, 5:27 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-13102
>     https://issues.apache.org/jira/browse/HIVE-13102
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13102
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java 58a7cff97cf8120b7afdb52af6a8f6cd07b408ea 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexUtil.java 3f6dd6a3738bdcb3c4756136965c54ea6f943f53 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinPushTransitivePredicatesRule.java 994af976f12f8b59c2379f23a33cf8fc94a2efc5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveReduceExpressionsRule.java 8f15ec7e5f075ceee1ca71b9a389837b9a31167e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java 36d0b458d4c33707f8e3a5b823e35edf8c5d09d1 
>   ql/src/test/queries/clientpositive/constprog3.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/infer_join_preds.q PRE-CREATION 
>   ql/src/test/results/clientpositive/constprog3.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/fold_case.q.out 90ea0afa49673910be4b28ce5f96e942b42936b0 
>   ql/src/test/results/clientpositive/infer_join_preds.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/mergejoin.q.out f2c04e2221c1e7164ffb7cfe49f2fe3e0536e321 
>   ql/src/test/results/clientpositive/partition_boolexpr.q.out 5272f33621a45616895f81684e4f620f6b0ec765 
>   ql/src/test/results/clientpositive/ppd_udf_col.q.out 2641f5c89a1a8839e8c3e5bfa5f4b6d756ec47fb 
>   ql/src/test/results/clientpositive/tez/mergejoin.q.out 14d04317d17684877566ae4aad29a862255ba02c 
> 
> Diff: https://reviews.apache.org/r/43885/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 43885: HIVE-13102

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43885/#review120363
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexUtil.java (line 199)
<https://reviews.apache.org/r/43885/#comment181800>

    Can you add a comment about why we are treating inverted and negated terms in same manner. My assumption was we can fold to false only in presence of negated terms, not inverted terms.



ql/src/test/results/clientpositive/ppd_udf_col.q.out (line 117)
<https://reviews.apache.org/r/43885/#comment181798>

    Seems like we missed opportunity to fold constant here.


- Ashutosh Chauhan


On Feb. 23, 2016, 5:27 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43885/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2016, 5:27 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-13102
>     https://issues.apache.org/jira/browse/HIVE-13102
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13102
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java 58a7cff97cf8120b7afdb52af6a8f6cd07b408ea 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexUtil.java 3f6dd6a3738bdcb3c4756136965c54ea6f943f53 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinPushTransitivePredicatesRule.java 994af976f12f8b59c2379f23a33cf8fc94a2efc5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveReduceExpressionsRule.java 8f15ec7e5f075ceee1ca71b9a389837b9a31167e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java 36d0b458d4c33707f8e3a5b823e35edf8c5d09d1 
>   ql/src/test/queries/clientpositive/constprog3.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/infer_join_preds.q PRE-CREATION 
>   ql/src/test/results/clientpositive/constprog3.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/fold_case.q.out 90ea0afa49673910be4b28ce5f96e942b42936b0 
>   ql/src/test/results/clientpositive/infer_join_preds.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/mergejoin.q.out f2c04e2221c1e7164ffb7cfe49f2fe3e0536e321 
>   ql/src/test/results/clientpositive/partition_boolexpr.q.out 5272f33621a45616895f81684e4f620f6b0ec765 
>   ql/src/test/results/clientpositive/ppd_udf_col.q.out 2641f5c89a1a8839e8c3e5bfa5f4b6d756ec47fb 
>   ql/src/test/results/clientpositive/tez/mergejoin.q.out 14d04317d17684877566ae4aad29a862255ba02c 
> 
> Diff: https://reviews.apache.org/r/43885/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>