Re: Review Request 72437: Reduce number of DB calls in ObjectStore::getPartitionsByExprInternal

2020-05-04 Thread Ashutosh Chauhan

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
Line 1548 (original), 1548 (patched)


assert table != null ?
At this point there is no advantage of passing null for partition keys, 
since if its null then retrieving partitions of this table won't make sense.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Line 832 (original), 830 (patched)


isView only used for this check here, which can be eliminated.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1017 (patched)


assert partitonKeys != null
If they are null, retrieving partitions won't make sense.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3473-3474 (patched)


This needs to be avoided, else it will generate additional RDBMS queries. 
It's only needed for getPartitionsViaSqlFilter() and not on other paths. And 
even there its needed only to make checks which can be avoided altogether. so, 
there is no need for isViewTable anywhere.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3832 (patched)


may remove this.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4077-4078 (patched)


This needs to be avoided, else it will generate additional RDBMS queries. 
It's only needed for one path and even there its only needed to make checks. 
isViewTable can be eliminated entirely.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4287 (patched)


LOG.debug



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 4114 (original), 4307 (patched)


LOG.debug


- Ashutosh Chauhan


On April 27, 2020, 9:15 a.m., Attila Magyar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72437/
> ---
> 
> (Updated April 27, 2020, 9:15 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan, Rajesh Balamohan, and Vineet Garg.
> 
> 
> Bugs: HIVE-23282
> https://issues.apache.org/jira/browse/HIVE-23282
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> ObjectStore::getPartitionsByExprInternal internally uses Table information 
> for getting partitionKeys, table, catalog name.
> 
>  
> 
> For this, it ends up populating entire table data from DB (including skew 
> column, parameters, sort, bucket cols etc). This makes it a lot more 
> expensive call. It would be good to check if MTable itself can be used 
> instead of Table.
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
>  4f58cd91efc 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
>  d1558876f14 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  53b7a67a429 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
>  9834883f00f 
> 
> 
> Diff: https://reviews.apache.org/r/72437/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>



Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread John Sherman


> On May 5, 2020, 1:59 a.m., John Sherman wrote:
> > ql/src/test/results/clientpositive/llap/mapjoin_hook.q.out
> > Line 41 (original), 41 (patched)
> > 
> >
> > Same as above

Not same as above, prior one failed due to exhausted memory


- John


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


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
>   ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
>   ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
>   ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out a0a193d720 
>   ql/src/test/results/clientpositive/llap/groupby_sort_8.q.out b5f581e6e6 
>   ql/src/test/results/clientpositive/llap/groupby_sort_9.q.out 33e21a3e08 
>   

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread John Sherman

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



I didn't call out every spot where order changed of results unless it made it 
impossible to actually verify if rows disappeared or changed.
I didn't call out every spot where Statistics lines disappeared like 
"Statistics: Num rows: 500 Data size: 47500 Basic stats: COMPLETE Column stats: 
COMPLETE", I assumed it was systemic afterwhile and not isolated to certain 
tests.


ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Lines 238-240 (original), 255-257 (patched)


the /src disappeared



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Line 382 (original), 407 (patched)


auto parallelism becomes true, I assume that is okay



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Line 410 (original), 415 (patched)


this changed from src to hr=11



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Lines 410-432 (original), 415-441 (patched)


maybe these changes are related to src disappearing above?



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Line 416 (original)


bucketing version gone?



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Line 436 (original)


stats disappear



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Lines 444-451 (original), 452-457 (patched)


I assume this diff is due to src disappearing above



ql/src/test/results/clientpositive/llap/louter_join_ppr.q.out
Line 507 (original), 545 (patched)


I guess src shows up down here which causes the diffs.



ql/src/test/results/clientpositive/llap/macro.q.out
Line 34 (original)


this whole file lost stats it seems



ql/src/test/results/clientpositive/llap/mapjoin1.q.out
Line 154 (original), 159 (patched)


Is it expected HashTable Sink become Reduce Output Operator?



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Line 231 (original), 239 (patched)


There are many warnings of this nature in these mapjoin tests.

Where they go from a clearly labeled MAPJOIN to Shuffle Join MERGEJOIN or 
some other algorithm.

It seems like an earlier stage is added and throws the warning and they 
still end up doing a Map Join afterwards. But my domain knowledge in these 
areas are not the greatest.



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Line 326 (original), 342 (patched)


file is about mapjoins but this warning becomes about a mergejoin, not sure 
if its important or if the warning just comes from a different part now



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Line 347 (original), 363 (patched)


Same as above



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Line 406 (original), 423 (patched)


Yeah, this seems like I was discussing above. A new stage that does a 
different join algor before feeding into the MapJoin. I expect this is expected.



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Lines 459-468 (original), 484-493 (patched)


values changed, due to no order by and a limit



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Lines 691-701 (original), 734-744 (patched)


Values changed but I assume due to having no ORDER BY and a limit above



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Lines 1301-1310 (original), 1311-1320 (patched)


I expect this is due to no order by, but most of these values are NULL 
which looks suspecious



ql/src/test/results/clientpositive/llap/mapjoin47.q.out
Lines 2084-2093 (original), 1987-1996 (patched)


I assume the values change here due to no order by



ql/src/test/results/clientpositive/llap/mapjoin_hook.q.out
Line 15 (original), 15 (patched)


Hmm HINTED_MAPJOIN and HINTED_MAPJOIN_LOCAL become 0 from 1, in a file 
called mapjoin_hook.q.out




Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Krisztian Kasa

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




ql/src/test/results/clientpositive/llap/leadlag_queries.q.out
Line 173 (original), 173 (patched)


Order of result set changed. Do we need SORT_RESULTS here?



ql/src/test/results/clientpositive/llap/load_dyn_part11.q.out
Lines 1066 (patched)


Result set mismatch. Seems that ordering changed.


- Krisztian Kasa


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
>   ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
>   ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
>   ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out a0a193d720 
>   ql/src/test/results/clientpositive/llap/groupby_sort_8.q.out b5f581e6e6 
>   

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Steve Carlin via Review Board

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




ql/src/test/results/clientpositive/llap/infer_bucket_sort_grouping_operators.q.out
Line 156 (original)


order is different here?


- Steve Carlin


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
>   ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
>   ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
>   ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out a0a193d720 
>   ql/src/test/results/clientpositive/llap/groupby_sort_8.q.out b5f581e6e6 
>   ql/src/test/results/clientpositive/llap/groupby_sort_9.q.out 33e21a3e08 
>   ql/src/test/results/clientpositive/llap/groupby_sort_skew_1_23.q.out 
> 38826ef32b 
>   ql/src/test/results/clientpositive/llap/groupby_sort_test_1.q.out 
> 405374af77 

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Vineet Garg

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




ql/src/test/results/clientpositive/llap/input_part3.q.out
Line 24 (original)


Statistics are missing



ql/src/test/results/clientpositive/llap/input_part4.q.out
Line 22 (original)


Stats missing



ql/src/test/results/clientpositive/llap/input_part6.q.out
Line 26 (original)


Stats missing



ql/src/test/results/clientpositive/llap/input_part8.q.out
Line 26 (original)


Stats missing



ql/src/test/results/clientpositive/llap/insert0.q.out
Line 126 (original), 122 (patched)


result changed


- Vineet Garg


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Jesús Camacho Rodríguez


> On May 4, 2020, 9:20 p.m., Steve Carlin wrote:
> > ql/src/test/results/clientpositive/llap/groupby_sort_skew_1_23.q.out
> > Line 49 (original), 49 (patched)
> > 
> >
> > This file is a bit overwhelming to check.  8K lines before 5K lines 
> > after.

I had one like this too... I believe such large files should be just be split 
into smaller ones in a follow-up, those files are just unmanageable.


- Jesús


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


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
>   ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
>   ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
>   ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out a0a193d720 
>   

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Jesús Camacho Rodríguez


> On May 4, 2020, 5:20 p.m., Zoltan Haindrich wrote:
> > ql/src/test/results/clientpositive/llap/join_cond_pushdown_unqual5.q.out
> > Line 100 (original), 114 (patched)
> > 
> >
> > the CASE have been pushed into the other CASE; and it seem to have 
> > started missing trivial simplifications like `('EMPTY' = 'EMPTY')`
> > 
> > Could you open a followup for this?

Can we check the logs? Can it be that CBO is failing for some reason?


- Jesús


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


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
> 95f95b0613 
>   ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
>   ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
>   ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
>   ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
>   ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
>   ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
>   ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
>   ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
>   ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
>   ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out 

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Steve Carlin via Review Board

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




ql/src/test/results/clientpositive/llap/groupby_sort_8.q.out
Lines 71 (patched)


is it ok that the "distinct" went away?



ql/src/test/results/clientpositive/llap/groupby_sort_9.q.out
Lines 133 (patched)


Interesting that this test is entitled "sort" but the rows aren't sorted.  
And I didn't see an order by.  But can this lead to a flaky test?



ql/src/test/results/clientpositive/llap/groupby_sort_skew_1_23.q.out
Line 49 (original), 49 (patched)


This file is a bit overwhelming to check.  8K lines before 5K lines after.



ql/src/test/results/clientpositive/llap/groupingset_high_columns.q.out
Lines 140 (patched)


Same concern: Will ordering changes cause flaky tests?



ql/src/test/results/clientpositive/llap/having2.q.out
Line 329 (original), 361 (patched)






ql/src/test/results/clientpositive/llap/infer_bucket_sort_merge.q.out
Line 102 (original), 102 (patched)


Buckets went from 2 to -1? I know there were bucket issues elsewhere, is 
this one ok?


- Steve Carlin


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
>   ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
>   

Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Jesús Camacho Rodríguez

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




ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out
Line 72 (original), 81 (patched)


Probably needs SORT in q file or order by in query.



ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out
Line 232 (original), 244 (patched)


SORT/orderby



ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out
Line 129 (original)


Same results, different order.



ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out
Line 3905 (original), 2765 (patched)


This is a difficult one to read, but it seems we lost the column stats for 
the table. Worth exploring.



ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out
Lines 3284 (patched)


Only basic stats. What about column stats?



ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out
Line 4687 (original)


One row is missing. Please let's give priority to explore this in follow-up 
since this is related to correctness.



ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out
Line 742 (original), 701 (patched)


This change is weird: Number of files from 1 to 0.


- Jesús Camacho Rodríguez


On May 4, 2020, 10:40 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 10:40 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
> 4b4d57f2a0 
>   ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
>   ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
> 3a92e71a75 
>   ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
>   

[jira] [Created] (HIVE-23366) Investigate why bucket_map_join*, bucketcontext_* and bucketmapjoin* tests are missing bucket map join when run under TestMiniLlapLocalCliDriver

2020-05-04 Thread Vineet Garg (Jira)
Vineet Garg created HIVE-23366:
--

 Summary: Investigate why bucket_map_join*, bucketcontext_* and 
bucketmapjoin* tests are missing bucket map join when run under 
TestMiniLlapLocalCliDriver
 Key: HIVE-23366
 URL: https://issues.apache.org/jira/browse/HIVE-23366
 Project: Hive
  Issue Type: Sub-task
  Components: Test
Reporter: Vineet Garg






--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (HIVE-23365) Put RS deduplication optimization under cost based decision

2020-05-04 Thread Jesus Camacho Rodriguez (Jira)
Jesus Camacho Rodriguez created HIVE-23365:
--

 Summary: Put RS deduplication optimization under cost based 
decision
 Key: HIVE-23365
 URL: https://issues.apache.org/jira/browse/HIVE-23365
 Project: Hive
  Issue Type: Improvement
  Components: Physical Optimizer
Reporter: Jesus Camacho Rodriguez


Currently, RS deduplication is always executed whenever it is semantically 
correct. However, it could be beneficial if t to leave both RS operators in the 
plan, e.g., if the NDV of the second RS is very low. Thus, we would like this 
decision to be cost-based. We could use a simple heuristic that would work fine 
for most of the cases without introducing regressions for existing cases, e.g., 
if NDV for partition column is less than estimated parallelism in the second 
RS, do not execute deduplication.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Zoltan Haindrich

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




ql/src/test/results/clientpositive/llap/join45.q.out
Line 1104 (original), 1191 (patched)


resultset change
could be because query contains a LIMIT and it queries the "src" tables

probably `order by src.key,a.key,b.key` will stabilize it



ql/src/test/results/clientpositive/llap/join45.q.out
Line 1257 (original)


results set change; same as prev?



ql/src/test/results/clientpositive/llap/join45.q.out
Line 1970 (original), 2029 (patched)


this test is full of "lets select some data" like queries...

as a result many resultsets are changed - in case the resultset is unstable 
(can be select 10 is not really specific) they might cause trouble...



ql/src/test/results/clientpositive/llap/join47.q.out
Lines 1189-1190 (patched)


resultset changes...

same kind of issues like in join45



ql/src/test/results/clientpositive/llap/join_1to1.q.out
Line 145 (original)


resultset changes.

this might be problematic; since the query doesn't have a LIMIT



ql/src/test/results/clientpositive/llap/join_1to1.q.out
Lines 221 (patched)


resultset changes - no limit



ql/src/test/results/clientpositive/llap/join_cond_pushdown_unqual5.q.out
Line 100 (original), 114 (patched)


the CASE have been pushed into the other CASE; and it seem to have started 
missing trivial simplifications like `('EMPTY' = 'EMPTY')`

Could you open a followup for this?


- Zoltan Haindrich


On May 4, 2020, 12:40 p.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72466/
> ---
> 
> (Updated May 4, 2020, 12:40 p.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
> Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.
> 
> 
> Bugs: HIVE-23337
> https://issues.apache.org/jira/browse/HIVE-23337
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
> different, batch 2
> 
> 
> Diffs
> -
> 
>   ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
>   ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
> e784a5e04a 
>   
> ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
>  dd2ea4a357 
>   ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
>   ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
> af37eaca1a 
>   ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
> 901d6378ff 
>   ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
> 44e8ef6952 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out 
> cdc063b370 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
> 43ab99b9f1 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
> 7831a49e95 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
> a08dd02490 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
> b61aba926d 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
> b6b4dcb339 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
> f6571b4645 
>   
> ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
> 93e081b729 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
> b4aa6d1dd0 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
> 582b78092c 
>   ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
> 21d92567d5 
>   ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
> 2138eae171 
>   ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
>   
> ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
> bd43f546dd 
>   
> ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
>  991f343394 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
> 756c179e8b 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
> d151470d6c 
>   ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 

Re: What's master key in hive metastore?

2020-05-04 Thread Alan Gates
It is used in storing delegation tokens, which HS2 and the megastore need
to act on the user's behalf when reading files, launching jobs, etc.  See
DelegationTokenStore.

Alan.

On Mon, May 4, 2020 at 2:00 AM Xi Chen  wrote:

> Dear all,
>
> I find several APIs about master key in hive metastore and
> IMetastoreClient. I searched around but didn't find any usage in hive DDL
> tasks. Any idea where this master key is designed for?
>
> Thanks in advance,
> bargitta
>
> int addMasterKey(String key) throws MetaException, TException;
>
> void updateMasterKey(Integer seqNo, String key)
> throws NoSuchObjectException, MetaException, TException;
>
> boolean removeMasterKey(Integer keySeq) throws TException;
>
> String[] getMasterKeys() throws TException;
>


Re: Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Denys Kuzmenko via Review Board


> On May 4, 2020, 3:21 p.m., Marton Bod wrote:
> > Looks good to me, just a couple of questions.

Thank you for the review!


> On May 4, 2020, 3:21 p.m., Marton Bod wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
> > Lines 2226 (patched)
> > 
> >
> > nit: the .getLockid() part is a leftover?

removed.


> On May 4, 2020, 3:21 p.m., Marton Bod wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
> > Lines 2241 (patched)
> > 
> >
> > maybe worth running the same tests with 'false' too?

added.


- Denys


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


On May 4, 2020, 4:33 p.m., Denys Kuzmenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72470/
> ---
> 
> (Updated May 4, 2020, 4:33 p.m.)
> 
> 
> Review request for hive, Marton Bod, Peter Varga, and Peter Vary.
> 
> 
> Bugs: HIVE-23349
> https://issues.apache.org/jira/browse/HIVE-23349
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 2 concurrent MERGE INSERT operations generate duplicates due to lack of 
> locking.
> MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
> doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or 
> EXCL_WRITE if hive.txn.write.xlock=false;
> 
> create table target (a int, b int) stored as orc TBLPROPERTIES 
> ('transactional'='true')");
> insert into target values (1,2), (3,4)
> create table source (a int, b int)
> execute in parallel:
> 
> insert into source values (5,6), (7,8)
> 
> PS:
> 
> Current patch doesn cover following scenario:
> 1) T1 (merge-insert) opens txns & records snapshot;
> 2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
> 3) T2 commits it's changes and unlocks T1; 
> 4) T1 locks snapshot and validates txn list, however only txns with txnId 
> lower than T1's is beeing considered, T2 changes are ignored -> duplicates 
> are generated.
> 
> 
> merge-insert/merge-insert scenario could be fixed by leveraging write-write 
> conflict check mechanism. We just need to set operation type for merge-insert 
> to update.
> However it won't solve issue with merge-insert/insert. 
> 
> We should consider moving locking before snapshot generation, current 
> snapshot re-check mechanism doesn't handle described scenarios.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
> 3ffdcec528 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  b435e79c3c 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 
> 
> 
> Diff: https://reviews.apache.org/r/72470/diff/2/
> 
> 
> Testing
> ---
> 
> Manual, added number of merge related test scenarios into TestDbTxnManager2, 
> modified explain_locks.q
> 
> 
> Thanks,
> 
> Denys Kuzmenko
> 
>



