> On May 2, 2017, 6:35 p.m., Vineet Garg wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > > Line 338 (original), 338 (patched) > > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line338> > > > > Can you add comments to explain why are we doing this i.e. why are we > > propagating hints?
Will do it thanks. > On May 2, 2017, 6:35 p.m., Vineet Garg wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > > Lines 368 (patched) > > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line368> > > > > If I understand it correctly before generating calcite plan (i.e. > > calling getOptimizedAST) QB has hints but after generating optimized AST QB > > is reset and lose hints? That is why you are propagating hints? Yes. > On May 2, 2017, 6:35 p.m., Vineet Garg wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > > Line 408 (original), 413 (patched) > > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line413> > > > > What happens in this case? Why is it necessary to log warning here? I think just paranoia. It shouldn't happen, however, if it does, we log it. > On May 2, 2017, 6:35 p.m., Vineet Garg wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java > > Line 444 (original), 444 (patched) > > <https://reviews.apache.org/r/58914/diff/1/?file=1705387#file1705387line444> > > > > Is this because we can have hint now? If so can you rather check for > > hint and continue instead of continuing for all nodes Makes sense. Will do. > On May 2, 2017, 6:35 p.m., Vineet Garg wrote: > > ql/src/test/queries/clientpositive/semijoin_hint.q > > Line 38 (original), 38 (patched) > > <https://reviews.apache.org/r/58914/diff/1/?file=1705394#file1705394line38> > > > > Why is returnpath on? > > This feature is not yet fully developed/supported and is off by > > default. > > You might want to try the same queries with this feature off. We have to make sure we dont break it. There are tests below with the feature turned off. - Deepak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58914/#review173558 ----------------------------------------------------------- On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58914/ > ----------------------------------------------------------- > > (Updated May 2, 2017, 5:47 a.m.) > > > Review request for hive, Jason Dere and Sergey Shelukhin. > > > Bugs: HIVE-16550 > https://issues.apache.org/jira/browse/HIVE-16550 > > > Repository: hive-git > > > Description > ------- > > Semijoin Hints should be able to skip the optimization if needed. > In addition to that, the patch fixes several issues with hints in general > such as, > - It now works with subqueries and unions. > - Uses a global data structure instead of per QB. > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java > e1a69526bc > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java > d375d1b58d > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 > ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 > ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 654f3b1772 > ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java > 3143554ec6 > ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d > ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java > f45daa8828 > ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 > ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec > > > Diff: https://reviews.apache.org/r/58914/diff/1/ > > > Testing > ------- > > > Thanks, > > Deepak Jaiswal > >
