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




ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java
Lines 722 (patched)
<https://reviews.apache.org/r/59137/#comment247743>

    Worth adding a comment for  totalSize/bucket.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/ReduceSinkMapJoinProc.java
Lines 292-293 (original), 292-293 (patched)
<https://reviews.apache.org/r/59137/#comment247794>

    Does both conditions need to be true? What if only first condition is true?
    Add in comments.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 66 (patched)
<https://reviews.apache.org/r/59137/#comment247748>

    Some ascii art will help:
    
    TS   TS             TS
    |    |   ->        /  \
    Op   Op           Op  Op



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 83 (patched)
<https://reviews.apache.org/r/59137/#comment247747>

    TableScans before ...



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 90 (patched)
<https://reviews.apache.org/r/59137/#comment247753>

    Map of dbName.TblName -> Pair(tableAlias, TSOperator)



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 108-109 (patched)
<https://reviews.apache.org/r/59137/#comment247762>

    Isn't this same as tableScanOpPair.getKey() ?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 114-115 (patched)
<https://reviews.apache.org/r/59137/#comment247767>

    getNeededColumns() are not respected by all formats, only by columnars like 
RC, Parquet, ORC etc. 
    
    Text for example will ignore it and read all columns regardless. So, we 
also need to check for formats perhaps as part of excludeTableScanOps().



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 222 (patched)
<https://reviews.apache.org/r/59137/#comment247749>

    Add comment:
    because these TS may potentially read different data for different pipeline.
    1) TODO: check partition list of diff TS and dont add if they are identical
    2) TODO: check if dynamic filters are identical and dont add.
    3) TODO: check for dynamic filters.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 286 (patched)
<https://reviews.apache.org/r/59137/#comment247754>

    Use of ImmutableMap.Builder.orderEntriesByValue()  here would make this 
more readable.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 301 (patched)
<https://reviews.apache.org/r/59137/#comment247768>

    Is this self-join case? Good to expand on why we dont merge.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 308-310 (patched)
<https://reviews.apache.org/r/59137/#comment247770>

    Add reason for this.
    Seems like we are trying to get input operators for TS, but TS is root 
operator, won't its inputs be null?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 347 (patched)
<https://reviews.apache.org/r/59137/#comment247774>

    Expand on why.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java
Lines 360 (patched)
<https://reviews.apache.org/r/59137/#comment247792>

    Add TODO this currently ignores GBY and PTF which may also buffer.



ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezWork.java
Lines 296 (patched)
<https://reviews.apache.org/r/59137/#comment247793>

    Expand on this. Why we do not need to do anything.



ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java
Lines 109 (patched)
<https://reviews.apache.org/r/59137/#comment247750>

    Comment about this represents.


- Ashutosh Chauhan