Re: Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Denys Kuzmenko via Review Board


> On May 4, 2020, 3:21 p.m., Marton Bod wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
> > Lines 2231 (patched)
> > 
> >
> > nit: this appears in a few places, might make sense to extract

i would keep it simple in a tests.


- Denys


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


On May 4, 2020, 4:33 p.m., Denys Kuzmenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72470/
> ---
> 
> (Updated May 4, 2020, 4:33 p.m.)
> 
> 
> Review request for hive, Marton Bod, Peter Varga, and Peter Vary.
> 
> 
> Bugs: HIVE-23349
> https://issues.apache.org/jira/browse/HIVE-23349
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 2 concurrent MERGE INSERT operations generate duplicates due to lack of 
> locking.
> MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
> doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or 
> EXCL_WRITE if hive.txn.write.xlock=false;
> 
> create table target (a int, b int) stored as orc TBLPROPERTIES 
> ('transactional'='true')");
> insert into target values (1,2), (3,4)
> create table source (a int, b int)
> execute in parallel:
> 
> insert into source values (5,6), (7,8)
> 
> PS:
> 
> Current patch doesn cover following scenario:
> 1) T1 (merge-insert) opens txns & records snapshot;
> 2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
> 3) T2 commits it's changes and unlocks T1; 
> 4) T1 locks snapshot and validates txn list, however only txns with txnId 
> lower than T1's is beeing considered, T2 changes are ignored -> duplicates 
> are generated.
> 
> 
> merge-insert/merge-insert scenario could be fixed by leveraging write-write 
> conflict check mechanism. We just need to set operation type for merge-insert 
> to update.
> However it won't solve issue with merge-insert/insert. 
> 
> We should consider moving locking before snapshot generation, current 
> snapshot re-check mechanism doesn't handle described scenarios.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
> 3ffdcec528 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  b435e79c3c 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 
> 
> 
> Diff: https://reviews.apache.org/r/72470/diff/2/
> 
> 
> Testing
> ---
> 
> Manual, added number of merge related test scenarios into TestDbTxnManager2, 
> modified explain_locks.q
> 
> 
> Thanks,
> 
> Denys Kuzmenko
> 
>



