> On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ReduceSinkMapJoinProc.java > > Lines 292-293 (original), 292-293 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713740#file1713740line292> > > > > Does both conditions need to be true? What if only first condition is > > true? > > Add in comments.
Revisited this condition once again, finally it is not necessary to change anything. Same condition is enforced in L166 in this method. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 66 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line66> > > > > Some ascii art will help: > > > > TS TS TS > > | | -> / \ > > Op Op Op Op That is cool, thanks! I will add a few more :) > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 83 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line83> > > > > TableScans before ... Actually this prints the whole plan. It helps a lot to figure out what this rule is doing and I thought since this is only in DEBUG mode, it should be fine. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 90 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line90> > > > > Map of dbName.TblName -> Pair(tableAlias, TSOperator) I added the comment, thanks. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 108-109 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line108> > > > > Isn't this same as tableScanOpPair.getKey() ? Actually, equals to tablePair.getKey(). But that is good catch! I have fixed it, thanks > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 114-115 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line114> > > > > 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(). Even if they are not respected, is this field populated? If it is populated, it should be good, because here we are just checking which columns the TS operators actually need. If TS operators use formats that read all columns, they would read all columns in both cases too. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 286 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line286> > > > > Use of ImmutableMap.Builder.orderEntriesByValue() here would make this > > more readable. I have been checking and it is only available since Guava 19.0 so we cannot use it. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 301 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line301> > > > > Is this self-join case? Good to expand on why we dont merge. Added additional explanation and ascii. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 308-310 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line308> > > > > 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? Added additional explanation and ascii. _findInputWorksOperators_ gathers all operators of input works to the current work, not only parents to the TS. > On May 11, 2017, 1:23 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedScanOptimizer.java > > Lines 347 (patched) > > <https://reviews.apache.org/r/59137/diff/1/?file=1713741#file1713741line347> > > > > Expand on why. Added additional explanation and ascii. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59137/#review174582 ----------------------------------------------------------- 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 > >
