----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35739/#review88903 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java (line 79) <https://reviews.apache.org/r/35739/#comment141523> The name should be ParquetPruneScanRule instead of PruneScanRule. exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java (line 103) <https://reviews.apache.org/r/35739/#comment141524> Same as above. exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java (line 129) <https://reviews.apache.org/r/35739/#comment141525> Agree with Jacques's comment about refactoring and sharing with PruneScanRule. Also add comments about whether these 2 rules should eventually be consolidated together. exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java (line 20) <https://reviews.apache.org/r/35739/#comment141518> This interface does not seem to be used..remove. exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java (line 39) <https://reviews.apache.org/r/35739/#comment141520> At first it wasn't clear why these functions are not part of the template NewValueFunctions; I understand it is because these are variable width types that cause the template code to be more complex. You could add some comments here to that effect. exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java (line 320) <https://reviews.apache.org/r/35739/#comment141522> Is the purpose of the 'first' flag to find the first column in the metadata that qualifies as a 'partition' column ? But if this column is not the one that we are looking for (i.e this column is not in the filter condition of the query) then we should keep looking...so in that case do we really need this flag ? exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java (line 426) <https://reviews.apache.org/r/35739/#comment141521> This return should be inside the if (stats != null) block and I suppose you could return false outside. - Aman Sinha On June 22, 2015, 10:22 p.m., Steven Phillips wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35739/ > ----------------------------------------------------------- > > (Updated June 22, 2015, 10:22 p.m.) > > > Review request for drill. > > > Bugs: DRILL-3333 > https://issues.apache.org/jira/browse/DRILL-3333 > > > Repository: drill-git > > > Description > ------- > > DRILL-3333: Parquet writer auto-partitioning and partition pruning > > Conflicts: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java > > > Diffs > ----- > > exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java > 6b6065f6b6c8469aa548acf194e0621b9f4ffea8 > exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java > 797f3cb8c83a89821ee46ce0b093f81406fa6067 > exec/java-exec/src/main/codegen/templates/NewValueFunctions.java > PRE-CREATION > exec/java-exec/src/main/codegen/templates/RecordWriter.java > c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 > exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java > f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java > 5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java > 2d16cd01b94ed8a5463c0e2fb896f019133f7f03 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java > d5d64a722ed6d9b5d97158046e6838f07c0d5381 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > d9b1354492454dcd2630c72f5dbc1c3badf958c7 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java > 920b2848d8edb62667b880e81f5aee12b459d63a > > exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java > a43a4a0f21bf11f29b6385e36db4d25003ffa98f > > exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java > cf39518b2a8b4564504a3971d1f89c268aee4b30 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java > 621f05c4d50ecf83071a5df414be88e7471f0490 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java > 31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 > > Diff: https://reviews.apache.org/r/35739/diff/ > > > Testing > ------- > > > Thanks, > > Steven Phillips > >
