> On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java > > Lines 223 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092501#file2092501line223> > > > > Should it be key.contains(columns) ?
You are correct, good eye. > On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java > > Line 285 (original), 311 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092501#file2092501line311> > > > > Can we either return a Pair, or make it void and that it sets both > > _keys_ and _nonNullablekeys_? Currently it is a bit weird that one of them > > is set via return of the method, and the second one from the method. Made it void, i thought it that was more simpler. > On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java > > Line 363 (original), 414 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092503#file2092503line420> > > > > return... Not sure what do you mean by that? > On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/EstimateUniqueKeys.java > > Lines 53 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092504#file2092504line53> > > > > This should be a new metadata provider or a modification of the > > existing _RelMdUniqueKeys_ but introducing a new boolean parameter > > _acceptEstimatedResults_. However, this would need changes in Calcite side. > > Please, leave a TODO comment though. Left a TODO comment > On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/EstimateUniqueKeys.java > > Lines 288 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092504#file2092504line288> > > > > We may have Volcano node too, we need to include it. How do I include volcano nodes? Can you point me to an example? Tried searching for it but couldn't figure out. > On Oct. 1, 2018, 8:07 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdRowCount.java > > Line 320 (original), 320 (patched) > > <https://reviews.apache.org/r/68868/diff/1/?file=2092506#file2092506line320> > > > > We do not use metadata providers anymore for unique keys estimation, > > which means some features will be disabled, e.g., caching of metadata > > results. I am not sure whether this may cause an increase in compilation > > time for several queries, specially those containing many joins, but it > > would be worth leaving a comment and maybe monitoring it in future. Good point, left a note - Vineet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68868/#review209136 ----------------------------------------------------------- On Oct. 3, 2018, 12:44 a.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68868/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2018, 12:44 a.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Bugs: HIVE-17043 > https://issues.apache.org/jira/browse/HIVE-17043 > > > Repository: hive-git > > > Description > ------- > > This patch implements/test the following optimizations > * Removal of group by on primary keys > * Reduction of group by keys on primary keys > * is NOT NULL filter removal if NOT NULL constraint is defined > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties fdd8ecc77c > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveDefaultRelMetadataProvider.java > 635d27e723 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java > 42e60de6a8 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveAggregateJoinTransposeRule.java > f43ef01293 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java > 5857f730a8 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/EstimateUniqueKeys.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdColumnUniqueness.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdRowCount.java > 1ca1937ed9 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdUniqueKeys.java > 3bf62c535c > ql/src/test/queries/clientpositive/constraints_optimization.q PRE-CREATION > ql/src/test/results/clientpositive/llap/constraints_optimization.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/68868/diff/2/ > > > Testing > ------- > > > Thanks, > > Vineet Garg > >