On May 10, 2017, 10:20 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59137/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 10:20 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Gopal V.
> 
> 
> Bugs: HIVE-16602
>     https://issues.apache.org/jira/browse/HIVE-16602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-16602: Implement shared scans with Tez
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99c26ce80ee1790dd2bba215404e552f665c2ee8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 
> d0fdb52fbca3d5e2ffa6615f64a86b71ccb8b323 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java 
> 85d46f32821c50c275277acf1195fb763ba2d79d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ReduceSinkMapJoinProc.java 
> 3a6baca4338e447e93125e8a8e3785653ec33801 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/GenMRSkewJoinProcessor.java
>  f78bd7cfb2fbba4a94d13fb627602e489410755c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/GenSparkSkewJoinProcessor.java
>  c9706117c92dfa2a3918b004b9e22479b54aa07f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezWork.java 
> c87de16071f9c20bc290f37d1e9ee11ad4a4d5a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java 
> f469cd29fbd529097fef6b20e97135e32fcd80b3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 
> c4fb3f300af4eed2b66ae4ee1418b2764abab8c6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/MapJoinDesc.java 
> 8da85d2436d6656a5e928dc0571a358a59cc6021 
>   ql/src/test/queries/clientpositive/perf/query88.q 
> 2be814e9ca2444dc352dca19c69e51afddccbb4b 
>   ql/src/test/results/clientpositive/llap/auto_join0.q.out 
> cba600169adafd3fb23df5e83aeae188e8288994 
>   ql/src/test/results/clientpositive/llap/auto_join30.q.out 
> a26db555cf70f08e9135c33b6f4ab0c0f6bf0ceb 
>   ql/src/test/results/clientpositive/llap/auto_sortmerge_join_9.q.out 
> b69d0bdbd9a78bfbcbc73e5456ff3969301b6880 
>   ql/src/test/results/clientpositive/llap/bucket_map_join_tez1.q.out 
> 964d05824c93e9091316f721debd88b8e96a9b0b 
>   ql/src/test/results/clientpositive/llap/correlationoptimizer2.q.out 
> b628cb15a8d705e2a5e223b169d6f5d17ece8fd9 
>   ql/src/test/results/clientpositive/llap/correlationoptimizer3.q.out 
> d3cfce851d072afe681828e832325350350f84ef 
>   ql/src/test/results/clientpositive/llap/correlationoptimizer6.q.out 
> 4fec2864a81e5e5acf904800f96df75d27f43c05 
>   ql/src/test/results/clientpositive/llap/dynamic_partition_pruning.q.out 
> 35dde96ea57cd093e82bdcc271098a276d195b86 
>   ql/src/test/results/clientpositive/llap/except_distinct.q.out 
> 4c32ebcb633c4368a0636f953386d46ed4bd93d2 
>   ql/src/test/results/clientpositive/llap/explainuser_1.q.out 
> 584c3b552091556b0226a5e0df9b5892cdda44e8 
>   ql/src/test/results/clientpositive/llap/explainuser_2.q.out 
> 2a27479276ef1399ff8d9f3ceaf4511f2e9f2144 
>   ql/src/test/results/clientpositive/llap/intersect_merge.q.out 
> 62177859725edab78d7a8ad60c46cc52adfde423 
>   ql/src/test/results/clientpositive/llap/join46.q.out 
> 56f6862a8ea522576b2616497d707c557223c732 
>   ql/src/test/results/clientpositive/llap/limit_join_transpose.q.out 
> 61b5c123cb55ba6b62732b0a5b43bb62b2b3351d 
>   ql/src/test/results/clientpositive/llap/limit_pushdown.q.out 
> dd54dd22a66351a32a5b44b3071bdd56c6d0ac41 
>   ql/src/test/results/clientpositive/llap/llap_nullscan.q.out 
> 7d01c695d022e06f8eda089783e03bfb28fd24e7 
>   ql/src/test/results/clientpositive/llap/mapjoin46.q.out 
> 73960ce26c790e8fdc6b93b2827daae609b7a930 
>   ql/src/test/results/clientpositive/llap/mrr.q.out 
> 1437d5dbafe9485d52d88380fc178020c5e32571 
>   ql/src/test/results/clientpositive/llap/multiMapJoin2.q.out 
> e9c016e5e5b250e4359c01a539d9d3bc11aee411 
>   ql/src/test/results/clientpositive/llap/offset_limit_ppd_optimizer.q.out 
> 83de1fbea1c2ec2f740d4818e06a920ca4456661 
>   ql/src/test/results/clientpositive/llap/subquery_in.q.out 
> 58e78c4c46d66a5ab79c3e5661f5d6aee9a6b9d3 
>   ql/src/test/results/clientpositive/llap/subquery_multi.q.out 
> 95c78f562fe0ea32d69fbf9832b9c25968d11ac4 
>   ql/src/test/results/clientpositive/llap/subquery_notin.q.out 
> d89361db14da7d000cab49d845db0cdef0e6190b 
>   ql/src/test/results/clientpositive/llap/subquery_null_agg.q.out 
> 7d9d77c1e50cf47febd8d8799527655f53582331 
>   ql/src/test/results/clientpositive/llap/subquery_scalar.q.out 
> b2b5458c005dc9142446c35fb5d2d899b727c85d 
>   ql/src/test/results/clientpositive/llap/subquery_select.q.out 
> 8eaec9e567bc338d3c92be146a1801d39fbe1133 
>   ql/src/test/results/clientpositive/llap/subquery_views.q.out 
> bfd56e6a3f39369ecf9acff212f5344e30f521bd 
>   ql/src/test/results/clientpositive/llap/tez_join_tests.q.out 
> 4fa585461ac4b3f5fb726cab05fbfdbb514a3840 
>   ql/src/test/results/clientpositive/llap/tez_joins_explain.q.out 
> b32e990b020450c9491bee1e3460aa13cbd9a2ca 
>   ql/src/test/results/clientpositive/llap/tez_smb_main.q.out 
> 4f9c95a2eca81e20a81b41fd51f28709b7eaf01a 
>   ql/src/test/results/clientpositive/llap/unionDistinct_1.q.out 
> a252c7491e45a5ec36b2c915004bc79de13a6bec 
>   ql/src/test/results/clientpositive/llap/union_top_level.q.out 
> 52926b6f4280973b13df382b8a444d0651f3a9fb 
>   ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets4.q.out 
> 0175c3871ebb5f44fb635330a54e405070eeb0a2 
>   ql/src/test/results/clientpositive/llap/vector_groupby_mapjoin.q.out 
> 7bfbd6f0a3da57573f5c9d935e90a8f2027a32a3 
>   ql/src/test/results/clientpositive/llap/vector_join30.q.out 
> 394393e152b601f08da175faad27583409e4c641 
>   
> ql/src/test/results/clientpositive/llap/vectorized_dynamic_partition_pruning.q.out
>  3967d110d144434a95947bf14ba4b211e0522c33 
>   ql/src/test/results/clientpositive/perf/query1.q.out 
> 07828dac168ca6eb12ecb6759e80f1899dcd7048 
>   ql/src/test/results/clientpositive/perf/query14.q.out 
> d6675bc00f6943b98524b7cdcab0ded0e468ae0a 
>   ql/src/test/results/clientpositive/perf/query16.q.out 
> 05b1871aaef9cf8a43e4fde67973af2291f74573 
>   ql/src/test/results/clientpositive/perf/query17.q.out 
> 651ba57883b7e63f682a48293cca6d212bc5038b 
>   ql/src/test/results/clientpositive/perf/query23.q.out 
> 7a3201e043f0fd0f79244328ff9e9e69aecb41f6 
>   ql/src/test/results/clientpositive/perf/query25.q.out 
> 7e15c268053abcd911566a7e3580fccdb95ef65d 
>   ql/src/test/results/clientpositive/perf/query28.q.out 
> f7c52250505dadd97f59770d7a23e460cd9b47c0 
>   ql/src/test/results/clientpositive/perf/query29.q.out 
> 675bdd3a824e1f04f1d6e9378adea8609f4eaf8b 
>   ql/src/test/results/clientpositive/perf/query30.q.out 
> 72871f4f2472985c91d1761239819839c3305fe1 
>   ql/src/test/results/clientpositive/perf/query31.q.out 
> 3ed312d3e33ac19cec7257c306fbdf9263445e26 
>   ql/src/test/results/clientpositive/perf/query32.q.out 
> 5a6514b0db1fab53070e726ad2e71509b7ace204 
>   ql/src/test/results/clientpositive/perf/query33.q.out 
> 342bd90821e2b3b018f9e79b074e0e7c62cbefc3 
>   ql/src/test/results/clientpositive/perf/query38.q.out 
> 133363fadaf6e97800ba838658928b4ecaf5768e 
>   ql/src/test/results/clientpositive/perf/query39.q.out 
> 19472c4d5ebec2f11a35eacf4ab0c79f439eef3b 
>   ql/src/test/results/clientpositive/perf/query46.q.out 
> 556e4b8f6c486620ca53d1149757591546654b3a 
>   ql/src/test/results/clientpositive/perf/query5.q.out 
> ad78d7edde952de0e4f866ae7f47903a9eae5fb7 
>   ql/src/test/results/clientpositive/perf/query51.q.out 
> 7da09bafe42d34586e2da2c23a218aac635e327c 
>   ql/src/test/results/clientpositive/perf/query56.q.out 
> 4fa28c2e60f364c1bad6fe4cb9fefaaf32ada703 
>   ql/src/test/results/clientpositive/perf/query58.q.out 
> d03a73667981ecee09aa76f8ba409a6b6cb76d78 
>   ql/src/test/results/clientpositive/perf/query6.q.out 
> aede0d70684ccad4fbf0df8a3443cfbadbf1a830 
>   ql/src/test/results/clientpositive/perf/query60.q.out 
> ad9d08ef0aa0db6c037b2a2d2903d0a41555b1c2 
>   ql/src/test/results/clientpositive/perf/query64.q.out 
> 6b42393aad25d56af92ca0354d5c98e809efc319 
>   ql/src/test/results/clientpositive/perf/query65.q.out 
> db671aa6f662828741cc585d04334b6826e7686f 
>   ql/src/test/results/clientpositive/perf/query66.q.out 
> 072bfee92b987cf37f42d7761186e840d174268e 
>   ql/src/test/results/clientpositive/perf/query68.q.out 
> a0158524282bdff376e39024b407001ab7704292 
>   ql/src/test/results/clientpositive/perf/query69.q.out 
> 87087ac6520fdb2e2e01a5c441b4afa5335b813b 
>   ql/src/test/results/clientpositive/perf/query70.q.out 
> 8e42fac9c5705f26a57cf4003246d9a5a14cf8ec 
>   ql/src/test/results/clientpositive/perf/query75.q.out 
> b1e236d32588a9c6600d350f21b23a9ef71d39dc 
>   ql/src/test/results/clientpositive/perf/query76.q.out 
> c7dbb370593232909526689ccf6bc67f7f78e3e5 
>   ql/src/test/results/clientpositive/perf/query80.q.out 
> be7ecdae25b6037d9378a403945a59b131bc7e8c 
>   ql/src/test/results/clientpositive/perf/query81.q.out 
> a09d5c99b588221f852ef3db619de3cbd4f40dc9 
>   ql/src/test/results/clientpositive/perf/query83.q.out 
> 41e8a656ebc38cad9321f39fc60c40ef315c3402 
>   ql/src/test/results/clientpositive/perf/query85.q.out 
> 168bcd2a4abeb420778abbaefa7d824079ed596e 
>   ql/src/test/results/clientpositive/perf/query87.q.out 
> c7dd1d94c67fbae538854fea51aa0234f65ed2a1 
>   ql/src/test/results/clientpositive/perf/query88.q.out 
> fcb4042ade2007f850401088979d59861ea2d8f0 
>   ql/src/test/results/clientpositive/perf/query9.q.out 
> d714d411c440bbf4dd98377070ff4ab508cb9eee 
>   ql/src/test/results/clientpositive/perf/query90.q.out 
> 5ae9fe5f98e6eed7aef33614803f889c33c494b7 
>   ql/src/test/results/clientpositive/perf/query92.q.out 
> a53c8e799bff47e9940e80b1aeef113a78511540 
>   ql/src/test/results/clientpositive/perf/query95.q.out 
> 9b0d1b29d78937d4d0370dc4c73f5f96a358286c 
>   ql/src/test/results/clientpositive/perf/query97.q.out 
> 54152e93f9a3e0c38086faf35cdef7fde4cf1a9c 
>   ql/src/test/results/clientpositive/tez/explainanalyze_2.q.out 
> ea572cd669a4f906fbfc5d75bf7a835707a585dd 
> 
> 
> Diff: https://reviews.apache.org/r/59137/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Reply via email to