Re: Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Denys Kuzmenko via Review Board

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

(Updated May 4, 2020, 4:33 p.m.)


Review request for hive, Marton Bod, Peter Varga, and Peter Vary.


Changes
---

Uploaded new patch.


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


Repository: hive-git


Description
---

2 concurrent MERGE INSERT operations generate duplicates due to lack of locking.
MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or EXCL_WRITE 
if hive.txn.write.xlock=false;

create table target (a int, b int) stored as orc TBLPROPERTIES 
('transactional'='true')");
insert into target values (1,2), (3,4)
create table source (a int, b int)
execute in parallel:

insert into source values (5,6), (7,8)

PS:

Current patch doesn cover following scenario:
1) T1 (merge-insert) opens txns & records snapshot;
2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
3) T2 commits it's changes and unlocks T1; 
4) T1 locks snapshot and validates txn list, however only txns with txnId lower 
than T1's is beeing considered, T2 changes are ignored -> duplicates are 
generated.


merge-insert/merge-insert scenario could be fixed by leveraging write-write 
conflict check mechanism. We just need to set operation type for merge-insert 
to update.
However it won't solve issue with merge-insert/insert. 

We should consider moving locking before snapshot generation, current snapshot 
re-check mechanism doesn't handle described scenarios.


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
  ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
3ffdcec528 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
b435e79c3c 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
1687425bcb 
  ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 


Diff: https://reviews.apache.org/r/72470/diff/2/

Changes: https://reviews.apache.org/r/72470/diff/1-2/


Testing
---

Manual, added number of merge related test scenarios into TestDbTxnManager2, 
modified explain_locks.q


Thanks,

Denys Kuzmenko



[jira] [Created] (HIVE-23364) Push down null-safe cast-s

2020-05-04 Thread Zoltan Haindrich (Jira)
Zoltan Haindrich created HIVE-23364:
---

 Summary: Push down null-safe cast-s 
 Key: HIVE-23364
 URL: https://issues.apache.org/jira/browse/HIVE-23364
 Project: Hive
  Issue Type: Improvement
Reporter: Zoltan Haindrich


right now we may endup with conditionals like this:

{code}
  predicate: CASE WHEN (_col1 is not null) THEN (CASE WHEN 
(_col4 is not null) THEN ((CAST( _col4 AS STRING) = CAST( _col1 AS STRING))) 
ELSE (('EMPTY' = CAST( _col1 AS STRING))) END) ELSE (CASE WHEN (_col4 is not 
null) THEN ((CAST( _col4 AS STRING) = 'EMPTY')) ELSE (('EMPTY' = 'EMPTY')) END) 
END (type: boolean)
{code}

