Re: Review Request 68310: HIVE-17040

2018-09-16 Thread Vineet Garg 

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


Ship it!




Ship It!

- Vineet Garg


On Sept. 14, 2018, 9:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68310/
> ---
> 
> (Updated Sept. 14, 2018, 9:51 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-17040
> https://issues.apache.org/jira/browse/HIVE-17040
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-17040
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 0bfee2e43e8bf35234ddabb173d71de4e00b11a2 
>   itests/src/test/resources/testconfiguration.properties 
> 3a5aec7d6b8ab624f5f4526cf81e48678eca601f 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptUtil.java 
> 268284a6da22755f801be87651f9d4a6d228b24d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectJoinTransposeRule.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectMergeRule.java
>  07518df9ec1cea1c331846bbe636cf1a039e762f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> 8a1a2520c83bf645ad8b402bef20d9990e434779 
>   ql/src/test/queries/clientpositive/join_constraints_optimization.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
> ee4844277e5dd1d9ea3911ad2e33a9c3f2481344 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
> 4aadd5fb0a4bc74c5bc4088b3d4e62c81bf9cf8b 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
> dc20b68ba9a9e0bbcb9c414b10b00f3228fe63fe 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
> 0e4fdf49ac04935f6f14c2ceaa0969008b34f926 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
> 4f05f76330cb74be50187e3b175d4675f5ea8763 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
> 59ed5757569a8dde70fe04eb9ec5e8c91b5931bf 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
> PRE-CREATION 
>   
> ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
> 5a2e74c8a005ae8422c00a998fa3c07183749176 
>   ql/src/test/results/clientpositive/ambiguitycheck.q.out 
> 80c9582fec9754fe56400064ab3f88e3e9ea2da7 
>   
> ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out
>  7813aac29465b5193789464fcd32771741a98071 
>   ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
> 806262d72e687bbdd09b47380eed77c14764c2a5 
>   ql/src/test/results/clientpositive/list_bucket_dml_2.q.out 
> bd8e215c2207c48ce2e446fcc10333ec0fb4648c 
>   ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 
> 520d48e3d9fa67301852efc9ed1c92494fd528a0 
>   ql/src/test/results/clientpositive/list_bucket_dml_9.q.out 
> fbd4fde1bd8d2f7379585afe07c018139b7d64e8 
>   ql/src/test/results/clientpositive/list_bucket_query_multiskew_1.q.out 
> e324cab738c22e31089f437d6e7d8a65160dc5b9 
>   ql/src/test/results/clientpositive/list_bucket_query_multiskew_2.q.out 
> ec1e54060cb7d380bda4d965fff540d002bd456a 
>   ql/src/test/results/clientpositive/list_bucket_query_multiskew_3.q.out 
> 889f23c6da7e3c7b955b83d44afd1bd048468b49 
>   ql/src/test/results/clientpositive/list_bucket_query_oneskew_1.q.out 
> dcff8a50370b4ebb07300d4a7d9c34aa84f4ae17 
>   ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out 
> 268051e2acbbabd02206c5a21690fb563a3dcd2b 
>   ql/src/test/results/clientpositive/llap/acid_bucket_pruning.q.out 
> 3951b71227d93469347264362492e64554efee32 
>   ql/src/test/results/clientpositive/llap/bucketpruning1.q.out 
> 55442ad04668a193a2d40b6a3088149f6072b28c 
>   ql/src/test/results/clientpositive/llap/current_date_timestamp.q.out 
> 6831fb2573788033393544b835f1a56d69fb1712 
>   ql/src/test/results/clientpositive/llap/join_constraints_optimization.q.out 
> PRE-CREATION 
>   
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out
>  71adebb2acad1545c48d45835cd5876434b141b5 
>   
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out
>  ce1c281bea0bd2a96e67057ee990fa5b45850905 
>   
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_multi_db.q.out
>  98f74379f6275aed90273929c65144e4bbd51a97 
>   ql/src/test/results/clientpositive/llap/materialized_view_rewrite_1.q.out 
> 4d8fa52aa94a77faa9d1d1872afcb016e5a3c438 
>

Re: Review Request 68310: HIVE-17040

2018-09-14 Thread Jesús Camacho Rodríguez 

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

(Updated Sept. 14, 2018, 9:51 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-17040


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
0bfee2e43e8bf35234ddabb173d71de4e00b11a2 
  itests/src/test/resources/testconfiguration.properties 
3a5aec7d6b8ab624f5f4526cf81e48678eca601f 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptUtil.java 
268284a6da22755f801be87651f9d4a6d228b24d 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectJoinTransposeRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectMergeRule.java
 07518df9ec1cea1c331846bbe636cf1a039e762f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
8a1a2520c83bf645ad8b402bef20d9990e434779 
  ql/src/test/queries/clientpositive/join_constraints_optimization.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
ee4844277e5dd1d9ea3911ad2e33a9c3f2481344 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
4aadd5fb0a4bc74c5bc4088b3d4e62c81bf9cf8b 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
dc20b68ba9a9e0bbcb9c414b10b00f3228fe63fe 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
0e4fdf49ac04935f6f14c2ceaa0969008b34f926 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
4f05f76330cb74be50187e3b175d4675f5ea8763 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
59ed5757569a8dde70fe04eb9ec5e8c91b5931bf 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
5a2e74c8a005ae8422c00a998fa3c07183749176 
  ql/src/test/results/clientpositive/ambiguitycheck.q.out 
80c9582fec9754fe56400064ab3f88e3e9ea2da7 
  
ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out
 7813aac29465b5193789464fcd32771741a98071 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
806262d72e687bbdd09b47380eed77c14764c2a5 
  ql/src/test/results/clientpositive/list_bucket_dml_2.q.out 
bd8e215c2207c48ce2e446fcc10333ec0fb4648c 
  ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 
520d48e3d9fa67301852efc9ed1c92494fd528a0 
  ql/src/test/results/clientpositive/list_bucket_dml_9.q.out 
fbd4fde1bd8d2f7379585afe07c018139b7d64e8 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_1.q.out 
e324cab738c22e31089f437d6e7d8a65160dc5b9 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_2.q.out 
ec1e54060cb7d380bda4d965fff540d002bd456a 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_3.q.out 
889f23c6da7e3c7b955b83d44afd1bd048468b49 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_1.q.out 
dcff8a50370b4ebb07300d4a7d9c34aa84f4ae17 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out 
268051e2acbbabd02206c5a21690fb563a3dcd2b 
  ql/src/test/results/clientpositive/llap/acid_bucket_pruning.q.out 
3951b71227d93469347264362492e64554efee32 
  ql/src/test/results/clientpositive/llap/bucketpruning1.q.out 
55442ad04668a193a2d40b6a3088149f6072b28c 
  ql/src/test/results/clientpositive/llap/current_date_timestamp.q.out 
6831fb2573788033393544b835f1a56d69fb1712 
  ql/src/test/results/clientpositive/llap/join_constraints_optimization.q.out 
PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out 
71adebb2acad1545c48d45835cd5876434b141b5 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out
 ce1c281bea0bd2a96e67057ee990fa5b45850905 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_multi_db.q.out
 98f74379f6275aed90273929c65144e4bbd51a97 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_1.q.out 
4d8fa52aa94a77faa9d1d1872afcb016e5a3c438 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_2.q.out 
8e54abe61d48ebd3439ee215e88c2186601a4cd3 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_3.q.out 
d7536e408798d38970a30226f7248d69a10b0903 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_4.q.out 
3fd4c59ee68c3b0df56d070df900fbea31e3f6aa 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_5.q.out 
9992409f6aa36f6be18f6b07a6de380b2be18a9a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_6.q.out

Re: Review Request 68310: HIVE-17040

2018-09-13 Thread Jesús Camacho Rodríguez 

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

(Updated Sept. 13, 2018, 8:54 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-17040


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
aa58d7445ce18c470641558231b635d325454d65 
  itests/src/test/resources/testconfiguration.properties 
3a5aec7d6b8ab624f5f4526cf81e48678eca601f 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptUtil.java 
268284a6da22755f801be87651f9d4a6d228b24d 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectJoinTransposeRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectMergeRule.java
 07518df9ec1cea1c331846bbe636cf1a039e762f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
39f27b10eb0db29f28add4e325101b192ad708d7 
  ql/src/test/queries/clientpositive/join_constraints_optimization.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
ee4844277e5dd1d9ea3911ad2e33a9c3f2481344 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
4aadd5fb0a4bc74c5bc4088b3d4e62c81bf9cf8b 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
dc20b68ba9a9e0bbcb9c414b10b00f3228fe63fe 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
0e4fdf49ac04935f6f14c2ceaa0969008b34f926 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
4f05f76330cb74be50187e3b175d4675f5ea8763 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
59ed5757569a8dde70fe04eb9ec5e8c91b5931bf 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
5a2e74c8a005ae8422c00a998fa3c07183749176 
  ql/src/test/results/clientpositive/ambiguitycheck.q.out 
80c9582fec9754fe56400064ab3f88e3e9ea2da7 
  
ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out
 7813aac29465b5193789464fcd32771741a98071 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
806262d72e687bbdd09b47380eed77c14764c2a5 
  ql/src/test/results/clientpositive/list_bucket_dml_2.q.out 
bd8e215c2207c48ce2e446fcc10333ec0fb4648c 
  ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 
520d48e3d9fa67301852efc9ed1c92494fd528a0 
  ql/src/test/results/clientpositive/list_bucket_dml_9.q.out 
fbd4fde1bd8d2f7379585afe07c018139b7d64e8 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_1.q.out 
e324cab738c22e31089f437d6e7d8a65160dc5b9 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_2.q.out 
ec1e54060cb7d380bda4d965fff540d002bd456a 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_3.q.out 
889f23c6da7e3c7b955b83d44afd1bd048468b49 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_1.q.out 
dcff8a50370b4ebb07300d4a7d9c34aa84f4ae17 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out 
268051e2acbbabd02206c5a21690fb563a3dcd2b 
  ql/src/test/results/clientpositive/llap/acid_bucket_pruning.q.out 
3951b71227d93469347264362492e64554efee32 
  ql/src/test/results/clientpositive/llap/bucketpruning1.q.out 
55442ad04668a193a2d40b6a3088149f6072b28c 
  ql/src/test/results/clientpositive/llap/current_date_timestamp.q.out 
6831fb2573788033393544b835f1a56d69fb1712 
  ql/src/test/results/clientpositive/llap/join_constraints_optimization.q.out 
PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out 
71adebb2acad1545c48d45835cd5876434b141b5 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out
 ce1c281bea0bd2a96e67057ee990fa5b45850905 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_multi_db.q.out
 98f74379f6275aed90273929c65144e4bbd51a97 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_1.q.out 
4d8fa52aa94a77faa9d1d1872afcb016e5a3c438 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_2.q.out 
8e54abe61d48ebd3439ee215e88c2186601a4cd3 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_3.q.out 
d7536e408798d38970a30226f7248d69a10b0903 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_4.q.out 
3fd4c59ee68c3b0df56d070df900fbea31e3f6aa 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_5.q.out 
9992409f6aa36f6be18f6b07a6de380b2be18a9a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_6.q.out

Re: Review Request 68310: HIVE-17040

2018-09-13 Thread Jesús Camacho Rodríguez 


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
> > Lines 2219 (patched)
> > 
> >
> > Should we control all constraint related optimization with one flag and 
> > therefore rename this one to e.g. hive.optimize.constraints.rules or 
> > something?
> > Or do you think it is better for each rule to have seperate conf?

I had not given it much thought. We could have different configuration 
properties, since disabling a rule would only be for debugging purposes and 
probably better to do it selectively. I changed the name to 
hive.optimize.constraints.join , so they all use same prefix 
hive.optimize.constraints .


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 115 (patched)
> > 
> >
> > Just a minor comment about readability. leftFK, rightFK name gets a bit 
> > confusing may be comment to describe what it represents e.g. they 
> > represents corresponding left, right input which is potential FK

Added comment and changed variable names.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 117 (patched)
> > 
> >
> > Not sure what this logic is trying to do. Can you explain? Both of the 
> > inputs are being referenced above so how can we still proceed?

I added a bit of explanation below the condition, I think it should be clearer 
now.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 203 (patched)
> > 
> >
> > I didn't understand this part. Irrespective of what input side is being 
> > referenced we assume leftInput to be FK. Lets say rightFK is true i.e. 
> > right side input is being referenced then wouldn't assuing leftInput as FK 
> > side cause issues because we will end up removing right side (PK side) when 
> > there are references to it

This is a left outer join, either we can 1) transform it into inner join if 
operator on top of it references columns from both sides, or 2) remove it if it 
only references columns from the left side. However, the case that you 
describe, i.e., left outer join with operator on top only referencing columns 
from right input, was not covered indeed! I have changed the logic slightly to 
take that into account.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 215 (patched)
> > 
> >
> > This whole step 2 can be encapsulated in a utility method. I can see 
> > this being quite useful for other constraint optimizations

Done, moved to HiveRelOptUtil.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 253 (patched)
> > 
> >
> > Order by should be safe here as well? (assuming it is represented by 
> > seperate rel node)

Correct, I expect it to be less common, but it may happen. I have changed the 
method accordingly.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 261 (patched)
> > 
> >
> > Did't understand this step. Irrespective of the mode or fk-pk 
> > relationship OUTER join is being transformed into INNER?

We transform into INNER to extract the conditions correctly and be able to do 
the corresponding verifications (we cannot pull conditions from below if it is 
an outer join).
If the join is finally transformed, we use the join that we have already 
created. Otherwise, we will not use it at all.


> On Sept. 12, 2018, 9:40 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
> > Lines 329 (patched)
> > 
> >
> > isn't calling mq.getTableReferences(rightInput) straightforward and 
> > cheaper rather than calling it for join and leftInput and taking the 
> > difference?

The names of the tables are unique, which is important to know the lineage of a 
predicate in point 5.
For instance : A JOIN (B JOIN A).
If we extract from right input, we would get B#0, A#0 for

Re: Review Request 68310: HIVE-17040

2018-09-12 Thread Vineet Garg 

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2219 (patched)


Should we control all constraint related optimization with one flag and 
therefore rename this one to e.g. hive.optimize.constraints.rules or something?
Or do you think it is better for each rule to have seperate conf?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 115 (patched)


Just a minor comment about readability. leftFK, rightFK name gets a bit 
confusing may be comment to describe what it represents e.g. they represents 
corresponding left, right input which is potential FK



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 117 (patched)


Not sure what this logic is trying to do. Can you explain? Both of the 
inputs are being referenced above so how can we still proceed?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 203 (patched)


I didn't understand this part. Irrespective of what input side is being 
referenced we assume leftInput to be FK. Lets say rightFK is true i.e. right 
side input is being referenced then wouldn't assuing leftInput as FK side cause 
issues because we will end up removing right side (PK side) when there are 
references to it



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 207 (patched)


same as above



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 215 (patched)


This whole step 2 can be encapsulated in a utility method. I can see this 
being quite useful for other constraint optimizations



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 253 (patched)


Order by should be safe here as well? (assuming it is represented by 
seperate rel node)



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 261 (patched)


Did't understand this step. Irrespective of the mode or fk-pk relationship 
OUTER join is being transformed into INNER?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 329 (patched)


isn't calling mq.getTableReferences(rightInput) straightforward and cheaper 
rather than calling it for join and leftInput and taking the difference?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
Lines 373 (patched)


Why remove when size is 1? Shouldn't it removed when size is 0?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 1661 (patched)


A comment would be nice to comment what optimizations hive currently does 
for REFERENTIAL_CONSTRAINTS,



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 2059 (patched)


Why is HiveProjectJoinTransposeRule required only for JoinConstraintRule? 
Sounds like it could be beneficial in general.



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 2877 (patched)


Rather than adding 'REFERENTIAL_CONSTRAINTS' profilesCBO separately, may be 
add it once for both DRUID and JDBC type of tables (i.e. in parent IF condition)
Edit: It looks like it is being added for all kind of tables, so many be 
just add it once (at the end) instead of adding it for each type of table.



ql/src/test/results/clientpositive/druid/druidmini_mv.q.out
Line 169 (original), 169 (patched)


Before it was virtual column vc now instead if is column c? What caused 
this change and is it correct?



ql/src/test/results/clientpositive/list_bucket_dml_2.q.out
Line 364 (original), 364 (patched)


Curious what caused this change? I don't see anything relevant.


- Vineet Garg


On Sept. 5, 2018, 9:11 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated

Re: Review Request 68310: HIVE-17040

2018-09-05 Thread Jesús Camacho Rodríguez 

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

(Updated Sept. 5, 2018, 9:11 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-17040


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
40ea3ac0c5cd0943a4e9dbe2b0e8b952070a8a67 
  itests/src/test/resources/testconfiguration.properties 
a3a70ecd498bbfc6146cfbfbeb1f265032f440ef 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectJoinTransposeRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectMergeRule.java
 07518df9ec1cea1c331846bbe636cf1a039e762f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
df40a2878d328b76f7de74e2c2db0a029ba4610f 
  ql/src/test/queries/clientpositive/join_constraints_optimization.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
ee4844277e5dd1d9ea3911ad2e33a9c3f2481344 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
4aadd5fb0a4bc74c5bc4088b3d4e62c81bf9cf8b 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
dc20b68ba9a9e0bbcb9c414b10b00f3228fe63fe 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
0e4fdf49ac04935f6f14c2ceaa0969008b34f926 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
4f05f76330cb74be50187e3b175d4675f5ea8763 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
59ed5757569a8dde70fe04eb9ec5e8c91b5931bf 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
5a2e74c8a005ae8422c00a998fa3c07183749176 
  ql/src/test/results/clientpositive/ambiguitycheck.q.out 
80c9582fec9754fe56400064ab3f88e3e9ea2da7 
  
ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out
 7813aac29465b5193789464fcd32771741a98071 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
806262d72e687bbdd09b47380eed77c14764c2a5 
  ql/src/test/results/clientpositive/list_bucket_dml_2.q.out 
bd8e215c2207c48ce2e446fcc10333ec0fb4648c 
  ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 
520d48e3d9fa67301852efc9ed1c92494fd528a0 
  ql/src/test/results/clientpositive/list_bucket_dml_9.q.out 
fbd4fde1bd8d2f7379585afe07c018139b7d64e8 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_1.q.out 
e324cab738c22e31089f437d6e7d8a65160dc5b9 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_2.q.out 
ec1e54060cb7d380bda4d965fff540d002bd456a 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_3.q.out 
889f23c6da7e3c7b955b83d44afd1bd048468b49 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_1.q.out 
dcff8a50370b4ebb07300d4a7d9c34aa84f4ae17 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out 
268051e2acbbabd02206c5a21690fb563a3dcd2b 
  ql/src/test/results/clientpositive/llap/acid_bucket_pruning.q.out 
3951b71227d93469347264362492e64554efee32 
  ql/src/test/results/clientpositive/llap/bucketpruning1.q.out 
260ba1cbddee7f0946f0cdec1070359ab2a1d2aa 
  ql/src/test/results/clientpositive/llap/current_date_timestamp.q.out 
6831fb2573788033393544b835f1a56d69fb1712 
  ql/src/test/results/clientpositive/llap/join_constraints_optimization.q.out 
PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out 
71adebb2acad1545c48d45835cd5876434b141b5 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out
 ce1c281bea0bd2a96e67057ee990fa5b45850905 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_multi_db.q.out
 98f74379f6275aed90273929c65144e4bbd51a97 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_1.q.out 
4d8fa52aa94a77faa9d1d1872afcb016e5a3c438 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_2.q.out 
8e54abe61d48ebd3439ee215e88c2186601a4cd3 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_3.q.out 
d7536e408798d38970a30226f7248d69a10b0903 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_4.q.out 
3fd4c59ee68c3b0df56d070df900fbea31e3f6aa 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_5.q.out 
9992409f6aa36f6be18f6b07a6de380b2be18a9a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_6.q.out 
544c395c0125608b3954c7cc9d51ee088d2bdb51 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out

Review Request 68310: HIVE-17040

2018-08-12 Thread Jesús Camacho Rodríguez 

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

Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-17040


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
d406f51443def027f3cbdfcc7711616316a5f1fc 
  itests/src/test/resources/testconfiguration.properties 
27d297432201675bdda97f8c6711e4c84df19d57 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectJoinTransposeRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectMergeRule.java
 07518df9ec1cea1c331846bbe636cf1a039e762f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
361f150193a155d45eb64266f88eb88f0a881ad3 
  ql/src/test/queries/clientpositive/join_constraints_optimization.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
ee4844277e5dd1d9ea3911ad2e33a9c3f2481344 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
4aadd5fb0a4bc74c5bc4088b3d4e62c81bf9cf8b 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
dc20b68ba9a9e0bbcb9c414b10b00f3228fe63fe 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
0e4fdf49ac04935f6f14c2ceaa0969008b34f926 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
4f05f76330cb74be50187e3b175d4675f5ea8763 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
59ed5757569a8dde70fe04eb9ec5e8c91b5931bf 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
5a2e74c8a005ae8422c00a998fa3c07183749176 
  ql/src/test/results/clientpositive/ambiguitycheck.q.out 
80c9582fec9754fe56400064ab3f88e3e9ea2da7 
  
ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out
 7813aac29465b5193789464fcd32771741a98071 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
806262d72e687bbdd09b47380eed77c14764c2a5 
  ql/src/test/results/clientpositive/list_bucket_dml_2.q.out 
bd8e215c2207c48ce2e446fcc10333ec0fb4648c 
  ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 
520d48e3d9fa67301852efc9ed1c92494fd528a0 
  ql/src/test/results/clientpositive/list_bucket_dml_9.q.out 
fbd4fde1bd8d2f7379585afe07c018139b7d64e8 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_1.q.out 
e324cab738c22e31089f437d6e7d8a65160dc5b9 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_2.q.out 
ec1e54060cb7d380bda4d965fff540d002bd456a 
  ql/src/test/results/clientpositive/list_bucket_query_multiskew_3.q.out 
889f23c6da7e3c7b955b83d44afd1bd048468b49 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_1.q.out 
dcff8a50370b4ebb07300d4a7d9c34aa84f4ae17 
  ql/src/test/results/clientpositive/list_bucket_query_oneskew_2.q.out 
3251dc4ef369d0d5d7ea0c8acaed44dc4e4f3be5 
  ql/src/test/results/clientpositive/llap/acid_bucket_pruning.q.out 
3951b71227d93469347264362492e64554efee32 
  ql/src/test/results/clientpositive/llap/bucketpruning1.q.out 
260ba1cbddee7f0946f0cdec1070359ab2a1d2aa 
  ql/src/test/results/clientpositive/llap/current_date_timestamp.q.out 
6831fb2573788033393544b835f1a56d69fb1712 
  ql/src/test/results/clientpositive/llap/join_constraints_optimization.q.out 
PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out 
71adebb2acad1545c48d45835cd5876434b141b5 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out
 ce1c281bea0bd2a96e67057ee990fa5b45850905 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_multi_db.q.out
 98f74379f6275aed90273929c65144e4bbd51a97 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_1.q.out 
4d8fa52aa94a77faa9d1d1872afcb016e5a3c438 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_3.q.out 
d7536e408798d38970a30226f7248d69a10b0903 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_4.q.out 
3fd4c59ee68c3b0df56d070df900fbea31e3f6aa 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_5.q.out 
9992409f6aa36f6be18f6b07a6de380b2be18a9a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_6.q.out 
544c395c0125608b3954c7cc9d51ee088d2bdb51 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out 
1e44104ac5d4cb42247b44057c7b649d7d6c1e32 
  
ql/src/test/results/clientpositive/llap/materialized_view_rewrite_no_join_opt.q.out
 PRE-CREATION