subject:"Review Request 66285\\\\\\\: HIVE\\\\\\\-18770"

Re: Review Request 66285: HIVE-18770

2018-03-26 Thread Jesús Camacho Rodríguez


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

(Updated March 26, 2018, 11:16 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-18770


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
8d9b5a3194708ffacabfdb69d6af7d6193dcf156 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
5ad4406ceff5d83bf74264c33947f207ff2c1a61 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
 3f73fd7fcc2d6c52a2015bdd947c1708723058d6 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveConfPlannerContext.java
 b0f1a8dfafa46f2cb06ca05c673ba37c736d 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelBuilder.java 
efd8a35699ef2c4bb9c363925b8adc1e2ca3cbd3 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java
 88aedb6381a293c0dd0f7d4e767df6726a86f40f 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveTableScan.java
 94a3bac1a7df35c825247e51946ee6ef1b0b6342 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
 df9c1802c8983279500d3a06c1c526ce20af6146 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
 4dc48f4710196acb68a9df5331244827b212aefe 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
612deb8327d85966751834257ab686cfa74f9feb 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q PRE-CREATION 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
97f6d844806cf33ea4403b33665142c612da6e84 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 
4da3d0930fd30cc3ab74155efb4d82a910ea6944 
  
ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out
 d7ee468b49af904da93a74c86f0898c310970cab 
  ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 
PRE-CREATION 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 3e1fea9d4fe707c59ee99781bd4c5aacdbd9d381 


Diff: https://reviews.apache.org/r/66285/diff/3/

Changes: https://reviews.apache.org/r/66285/diff/2-3/


Testing
---


Thanks,

Jesús Camacho Rodríguez

Re: Review Request 66285: HIVE-18770

2018-03-26 Thread Jesús Camacho Rodríguez

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java
> > Lines 126-128 (patched)
> > 
> >
> > I don't follow this. No where in logic cost becomes zero (or lower) for 
> > heuristic. 
> > Further, method should break out of recursion as soon as there is a MV, 
> > instead of recursing further. Since in hueristic strategy as soon as we 
> > find a MV we will use that plan

The idea here is that if there is a possible rewriting, we should choose it 
over the original plan. But if there are multiple plans rewritings (e.g., 
multiple MVs, multiple ways of rewriting with same MV, partial rewritings with 
union, etc.), we should use the one with the lower cost among them.
Hence, for the heuristic, 1) we assign a tiny cost to the TS that reads the MV 
and to all operators that are on top of the MV, and 2) we multiply by a certain 
factor the cost of the operators that are not directly on top of a TS with a 
materialized view (e.g., in a partial rewriting, the branch of the union that 
contains execution on the original sources, or in a bushy join tree the join 
operators that do not read any MV). This will help us select plans that replace 
as many joins as possible, and plans that contain full rewritings better than 
partial rewritings. Does it make sense?
In any case, I will add additional comments to the class.

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
> > Lines 97 (patched)
> > 
> >
> > Doesn't look this rule is used anywhere.

This is used in CalcitePlanner (L1910).

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
> > Lines 106 (patched)
> > 
> >
> > Same rule as previous?

The rule definition is here, but the rule instantiation is above. From 
CalcitePlanner, we only reference the final static instance, but we still need 
this rule. The rule basically overrides the _getFloorSqlFunction_ and 
_getRollup_ methods.

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
> > Lines 123 (patched)
> > 
> >
> > Unused rule?

We instantiate it in L102 in this same class. The final static instance is then 
referenced in CalcitePlanner.

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
> > Lines 108 (patched)
> > 
> >
> > Unused method. If you intend to use it, 
> > better name: addNotNullProject()

This methods are not used because they are called via reflection. In any case, 
I have reverted the changes here because they were not needed (they were coming 
from previous version of the patch that was not changing the nullability of the 
input columns).

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
> > Lines 124 (patched)
> > 
> >
> > addNotNullProjects()

Same as above.

> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Line 1494 (original), 1489 (patched)
> > 
> >
> > Do we store optimized plan or unoptimized plan when loading MV registry 
> > for defined MVs? If its optimized plan invoking rewrite rule at the end of 
> > optimization will make it easier for rewriting rule, else this should be 
> > invoked without any optimization for same reason.