if this is after a join operator; then the same CAST might be evaluated for the 
same value...

in case the CAST is to a type which doesn't widen nullabiltity in Hive - we may 
push down the CAST



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Review Request 72469: HIVE-23325: Clean up cleanup tasks for TxnHandler/CompactionTxnHandler

2020-05-04 Thread Denys Kuzmenko via Review Board


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
> > Line 31 (original), 32 (patched)
> > 
> >
> > What does empty txn mean? Is it obsolete/abandoned/timed-out txns?
> 
> Marton Bod wrote:
> It refers to aborted/committed TXNs that do not have any entries in 
> TXN_COMPONENTS (hence the word empty).

could we name it TxnCleanerServic? EmptyTxn is a bit confusing.


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
> > Lines 84 (patched)
> > 
> >
> > Old AcidCompactionHistoryService was doing same stuff 
> > (txnHandler.purgeCompactionHistory()). Do we need than 
> > AcidEmptyTxnCleanerService and if yes can we embed it here?
> 
> Marton Bod wrote:
> Those operations that can run with the about same frequency (timedout txn 
> cleaning, writeset cleaning, compaction history pruning) have been merged and 
> consolidated into a single HouseKeeper for simplicity (along with the 
> cleaning ops Initiator used to do). However, AcidEmptyTxnCleaner needs to be 
> treated separately it has to run significantly more frequently than 
> AcidHouseKeeper due to how the new open txn mechanism works.

got it, thanks!


- Denys


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


On May 4, 2020, 12:58 p.m., Marton Bod wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72469/
> ---
> 
> (Updated May 4, 2020, 12:58 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Consolidate different metastore housekeeper threads into one, move cleaner 
> methods out of compactor initiator. 
> Create separate txn cleaner service which will need to run more frequently.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 829791e0a9 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 23512e252e 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 48bf8529fa 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 5b8c6701e1 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> eac2c6307f 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> e4ff14a140 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  842b7fe53d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  7bba8d6ee6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java
>  d35c9602a6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
>  e96a7ba289 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
>  c4a488bac0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidOpenTxnsCounterService.java
>  2ad5a89f03 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidWriteSetService.java
>  5ec513dfd4 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  8fded608d0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  e8ac71ae55 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
>  9905a14983 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidEmptyTxnCleanerService.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72469/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marton Bod
> 
>



Re: Review Request 72469: HIVE-23325: Clean up cleanup tasks for TxnHandler/CompactionTxnHandler

2020-05-04 Thread Marton Bod


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > LGTM , just few questions

Thanks for the review!


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
> > Line 153 (original)
> > 
> >
> > Cleanup functionality was moved out of the Initiator and is covered by 
> > HousekeeperThreadnow. Is it right?

Yes, that's correct.


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java
> > Line 205 (original)
> > 
> >
> > Do we have similar test for Housekeeper?

Yes. The various TxnStore operations (e.g. writeset pruning) that 
AcidHouseKeeper executes in sequence are already covered by tests in various 
places (e.g. TestTxnHandler, TestDbTxnManager2).


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
> > Line 31 (original), 32 (patched)
> > 
> >
> > What does empty txn mean? Is it obsolete/abandoned/timed-out txns?

It refers to aborted/committed TXNs that do not have any entries in 
TXN_COMPONENTS (hence the word empty).


> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
> > Lines 84 (patched)
> > 
> >
> > Old AcidCompactionHistoryService was doing same stuff 
> > (txnHandler.purgeCompactionHistory()). Do we need than 
> > AcidEmptyTxnCleanerService and if yes can we embed it here?

Those operations that can run with the about same frequency (timedout txn 
cleaning, writeset cleaning, compaction history pruning) have been merged and 
consolidated into a single HouseKeeper for simplicity (along with the cleaning 
ops Initiator used to do). However, AcidEmptyTxnCleaner needs to be treated 
separately it has to run significantly more frequently than AcidHouseKeeper due 
to how the new open txn mechanism works.


- Marton


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


On May 4, 2020, 12:58 p.m., Marton Bod wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72469/
> ---
> 
> (Updated May 4, 2020, 12:58 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Consolidate different metastore housekeeper threads into one, move cleaner 
> methods out of compactor initiator. 
> Create separate txn cleaner service which will need to run more frequently.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 829791e0a9 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 23512e252e 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 48bf8529fa 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 5b8c6701e1 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> eac2c6307f 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> e4ff14a140 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  842b7fe53d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  7bba8d6ee6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java
>  d35c9602a6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
>  e96a7ba289 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
>  c4a488bac0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidOpenTxnsCounterService.java
>  2ad5a89f03 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidWriteSetService.java
>  5ec513dfd4 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  8fded608d0 
>   
> 

Re: Review Request 72469: HIVE-23325: Clean up cleanup tasks for TxnHandler/CompactionTxnHandler

2020-05-04 Thread Denys Kuzmenko via Review Board

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



LGTM , just few questions


ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
Line 153 (original)


Cleanup functionality was moved out of the Initiator and is covered by 
HousekeeperThreadnow. Is it right?



ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java
Line 205 (original)


Do we have similar test for Housekeeper?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
Line 31 (original), 32 (patched)


What does empty txn mean? Is it obsolete/abandoned/timed-out txns?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
Lines 84 (patched)


Old AcidCompactionHistoryService was doing same stuff 
(txnHandler.purgeCompactionHistory()). Do we need than 
AcidEmptyTxnCleanerService and if yes can we embed it here?


- Denys Kuzmenko


On May 4, 2020, 12:58 p.m., Marton Bod wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72469/
> ---
> 
> (Updated May 4, 2020, 12:58 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Consolidate different metastore housekeeper threads into one, move cleaner 
> methods out of compactor initiator. 
> Create separate txn cleaner service which will need to run more frequently.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 829791e0a9 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 23512e252e 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 48bf8529fa 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 5b8c6701e1 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> eac2c6307f 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> e4ff14a140 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  842b7fe53d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  7bba8d6ee6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java
>  d35c9602a6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
>  e96a7ba289 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
>  c4a488bac0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidOpenTxnsCounterService.java
>  2ad5a89f03 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidWriteSetService.java
>  5ec513dfd4 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  8fded608d0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  e8ac71ae55 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
>  9905a14983 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidEmptyTxnCleanerService.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72469/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marton Bod
> 
>



Re: Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Marton Bod

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



Looks good to me, just a couple of questions.


ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Lines 2226 (patched)


nit: the .getLockid() part is a leftover?



ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Lines 2231 (patched)


nit: this appears in a few places, might make sense to extract



ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Lines 2241 (patched)


maybe worth running the same tests with 'false' too?


- Marton Bod


On May 4, 2020, 1:32 p.m., Denys Kuzmenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72470/
> ---
> 
> (Updated May 4, 2020, 1:32 p.m.)
> 
> 
> Review request for hive, Marton Bod, Peter Varga, and Peter Vary.
> 
> 
> Bugs: HIVE-23349
> https://issues.apache.org/jira/browse/HIVE-23349
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 2 concurrent MERGE INSERT operations generate duplicates due to lack of 
> locking.
> MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
> doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or 
> EXCL_WRITE if hive.txn.write.xlock=false;
> 
> create table target (a int, b int) stored as orc TBLPROPERTIES 
> ('transactional'='true')");
> insert into target values (1,2), (3,4)
> create table source (a int, b int)
> execute in parallel:
> 
> insert into source values (5,6), (7,8)
> 
> PS:
> 
> Current patch doesn cover following scenario:
> 1) T1 (merge-insert) opens txns & records snapshot;
> 2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
> 3) T2 commits it's changes and unlocks T1; 
> 4) T1 locks snapshot and validates txn list, however only txns with txnId 
> lower than T1's is beeing considered, T2 changes are ignored -> duplicates 
> are generated.
> 
> 
> merge-insert/merge-insert scenario could be fixed by leveraging write-write 
> conflict check mechanism. We just need to set operation type for merge-insert 
> to update.
> However it won't solve issue with merge-insert/insert. 
> 
> We should consider moving locking before snapshot generation, current 
> snapshot re-check mechanism doesn't handle described scenarios.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
> 3ffdcec528 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  b435e79c3c 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 1687425bcb 
>   ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 
> 
> 
> Diff: https://reviews.apache.org/r/72470/diff/1/
> 
> 
> Testing
> ---
> 
> Manual, added number of merge related test scenarios into TestDbTxnManager2, 
> modified explain_locks.q
> 
> 
> Thanks,
> 
> Denys Kuzmenko
> 
>



