----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67887/#review205976 -----------------------------------------------------------
LGTM. I have some minor comments. ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java Lines 417 (patched) <https://reviews.apache.org/r/67887/#comment288929> Thanks! ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java Lines 419 (patched) <https://reviews.apache.org/r/67887/#comment288945> Each of these functions check if semijoin reduction is enabled or not. I think it would be a bit efficient if the check happens at the beginning of this function and remove it from all the underlying functions. if (!procCtx.conf.getBoolVar(ConfVars.TEZ_DYNAMIC_SEMIJOIN_REDUCTION) || procCtx.parseContext.getRsToSemiJoinBranchInfo().size() == 0) { return; } ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java Lines 1030 (patched) <https://reviews.apache.org/r/67887/#comment288953> this code is very similar to SemiJoinRemovalIfNoStatsProc. If possible, can we refactor it to void duplication? ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java Lines 1064 (patched) <https://reviews.apache.org/r/67887/#comment288954> Is the first condition to handle cycles? ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java Lines 1089 (patched) <https://reviews.apache.org/r/67887/#comment288955> Please move this line after the instanceof check. ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Lines 309 (patched) <https://reviews.apache.org/r/67887/#comment288948> Extreme nit : Can you add a blank line before the numbered comments for better readability? - Deepak Jaiswal On July 11, 2018, 5:26 p.m., Jesús Camacho Rodríguez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67887/ > ----------------------------------------------------------- > > (Updated July 11, 2018, 5:26 p.m.) > > > Review request for hive, Ashutosh Chauhan, Deepak Jaiswal, and Gopal V. > > > Bugs: HIVE-20090 > https://issues.apache.org/jira/browse/HIVE-20090 > > > Repository: hive-git > > > Description > ------- > > HIVE-20090 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 6ea68c35000a5dadb7a01db47bbd8183bff966da > itests/src/test/resources/testconfiguration.properties > 9e012ce2f8f789bde3f95acc43052bf4446fccbc > ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java > dfd790853b2f73a465989374e78c01d282d16891 > ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java > dec2d1ef38b748a5c9b40d06af491dd168d70b72 > ql/src/test/queries/clientpositive/dynamic_semijoin_reduction_sw2.q > PRE-CREATION > > ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction_sw2.q.out > PRE-CREATION > ql/src/test/results/clientpositive/llap/explainuser_1.q.out > f87fe36e11a7c7e535678dbfaaced04f33bbb501 > ql/src/test/results/clientpositive/llap/tez_fixed_bucket_pruning.q.out > 6987a96809e3c3300e1b76ea5df3069b3c1d162f > ql/src/test/results/clientpositive/perf/tez/query1.q.out > 579940c66e25ebf5e7d0635aaedd0c0cc994f4e0 > ql/src/test/results/clientpositive/perf/tez/query16.q.out > 0b64c55b0f4ba036aeba4c49f478e9ee1409087c > ql/src/test/results/clientpositive/perf/tez/query17.q.out > 2e5e254b2ddc3507f962cbc7691db51f1abafbca > ql/src/test/results/clientpositive/perf/tez/query18.q.out > e8585275b4e51a55ce778dd154033fcdf859e617 > ql/src/test/results/clientpositive/perf/tez/query2.q.out > d24899ccf371ad42ef88cebc26cc671c097686da > ql/src/test/results/clientpositive/perf/tez/query23.q.out > 6725bec30106bc3321c2869dfc304d0a4da82cf8 > ql/src/test/results/clientpositive/perf/tez/query24.q.out > 9fcec42c3ab29b898c9c947544a2e29dd08e95e8 > ql/src/test/results/clientpositive/perf/tez/query25.q.out > a885cf344b7e29dcf1b2d93d1914e7f9a8d4b921 > ql/src/test/results/clientpositive/perf/tez/query29.q.out > 46ff49d41a01591f075b2c48ae5a692640fd6eec > ql/src/test/results/clientpositive/perf/tez/query31.q.out > c4d717d8680f6ac6f8f8b6ed01742384a84ddcf9 > ql/src/test/results/clientpositive/perf/tez/query32.q.out > 6be6f7aa6e6fc50bcedebe3f4d1b5fc00b52ee86 > ql/src/test/results/clientpositive/perf/tez/query39.q.out > 5966e243ea79b4b884950f34a5b7336e40f92889 > ql/src/test/results/clientpositive/perf/tez/query40.q.out > 2f116f12ebcba44b876508d0d0f0d827e3a8b28d > ql/src/test/results/clientpositive/perf/tez/query54.q.out > 8ab239ce260fb37d988d956fcb9e4eb98a3aeb88 > ql/src/test/results/clientpositive/perf/tez/query59.q.out > 6b2dcc38737cfc9b955cca1d5b1ac99a7901370b > ql/src/test/results/clientpositive/perf/tez/query64.q.out > a673b9f753a641e111e30a7a4427206d5f2c3da3 > ql/src/test/results/clientpositive/perf/tez/query69.q.out > a9c7ac3b21b3e0588e7df7e8c2129fc641d090f1 > ql/src/test/results/clientpositive/perf/tez/query72.q.out > 48682e340db2916800e9bc5ad61c08c0fb4a8a8b > ql/src/test/results/clientpositive/perf/tez/query77.q.out > 163805b2a3dba3e4169d487bd44e7906f66e5868 > ql/src/test/results/clientpositive/perf/tez/query78.q.out > 90b6f17e1d10ca1e3af17bc53b6df50ffa310af4 > ql/src/test/results/clientpositive/perf/tez/query80.q.out > 816b525c301fe74460e5657d0b230287d0a6729f > ql/src/test/results/clientpositive/perf/tez/query91.q.out > 5e0f00a3e7321c4233f927703701051cab641fb0 > ql/src/test/results/clientpositive/perf/tez/query92.q.out > 061fcf729d6fa7fde52de3ccd46a800379a92211 > ql/src/test/results/clientpositive/perf/tez/query94.q.out > 5d19a1634b4657e9ef9595891401e8831d9b0bd4 > ql/src/test/results/clientpositive/perf/tez/query95.q.out > 400cc1958116b2347a06b52a1460320fd0e0be43 > > ql/src/test/results/clientpositive/spark/spark_dynamic_partition_pruning_3.q.out > eafc1c4a005fa2b3bc169aa4453376f5da6841bc > > > Diff: https://reviews.apache.org/r/67887/diff/1/ > > > Testing > ------- > > > Thanks, > > Jesús Camacho Rodríguez > >
