----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69663/#review211725 -----------------------------------------------------------
Can we add some tests for the new feature? ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Line 182 (original) <https://reviews.apache.org/r/69663/#comment297292> Can you bring this back and delete 'if (sourceKeys.size() > 0) {' below? This is just a style change and indenting so many lines will just make more difficult following code provenance. ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Lines 284 (patched) <https://reviews.apache.org/r/69663/#comment297291> We should add a call to extended version here as we did above for equality predicates. The only required change seems to be in _addParentReduceSink_ called from _createDerivatives_, which would receive the comparison operator from here. All the rest should already work as expected. I believe this could be addressed in this JIRA since it is not a lot of code. However, if it is not addressed, please create follow-up and leave a TODO. ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Lines 318 (patched) <https://reviews.apache.org/r/69663/#comment297296> return colExprMap.get(rsColName) ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Lines 328 (patched) <https://reviews.apache.org/r/69663/#comment297293> Can we move this method as _invertFunction_ utility method to _FunctionRegistry.java_? In addition, instead of relying on the function text, I believe it would be more robust to have the UDF as the input. In particular, we can use _funcDesc.getGenericUDF();_ when calling this method, then rely in e.g. _udf instanceof GenericUDFOPEqualOrGreaterThan_ for the checks. ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java Lines 330 (patched) <https://reviews.apache.org/r/69663/#comment297295> Should we still return null if function text is not recognized? - Jesús Camacho Rodríguez On Jan. 3, 2019, 8:39 p.m., Deepak Jaiswal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69663/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2019, 8:39 p.m.) > > > Review request for hive, Ashutosh Chauhan, Gopal V, Jesús Camacho Rodríguez, > and Jason Dere. > > > Bugs: HIVE-16976 > https://issues.apache.org/jira/browse/HIVE-16976 > > > Repository: hive-git > > > Description > ------- > > DPP: SyntheticJoinPredicate transitivity for < > and BETWEEN > > The patch supports predicates on non-equi joins and provides an interface for > storage handler to decide if it can use this optimization. > Work to integrate this with DPP and semijoin will be done in separate JIRA. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java > 2ebb149354 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java > a1401aac72 > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java f8c7e18eb1 > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java > 676dfc9421 > ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java > e97e44796f > ql/src/test/results/clientpositive/llap/cross_prod_1.q.out ac1f4eabd8 > ql/src/test/results/clientpositive/llap/groupby_groupingset_bug.q.out > de74af6dff > ql/src/test/results/clientpositive/llap/semijoin.q.out 63a270e57d > ql/src/test/results/clientpositive/llap/subquery_in.q.out 07cc4dbabc > ql/src/test/results/clientpositive/llap/subquery_notin.q.out 29d8bbfb48 > ql/src/test/results/clientpositive/llap/subquery_scalar.q.out e830835445 > ql/src/test/results/clientpositive/llap/subquery_select.q.out d3cc980ca1 > > > Diff: https://reviews.apache.org/r/69663/diff/1/ > > > Testing > ------- > > > Thanks, > > Deepak Jaiswal > >