Re: Review Request 72465: HIVE-23340 TxnHandler cleanup

2020-05-04 Thread Denys Kuzmenko via Review Board

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




standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Line 107 (original), 110 (patched)


Could we add helper methods into the enum to avoid calling quoteChar 
everywhere, like TxnSatus.aborted()?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 237 (original), 238 (patched)


By adding TxnState as 2nd argument, you can simplify toTxnState()
Also could we extract this enum to util package or something, TxnHandler is 
massive enough.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 271 (patched)


Better create and use reverse lookup map, current approach not really 
maintainable.

Map LOOKUP = Maps.uniqueIndex(
Arrays.asList(TxnStatus.values()),
MyEnum::getValue
);



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 495 (patched)


Could we reuse TxnInfo instead of OpenTxn? We won't have to iterate again.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 464 (original), 551 (patched)


Could we have 2 constants for query with and without info fields instead of 
constructing them here?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 1093 (original), 1094 (patched)


1 line predicate is much more readable, at least for me



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
Lines 400 (patched)


Is is a generic txn object (i.e TxnInfo)? I don't think TxnUtils is a good 
place for domain objects.


- Denys Kuzmenko


On May 4, 2020, 1:22 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72465/
> ---
> 
> (Updated May 4, 2020, 1:22 p.m.)
> 
> 
> Review request for hive and Denys Kuzmenko.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
>   * Remove TxnStatus character constants and use the enum values
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  a1bc10955a 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  8fded608d0 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
>  4ee1a45aae 
> 
> 
> Diff: https://reviews.apache.org/r/72465/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>



[jira] [Created] (HIVE-23363) Upgrade DataNucleus dependency to 5.2

2020-05-04 Thread Zoltan Chovan (Jira)
Zoltan Chovan created HIVE-23363:


 Summary: Upgrade DataNucleus dependency to 5.2
 Key: HIVE-23363
 URL: https://issues.apache.org/jira/browse/HIVE-23363
 Project: Hive
  Issue Type: Improvement
Reporter: Zoltan Chovan






--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Denys Kuzmenko via Review Board

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

(Updated May 4, 2020, 1:32 p.m.)


Review request for hive, Marton Bod, Peter Varga, and Peter Vary.


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


Repository: hive-git


Description (updated)
---

2 concurrent MERGE INSERT operations generate duplicates due to lack of locking.
MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or EXCL_WRITE 
if hive.txn.write.xlock=false;

create table target (a int, b int) stored as orc TBLPROPERTIES 
('transactional'='true')");
insert into target values (1,2), (3,4)
create table source (a int, b int)
execute in parallel:

insert into source values (5,6), (7,8)

PS:

Current patch doesn cover following scenario:
1) T1 (merge-insert) opens txns & records snapshot;
2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
3) T2 commits it's changes and unlocks T1; 
4) T1 locks snapshot and validates txn list, however only txns with txnId lower 
than T1's is beeing considered, T2 changes are ignored -> duplicates are 
generated.


merge-insert/merge-insert scenario could be fixed by leveraging write-write 
conflict check mechanism. We just need to set operation type for merge-insert 
to update.
However it won't solve issue with merge-insert/insert. 

We should consider moving locking before snapshot generation, current snapshot 
re-check mechanism doesn't handle described scenarios.


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
  ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
3ffdcec528 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
b435e79c3c 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
1687425bcb 
  ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 


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


Testing
---

Manual, added number of merge related test scenarios into TestDbTxnManager2, 
modified explain_locks.q


Thanks,

Denys Kuzmenko



Review Request 72470: ACID: Concurrent MERGE INSERT operations produce duplicates

2020-05-04 Thread Denys Kuzmenko via Review Board

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

Review request for hive, Marton Bod, Peter Varga, and Peter Vary.


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


Repository: hive-git


Description
---

2 concurrent MERGE INSERT operations generate duplicates due to lack of locking.
MERGE INSERT is treated as regular INSERT, it acquires SHARED_READ lock that 
doesn't prevent other INSERTs. We should use EXCLUSIVE lock here or EXCL_WRITE 
if hive.txn.write.xlock=false;

create table target (a int, b int) stored as orc TBLPROPERTIES 
('transactional'='true')");
insert into target values (1,2), (3,4)
create table source (a int, b int)
execute in parallel:

insert into source values (5,6), (7,8)

PS:

Current patch doesn cover following scenario:
1) T1 (merge-insert) opens txns & records snapshot;
2) T2 (insert/merge-insert) opens txns, records snapshot & locks it;
3) T2 commits it's changes and unlocks T1; 
4) T1 locks snapshot and validates txn list, however only txns with txnId lower 
than T1's is beeing considered, T2 changes are ignored -> duplicates are 
generated.


merge-insert/merge-insert scenario could be fixed by leveraging write-write 
conflict check mechanism. We just need to set operation type for merge-insert 
to update.
However it won't solve issue with merge-insert/insert. 

We should consider moving locking before snapshot generation, current snapshot 
re-check mechanism doesn't handle describe scenarios.


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/Context.java 9f59d4cea3 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java c1f94d165b 
  ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 998c05e37d 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java 
3ffdcec528 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
b435e79c3c 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
1687425bcb 
  ql/src/test/results/clientpositive/llap/explain_locks.q.out d62f6ccafd 


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


Testing
---

Manual, added number of merge related test scenarios into TestDbTxnManager2, 
modified explain_locks.q


Thanks,

Denys Kuzmenko



Re: Review Request 72465: HIVE-23340 TxnHandler cleanup

2020-05-04 Thread Peter Varga via Review Board

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

(Updated May 4, 2020, 1:22 p.m.)


Review request for hive and Denys Kuzmenko.


Changes
---

small performance improvement


Repository: hive-git


Description
---

* Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
  * Remove TxnStatus character constants and use the enum values


Diffs (updated)
-

  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 a1bc10955a 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 8fded608d0 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
 4ee1a45aae 


Diff: https://reviews.apache.org/r/72465/diff/2/

Changes: https://reviews.apache.org/r/72465/diff/1-2/


Testing
---


Thanks,

Peter Varga



Review Request 72469: HIVE-23325: Clean up cleanup tasks for TxnHandler/CompactionTxnHandler

2020-05-04 Thread Marton Bod

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

Review request for hive, Denys Kuzmenko and Peter Vary.


Repository: hive-git


Description
---