We store the optimized plan. However, for matching purposes, the plan of the MV 
does not really matter because the rule can extract structural information from 
the top of the plan (filters present, columns present, joins, aggregation 
functions, etc).
However, the shape of the plan for the (sub)query that we are trying to match 
matters, as prejoin optimization stage may help us to infer some new 
predicates, remove some unused columns, etc.

- Jesús

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

On March 26, 2018, 6:38 p.m., Jesús

Re: Review Request 66285: HIVE-18770

2018-03-26 Thread Ashutosh Chauhan


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




ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java
Lines 126-128 (patched)


I don't follow this. No where in logic cost becomes zero (or lower) for 
heuristic. 
Further, method should break out of recursion as soon as there is a MV, 
instead of recursing further. Since in hueristic strategy as soon as we find a 
MV we will use that plan



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
Lines 97 (patched)


Doesn't look this rule is used anywhere.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
Lines 106 (patched)


Same rule as previous?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
Lines 123 (patched)


Unused rule?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
Lines 108 (patched)


Unused method. If you intend to use it, 
better name: addNotNullProject()



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
Lines 124 (patched)


addNotNullProjects()



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Line 1494 (original), 1489 (patched)


Do we store optimized plan or unoptimized plan when loading MV registry for 
defined MVs? If its optimized plan invoking rewrite rule at the end of 
optimization will make it easier for rewriting rule, else this should be 
invoked without any optimization for same reason.


- Ashutosh Chauhan


On March 26, 2018, 6:38 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66285/
> ---
> 
> (Updated March 26, 2018, 6:38 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18770
> https://issues.apache.org/jira/browse/HIVE-18770
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-18770
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 8d9b5a3194708ffacabfdb69d6af7d6193dcf156 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> 5ad4406ceff5d83bf74264c33947f207ff2c1a61 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
>  3f73fd7fcc2d6c52a2015bdd947c1708723058d6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveConfPlannerContext.java
>  b0f1a8dfafa46f2cb06ca05c673ba37c736d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelBuilder.java 
> efd8a35699ef2c4bb9c363925b8adc1e2ca3cbd3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java
>  88aedb6381a293c0dd0f7d4e767df6726a86f40f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveTableScan.java
>  94a3bac1a7df35c825247e51946ee6ef1b0b6342 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
>  df9c1802c8983279500d3a06c1c526ce20af6146 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
>  4dc48f4710196acb68a9df5331244827b212aefe 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 612deb8327d85966751834257ab686cfa74f9feb 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
> 97f6d844806cf33ea4403b33665142c612da6e84 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 
>

Review Request 66285: HIVE-18770

2018-03-26 Thread Jesús Camacho Rodríguez


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

Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-18770


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
8d9b5a3194708ffacabfdb69d6af7d6193dcf156 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
5ad4406ceff5d83bf74264c33947f207ff2c1a61 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
 3f73fd7fcc2d6c52a2015bdd947c1708723058d6 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveConfPlannerContext.java
 b0f1a8dfafa46f2cb06ca05c673ba37c736d 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelBuilder.java 
efd8a35699ef2c4bb9c363925b8adc1e2ca3cbd3 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java
 88aedb6381a293c0dd0f7d4e767df6726a86f40f 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveTableScan.java
 94a3bac1a7df35c825247e51946ee6ef1b0b6342 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java
 df9c1802c8983279500d3a06c1c526ce20af6146 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java
 4dc48f4710196acb68a9df5331244827b212aefe 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
612deb8327d85966751834257ab686cfa74f9feb 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q PRE-CREATION 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
97f6d844806cf33ea4403b33665142c612da6e84 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 
4da3d0930fd30cc3ab74155efb4d82a910ea6944 
  
ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out
 d7ee468b49af904da93a74c86f0898c310970cab 
  ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 
PRE-CREATION 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 3e1fea9d4fe707c59ee99781bd4c5aacdbd9d381 


Diff: https://reviews.apache.org/r/66285/diff/1/


Testing
---


Thanks,

Jesús Camacho Rodríguez

Re: Review Request 66285: HIVE-18770

Re: Review Request 66285: HIVE-18770

Re: Review Request 66285: HIVE-18770

Review Request 66285: HIVE-18770

4 matches

Site Navigation

Mail list logo

Footer information