----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20399/#review42627 -----------------------------------------------------------
Patch looks good. But looks like there are few changes which may not be essential for the patch. ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java <https://reviews.apache.org/r/20399/#comment76596> Its not clear whats the difference between neededColumns & referencedColumns. If not, can we just use neededColumns? If there is any, it would be good to add a comment, why neededColumns is not sufficient here. ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java <https://reviews.apache.org/r/20399/#comment76454> Operator should not contain any compile time info, only runtime info. Compile time info belongs to Desc classes. So, move this field to TableScanDesc class. ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java <https://reviews.apache.org/r/20399/#comment76455> In line with above comment, this should then be scanOp.getConf().setReferencedColumns() ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java <https://reviews.apache.org/r/20399/#comment76598> Its not clear how this referredColumns are used. Its populated, but seems like no one is making use of it. - Ashutosh Chauhan On May 7, 2014, 4:06 a.m., Navis Ryu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20399/ > ----------------------------------------------------------- > > (Updated May 7, 2014, 4:06 a.m.) > > > Review request for hive. > > > Bugs: HIVE-6910 > https://issues.apache.org/jira/browse/HIVE-6910 > > > Repository: hive-git > > > Description > ------- > > From http://www.mail-archive.com/user@hive.apache.org/msg11324.html > > neededColumnIDs in TS is only for non-partition columns. But > ColumnAccessAnalyzer is calculating it on all columns. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 58ed550 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java > 6a4dc9b > ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java > 8c4b891 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java f285312 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java > 6bdf394 > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessAnalyzer.java > 74b595a > ql/src/java/org/apache/hadoop/hive/ql/parse/ProcessAnalyzeTable.java > c26be3c > ql/src/java/org/apache/hadoop/hive/ql/parse/PrunedPartitionList.java > d3268dd > ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java a7cec5d > ql/src/test/queries/clientpositive/column_access_stats.q fbf8bba > ql/src/test/results/clientpositive/column_access_stats.q.out 7eee4ba > > Diff: https://reviews.apache.org/r/20399/diff/ > > > Testing > ------- > > > Thanks, > > Navis Ryu > >