Consolidate different metastore housekeeper threads into one, move cleaner 
methods out of compactor initiator. 
Create separate txn cleaner service which will need to run more frequently.


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 829791e0a9 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 23512e252e 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 48bf8529fa 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 5b8c6701e1 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
eac2c6307f 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
1687425bcb 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
e4ff14a140 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 842b7fe53d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 7bba8d6ee6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java
 d35c9602a6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java
 e96a7ba289 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java
 c4a488bac0 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidOpenTxnsCounterService.java
 2ad5a89f03 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidWriteSetService.java
 5ec513dfd4 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 8fded608d0 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 e8ac71ae55 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
 9905a14983 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidEmptyTxnCleanerService.java
 PRE-CREATION 


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


Testing
---


Thanks,

Marton Bod



Review Request 72466: Move q tests to TestMiniLlapLocal from TestCliDriver where the output is different, batch 2

2020-05-04 Thread Miklos Gergely

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

Review request for hive, Jesús Camacho Rodríguez, John Sherman, Zoltan 
Haindrich, Krisztian Kasa, Steve Carlin, and Vineet Garg.


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


Repository: hive-git


Description
---

Move q tests to TestMiniLlapLocal from TestCliDriver where the output is 
different, batch 2


Diffs
-

  ql/src/test/results/clientpositive/llap/groupby9.q.out 8eaa2e9d1f 
  ql/src/test/results/clientpositive/llap/groupby_complex_types.q.out 
e784a5e04a 
  
ql/src/test/results/clientpositive/llap/groupby_complex_types_multi_single_reducer.q.out
 dd2ea4a357 
  ql/src/test/results/clientpositive/llap/groupby_cube1.q.out 0ac1490e34 
  ql/src/test/results/clientpositive/llap/groupby_cube_multi_gby.q.out 
af37eaca1a 
  ql/src/test/results/clientpositive/llap/groupby_distinct_samekey.q.out 
901d6378ff 
  ql/src/test/results/clientpositive/llap/groupby_duplicate_key.q.out 
44e8ef6952 
  ql/src/test/results/clientpositive/llap/groupby_grouping_id3.q.out cdc063b370 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets1.q.out 
43ab99b9f1 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets2.q.out 
7831a49e95 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets3.q.out 
a08dd02490 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets4.q.out 
b61aba926d 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets5.q.out 
b6b4dcb339 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets6.q.out 
f6571b4645 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets_grouping.q.out 
93e081b729 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets_limit.q.out 
b4aa6d1dd0 
  ql/src/test/results/clientpositive/llap/groupby_grouping_sets_view.q.out 
582b78092c 
  ql/src/test/results/clientpositive/llap/groupby_grouping_window.q.out 
21d92567d5 
  ql/src/test/results/clientpositive/llap/groupby_join_pushdown.q.out 
2138eae171 
  ql/src/test/results/clientpositive/llap/groupby_map_ppr.q.out 952f310071 
  ql/src/test/results/clientpositive/llap/groupby_map_ppr_multi_distinct.q.out 
bd43f546dd 
  
ql/src/test/results/clientpositive/llap/groupby_multi_insert_common_distinct.q.out
 991f343394 
  ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer.q.out 
756c179e8b 
  ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer2.q.out 
d151470d6c 
  ql/src/test/results/clientpositive/llap/groupby_multi_single_reducer3.q.out 
4b4d57f2a0 
  ql/src/test/results/clientpositive/llap/groupby_multialias.q.out 1a42ff23a7 
  ql/src/test/results/clientpositive/llap/groupby_nocolumnalign.q.out 
3a92e71a75 
  ql/src/test/results/clientpositive/llap/groupby_position.q.out 17f02c9089 
  ql/src/test/results/clientpositive/llap/groupby_ppd.q.out 5731e9d5c2 
  ql/src/test/results/clientpositive/llap/groupby_ppr.q.out d7549d9536 
  ql/src/test/results/clientpositive/llap/groupby_ppr_multi_distinct.q.out 
95f95b0613 
  ql/src/test/results/clientpositive/llap/groupby_rollup1.q.out e7b61b4a33 
  ql/src/test/results/clientpositive/llap/groupby_sort_10.q.out 570d3eeeaf 
  ql/src/test/results/clientpositive/llap/groupby_sort_11.q.out 76d3c7c51a 
  ql/src/test/results/clientpositive/llap/groupby_sort_1_23.q.out 6498e2422d 
  ql/src/test/results/clientpositive/llap/groupby_sort_2.q.out a6b2403f47 
  ql/src/test/results/clientpositive/llap/groupby_sort_3.q.out e657a28396 
  ql/src/test/results/clientpositive/llap/groupby_sort_4.q.out cadc717f68 
  ql/src/test/results/clientpositive/llap/groupby_sort_5.q.out 90312062f9 
  ql/src/test/results/clientpositive/llap/groupby_sort_6.q.out 69306412a7 
  ql/src/test/results/clientpositive/llap/groupby_sort_7.q.out a0a193d720 
  ql/src/test/results/clientpositive/llap/groupby_sort_8.q.out b5f581e6e6 
  ql/src/test/results/clientpositive/llap/groupby_sort_9.q.out 33e21a3e08 
  ql/src/test/results/clientpositive/llap/groupby_sort_skew_1_23.q.out 
38826ef32b 
  ql/src/test/results/clientpositive/llap/groupby_sort_test_1.q.out 405374af77 
  ql/src/test/results/clientpositive/llap/groupingset_high_columns.q.out 
a7c47cd453 
  ql/src/test/results/clientpositive/llap/hashjoin.q.out 27194bf841 
  ql/src/test/results/clientpositive/llap/having2.q.out 74bb312940 
  ql/src/test/results/clientpositive/llap/hll.q.out 9cb85bb61c 
  ql/src/test/results/clientpositive/llap/implicit_cast1.q.out 1b2d13ce34 
  ql/src/test/results/clientpositive/llap/implicit_cast_during_insert.q.out 
255823c08b 
  ql/src/test/results/clientpositive/llap/implicit_decimal.q.out 6d7b28a730 
  ql/src/test/results/clientpositive/llap/in_typecheck_char.q.out d2fcdf48f0 
  ql/src/test/results/clientpositive/llap/in_typecheck_mixed.q.out a7f89d1b65 
  

[jira] [Created] (HIVE-23362) Repl dump returns dump location and repl id even in case of failure

2020-05-04 Thread Aasha Medhi (Jira)
Aasha Medhi created HIVE-23362:
--

 Summary: Repl dump returns dump location and repl id even in case 
of failure
 Key: HIVE-23362
 URL: https://issues.apache.org/jira/browse/HIVE-23362
 Project: Hive
  Issue Type: Task
Reporter: Aasha Medhi
Assignee: Aasha Medhi


