You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Steven Phillips <sp...@maprtech.com> on 2015/06/22 21:06:39 UTC
Review Request 35739: Patch for DRILL-3333
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/
-----------------------------------------------------------
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
Re: Review Request 35739: Patch for DRILL-3333
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88916
-----------------------------------------------------------
For unit testing, you could adapt the tests in TestPartitionFilter and replace the dir0, dir1 references with the partition column references. Many of these tests do plan checking by testing inclusion or exclusion of the Filter node.
- 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
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review89113
-----------------------------------------------------------
Ship it!
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java (line 41)
<https://reviews.apache.org/r/35739/#comment141713>
wip ?
- Aman Sinha
On June 24, 2015, 12:39 a.m., Steven Phillips wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35739/
> -----------------------------------------------------------
>
> (Updated June 24, 2015, 12:39 a.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
>
>
> 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/PruneScanRule.java 2544d34bd4e531e48df66c8efba3cf6b6a6e6142
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 1e63748fe459760348501ad1fbe29553bee7264c
> 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
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 6d03d8176d3686740fd8fbcc158e664b3a3eeed9
>
> Diff: https://reviews.apache.org/r/35739/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Steven Phillips
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/
-----------------------------------------------------------
(Updated June 24, 2015, 12:39 a.m.)
Review request for drill.
Bugs: DRILL-3333
https://issues.apache.org/jira/browse/DRILL-3333
Repository: drill-git
Description (updated)
-------
DRILL-3333: Parquet writer auto-partitioning and partition pruning
Diffs (updated)
-----
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/PruneScanRule.java 2544d34bd4e531e48df66c8efba3cf6b6a6e6142
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 1e63748fe459760348501ad1fbe29553bee7264c
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
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 6d03d8176d3686740fd8fbcc158e664b3a3eeed9
Diff: https://reviews.apache.org/r/35739/diff/
Testing
-------
Thanks,
Steven Phillips
Re: Review Request 35739: Patch for DRILL-3333
Posted by Steven Phillips <sp...@maprtech.com>.
> On June 22, 2015, 10:25 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java, line 278
> > <https://reviews.apache.org/r/35739/diff/1/?file=989875#file989875line278>
> >
> > Is creating the metadata converter repeatedly expensive?
It's not expensive, but I will go ahead and reuse it anyway, as it looks cleaner.
- Steven
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88842
-----------------------------------------------------------
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
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88842
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java (line 28)
<https://reviews.apache.org/r/35739/#comment141431>
please add javadocs
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java (line 129)
<https://reviews.apache.org/r/35739/#comment141433>
These seems like a lot of duplication from the general partition pruning rule. Can you refactor shared code into a common place?
exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java (line 20)
<https://reviews.apache.org/r/35739/#comment141434>
javadoc
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java (line 278)
<https://reviews.apache.org/r/35739/#comment141452>
Is creating the metadata converter repeatedly expensive?
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java (line 319)
<https://reviews.apache.org/r/35739/#comment141436>
short comment.
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java (line 386)
<https://reviews.apache.org/r/35739/#comment141437>
What about other types, such as timestamp?
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java (line 220)
<https://reviews.apache.org/r/35739/#comment141438>
shouldn't swallow exception.
- Jacques Nadeau
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
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88864
-----------------------------------------------------------
exec/java-exec/src/main/codegen/templates/NewValueFunctions.java (line 44)
<https://reviews.apache.org/r/35739/#comment141451>
What will happen if the partition column is Decimal28, Decimal38, or Interval type? Will parquet writer check if the column data type is supported or not?
exec/java-exec/src/main/codegen/templates/NewValueFunctions.java (line 55)
<https://reviews.apache.org/r/35739/#comment141450>
Are we going to create multiple files if the same values are across multiple record batch boundary?
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java (line 156)
<https://reviews.apache.org/r/35739/#comment141469>
The compare function uses "NULL_IF_NULL" null handing policy. In case the partitioning column is null-able, should we use NullableBitVector.class here?
- Jinfeng Ni
On June 22, 2015, 3: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, 3: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
>
>
Re: Review Request 35739: Patch for DRILL-3333
Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
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 (updated)
-----
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