{code:java}
03:52:13.352 [HiveServer2-Background-Pool: Thread-76] ERROR hive.log - error in 
initSerDe: java.lang.ClassNotFoundException Class 
org.apache.hive.storage.jdbc.JdbcSerDe not found
java.lang.ClassNotFoundException: Class org.apache.hive.storage.jdbc.JdbcSerDe 
not found
at 
org.apache.hadoop.conf.Configuration.getClassByName(Configuration.java:2565) 
~[data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreUtils.getDeserializer(HiveMetaStoreUtils.java:84)
 [data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.metastore.HiveMetaStoreUtils.getDeserializer(HiveMetaStoreUtils.java:77)
 [data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.ql.metadata.Table.getDeserializerFromMetaStore(Table.java:329)
 [data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.ql.metadata.Table.getDeserializer(Table.java:311) 
[data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.ql.metadata.Table.getColsInternal(Table.java:711) 
[data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at org.apache.hadoop.hive.ql.metadata.Table.getCols(Table.java:694) 
[data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
at 
org.apache.hadoop.hive.ql.metadata.Table.setStatsStateLikeNewTable(Table.java:1167)
 [data_analytics_studio-event-processor-1.4.2.7.2.0.0-127.jar:?]
{code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


What's master key in hive metastore?

2020-05-04 Thread Xi Chen
Dear all,

I find several APIs about master key in hive metastore and
IMetastoreClient. I searched around but didn't find any usage in hive DDL
tasks. Any idea where this master key is designed for?

Thanks in advance,
bargitta

int addMasterKey(String key) throws MetaException, TException;

void updateMasterKey(Integer seqNo, String key)
throws NoSuchObjectException, MetaException, TException;

boolean removeMasterKey(Integer keySeq) throws TException;

String[] getMasterKeys() throws TException;


Re: Review Request 72451: Add tests to cover database managed location related DDL and fix minor issues

2020-05-04 Thread Zoltan Haindrich

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




ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java
Line 106 (original), 106 (patched)


what's this ` 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72451/
> ---
> 
> (Updated April 29, 2020, 6:03 p.m.)
> 
> 
> Review request for hive and Zoltan Haindrich.
> 
> 
> Bugs: HIVE-23316
> https://issues.apache.org/jira/browse/HIVE-23316
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Database managed location was recently introduced, but no tests were added to 
> cover it. also the following issues were fixed:
> 
> - ALTER DATABASE ... SET MANAGEDLOCATION ... commands were not handled in a 
> separate path as it should, as in DDL each command type have their own 
> Analyzer, Desc, and Operation class
> - in case of setting the LOCATION or the MANAGEDLOCATION the location was not 
> getting qualified as in the CREATE DATABASE command
> - in case of setting the LOCATION or the MANAGEDLOCATION it was not checked 
> if this modification makes the two the same
> - some minor checkstyle issues were fixed as well
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetLocationDesc.java
>  16d28f2aa5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetLocationOperation.java
>  0c4ade3538 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationAnalyzer.java
>  a0e92eb3ef 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationDesc.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationOperation.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseAnalyzer.java
>  f87dd2a64e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java
>  f3959f0b2a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseOperation.java
>  444db0a8b8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/desc/DescDatabaseDesc.java 
> be0e5a963c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseOperation.java
>  a116a0e414 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableAnalyzer.java
>  b362837439 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java
>  bf913442f9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/drop/DropTableOperation.java 
> 72b694f668 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/drop/AlterTableDropPartitionOperation.java
>  ae2c341c19 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/materialized/alter/rebuild/AlterMaterializedViewRebuildAnalyzer.java
>  4fb53785c2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/LoadDatabase.java
>  52777f3b20 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java
>  42fa88c5fb 
>   ql/src/test/queries/clientnegative/database_location_conflict.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/database_location_conflict2.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/database_location_conflict3.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/database_location.q 8571958c29 
>   ql/src/test/results/clientnegative/database_location_conflict.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/database_location_conflict2.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/database_location_conflict3.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/database_location.q.out f0567076a7 
> 
> 
> Diff: https://reviews.apache.org/r/72451/diff/1/
> 
> 
> Testing
> ---
> 
> Tests were added to cover the new DDL elements.
> 
> 
> Thanks,
> 
> Miklos Gergely
> 
>



Re: Review Request 72431: HIVE-23206

2020-05-04 Thread Krisztian Kasa

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

(Updated May 4, 2020, 7:55 a.m.)


Review request for hive, Jesús Camacho Rodríguez, Steve Carlin, and Vineet Garg.


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


Repository: hive-git


Description
---

Project not defined correctly after reordering a join


Diffs (updated)
-

  itests/src/test/resources/testconfiguration.properties 5468728f83 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinProjectTransposeRule.java
 492c55e050 
  ql/src/test/queries/clientpositive/join_reorder5.q PRE-CREATION 
  ql/src/test/results/clientpositive/join22.q.out ad34bc4310 
  ql/src/test/results/clientpositive/llap/auto_join22.q.out a7c9f81e86 
  ql/src/test/results/clientpositive/llap/correlationoptimizer3.q.out 
f063766a1f 
  ql/src/test/results/clientpositive/llap/correlationoptimizer5.q.out 
0f7cfa4a98 
  ql/src/test/results/clientpositive/llap/filter_cond_pushdown.q.out b78ea41263 
  ql/src/test/results/clientpositive/llap/join_reorder5.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/keep_uniform.q.out 54d0b5fab6 
  ql/src/test/results/clientpositive/llap/sharedwork.q.out f8d3b4b2f5 
  ql/src/test/results/clientpositive/llap/subquery_select.q.out ed5f43f699 
  ql/src/test/results/clientpositive/perf/tez/cbo_query2.q.out 26a98ffcec 
  ql/src/test/results/clientpositive/perf/tez/cbo_query59.q.out abc5d999b5 
  ql/src/test/results/clientpositive/perf/tez/cbo_query95.q.out 218ca7d8b6 
  ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query14.q.out 
eaa1defa81 
  ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query2.q.out 
4c90da4476 
  ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query59.q.out 
8d17cc79d1 
  ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query95.q.out 
ace074316b 
  ql/src/test/results/clientpositive/perf/tez/constraints/query14.q.out 
8204245245 
  ql/src/test/results/clientpositive/perf/tez/constraints/query2.q.out 
6669e6 
  ql/src/test/results/clientpositive/perf/tez/constraints/query59.q.out 
f7c7260077 
  ql/src/test/results/clientpositive/perf/tez/constraints/query95.q.out 
39d35ec330 
  ql/src/test/results/clientpositive/perf/tez/query2.q.out 0e67e97c02 
  ql/src/test/results/clientpositive/perf/tez/query59.q.out 1a2ba964f4 
  ql/src/test/results/clientpositive/perf/tez/query95.q.out f15afbed4b 
  ql/src/test/results/clientpositive/runtime_skewjoin_mapjoin_spark.q.out 
9547e4fa7c 
  ql/src/test/results/clientpositive/smb_mapjoin_25.q.out 8fb82e1659 


Diff: https://reviews.apache.org/r/72431/diff/5/

Changes: https://reviews.apache.org/r/72431/diff/4-5/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=join_reorder5.q -pl itests/qtest 
-Pitests


Thanks,

Krisztian Kasa



Review Request 72465: HIVE-23340 TxnHandler cleanup

2020-05-04 Thread Peter Varga via Review Board

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

Review request for hive and Denys Kuzmenko.


Repository: hive-git


Description
---

* Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
  * Remove TxnStatus character constants and use the enum values


Diffs
-

  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 a1bc10955a 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 8fded608d0 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
 4ee1a45aae 


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


Testing
---


Thanks,

Peter Varga



Re: Review Request 72431: HIVE-23206

2020-05-04 Thread Krisztian Kasa


> On April 27, 2020, 6:02 p.m., Vineet Garg wrote:
> > ql/src/test/results/clientpositive/llap/keep_uniform.q.out
> > Lines 946 (patched)
> > 
> >
> > Why is there an extra join in the plan now?
> 
> Krisztian Kasa wrote:
> I run `explain cbo` on master and the branch where this patch is applied 
> with this query. Both plans has 8 HiveJoin operators. Comparing the CBO plans 
> I see that 2 of the joins were reordered:
> On master: (web_returns + (web_sales + web_sales))
> On the branch: (web_sales + web_returns) + web_sales
> 
> It also turned out that on master the CBO plan contains a HiveProject 
> with all the columns from table `web_returns`. The reason is just the same as 
> in case of the example query mentioned in HIVE-23206. This project has only 
> the necessary columns (wr_order_number only) in the plan created after 
> applying this patch.
> 
> In the physical plan there are 7 joins on master and 8 when this applied. 
> SharedWorkOptimizer merge two of them i need to investigate further...

On master two joins are merged because their parent ReduceSinks are merged in 
`sharedWorkExtendedOptimization`. 
See the plan after `SharedWorkOptimizer` before `SharedWorkExtendedOptimizer`:

Plan on master
```
TS[0]-FIL[102]-SEL[2]-RS[47]-MERGEJOIN[231]-RS[50]-MERGEJOIN[232]-RS[53]-MERGEJOIN[236]-RS[56]-MERGEJOIN[237]-RS[59]-MERGEJOIN[238]-GBY[111]-RS[112]-GBY[113]-GBY[114]-RS[115]-GBY[116]-FS[67]
TS[3]-FIL[103]-SEL[5]-RS[48]-MERGEJOIN[231]
TS[6]-FIL[104]-SEL[8]-RS[51]-MERGEJOIN[232]
TS[9]-FIL[105]-SEL[11]-RS[15]-MERGEJOIN[233]-SEL[18]-GBY[19]-RS[20]-GBY[21]-RS[54]-MERGEJOIN[236]
  
-RS[32]-MERGEJOIN[234]-SEL[35]-RS[37]-MERGEJOIN[235]-GBY[40]-RS[41]-GBY[42]-RS[57]-MERGEJOIN[237]
TS[12]-FIL[106]-SEL[14]-RS[16]-MERGEJOIN[233]
   -RS[33]-MERGEJOIN[234]
TS[23]-FIL[107]-SEL[25]-RS[36]-MERGEJOIN[235]
TS[44]-FIL[110]-SEL[46]-RS[60]-MERGEJOIN[238]

```
RS[16] and RS[33] were merged.


Plan after applying patch
```
TS[0]-FIL[101]-SEL[2]-RS[46]-MERGEJOIN[213]-RS[49]-MERGEJOIN[214]-RS[52]-MERGEJOIN[218]-RS[55]-MERGEJOIN[219]-RS[58]-MERGEJOIN[220]-GBY[110]-RS[111]-GBY[112]-GBY[113]-RS[114]-GBY[115]-FS[66]
TS[3]-FIL[102]-SEL[5]-RS[47]-MERGEJOIN[213]
TS[6]-FIL[103]-SEL[8]-RS[50]-MERGEJOIN[214]
TS[9]-FIL[104]-SEL[11]-RS[15]-MERGEJOIN[215]-SEL[18]-GBY[19]-RS[20]-GBY[21]-RS[53]-MERGEJOIN[218]
  
-RS[32]-MERGEJOIN[216]-RS[35]-MERGEJOIN[217]-SEL[38]-GBY[39]-RS[40]-GBY[41]-RS[56]-MERGEJOIN[219]
  -RS[36]-MERGEJOIN[217]
TS[12]-FIL[105]-SEL[14]-RS[16]-MERGEJOIN[215]
TS[26]-FIL[107]-SEL[28]-RS[33]-MERGEJOIN[216]
TS[43]-FIL[109]-SEL[45]-RS[59]-MERGEJOIN[220]
```

RS[15] and RS[32] was not merged because MERGEJOIN[215] and MERGEJOIN[216] has 
different keys.
RS[15] and RS[36] was not merged because `tag` is different


- Krisztian


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


On May 4, 2020, 5:02 a.m., Krisztian Kasa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72431/
> ---
> 
> (Updated May 4, 2020, 5:02 a.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez, Steve Carlin, and Vineet 
> Garg.
> 
> 
> Bugs: HIVE-23206
> https://issues.apache.org/jira/browse/HIVE-23206
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Project not defined correctly after reordering a join
> 
> 
> Diffs
> -
> 
>   itests/src/test/resources/testconfiguration.properties 5468728f83 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinProjectTransposeRule.java
>  492c55e050 
>   ql/src/test/queries/clientpositive/join_reorder5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/join22.q.out ad34bc4310 
>   ql/src/test/results/clientpositive/llap/correlationoptimizer3.q.out 
> f063766a1f 
>   ql/src/test/results/clientpositive/llap/join_reorder5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/keep_uniform.q.out 54d0b5fab6 
>   ql/src/test/results/clientpositive/llap/sharedwork.q.out f8d3b4b2f5 
>   ql/src/test/results/clientpositive/llap/subquery_select.q.out 311cee743d 
>   ql/src/test/results/clientpositive/perf/tez/cbo_query2.q.out 26a98ffcec 
>   ql/src/test/results/clientpositive/perf/tez/cbo_query59.q.out abc5d999b5 
>   ql/src/test/results/clientpositive/perf/tez/cbo_query95.q.out 218ca7d8b6 
>   ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query14.q.out 
> eaa1defa81 
>   ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query2.q.out 
> 4c90da4476 
>   

[jira] [Created] (HIVE-23361) Optimising privilege synchroniser

2020-05-04 Thread Simhadri G (Jira)
Simhadri G created HIVE-23361:
-

 Summary: Optimising privilege synchroniser
 Key: HIVE-23361
 URL: https://issues.apache.org/jira/browse/HIVE-23361
 Project: Hive
  Issue Type: Improvement
  Components: Metastore
Reporter: Simhadri G


Privilege synchronizer pulls the list of databases, tables and columns from the 
Hive Metastore. For each of these objects it fetches the privilege information 
and invokes HMS API to refresh the privilege information in HMS. This patch 
store the privilege information as bit string. This is done to reduce the size 
of the tbl_col_privs tables in metastore.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (HIVE-23360) Unable to create Connection using the Ambari default PORT (working fine if Port is taken as 10000)

2020-05-04 Thread Sukhpreet Kaur (Jira)
Sukhpreet Kaur created HIVE-23360:
-

 Summary: Unable to create Connection using the Ambari default PORT 
(working fine if Port is taken as 1)
 Key: HIVE-23360
 URL: https://issues.apache.org/jira/browse/HIVE-23360
 Project: Hive
  Issue Type: Bug
  Components: Hive, HiveServer2, JDBC
 Environment: # hive version 3.1.0.3.0.1.0-187
 * success -- hive jdbc used 3.0.0 -- 
 * success -- hive jdbc used 2.0.0 -- 
 * success -- hive jdbc used 2.1.0 -- 
 * success -- hive jdbc used 2.2.0 -- 
 * success -- hive jdbc used 2.1.1 -- 
 * connection.setUrl(
 * 
"jdbc:hive2://xen188-hdp30-centos7:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=hiveserver2"
 # hive version 1.2.1000.2.6.0.3-8
 * failed -- hive jdbc used 3.0.0 -- 
 * success -- hive jdbc used 2.0.0 -- 
 * success -- hive jdbc used 2.1.0 -- 
 * failed -- hive jdbc used 2.2.0 -- 
 * failed -- hive jdbc used 2.1.1 --
 * connection.setUrl(
 * 
"jdbc:hive2://rack167-hdp26-dev:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=hiveserver2"
 * );
Reporter: Sukhpreet Kaur
 Attachments: HiveConnection.java, MainClass.java, build.gradle

Unable to create a connection using the Ambari JDBC URL (port 2181). Working 
fine with the default port 1.

Please look into the environment details (found after doing a lot of 
hit-and-trial) and attached files to further investigate the issue.

Using a higher hive-jdbc version results in conflicts while deploying the 
project.

Our R concluded that the issue mainly lies with the zookeeper jar.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)