You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Venki Korukanti <ve...@gmail.com> on 2014/12/23 01:41:57 UTC
Review Request 29343: DRILL-1786: Pushdown filter into InfoSchema POJO
Record Generator to avoid visiting unnecessary schemas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29343/
-----------------------------------------------------------
Review request for drill and Mehant Baid.
Bugs: DRILL-1786
https://issues.apache.org/jira/browse/DRILL-1786
Repository: drill-git
Description
-------
Please see the JIRA DRILL-1786
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java e61e0fe
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java 21d563c
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java ba901a8
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java cc9ee78
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaConstants.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilterBuilder.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java 5a18033
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaStoragePlugin.java f73bc05
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaSubScan.java 70e1258
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaTable.java 0cf0481
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/RecordGenerator.java 5ef8ad2
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/SelectedTable.java 5f56221
exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestInfoSchemaFilterPushDown.java PRE-CREATION
Diff: https://reviews.apache.org/r/29343/diff/
Testing
-------
Added unit test to test the filter pushdown.
Thanks,
Venki Korukanti
Re: Review Request 29343: DRILL-1786: Pushdown filter into InfoSchema
POJO Record Generator to avoid visiting unnecessary schemas
Posted by Venki Korukanti <ve...@gmail.com>.
> On Dec. 31, 2014, 7:31 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java, line 103
> > <https://reviews.apache.org/r/29343/diff/2/?file=799727#file799727line103>
> >
> > Here, we assume all the Constants have to be string type? I see some column might be numeric, like "NUMERIC_PRECISION", "ORDINAL_POSITION".
Currently the goal is to filter predicates on SCHEMA_NAME and TABLE_NAME (which are strings) as traversing a schema/table is expensive and would want to avoid visiting if it is not needed. Predicates on "NUMERIC_PRECISION" and "ORDINAL_POSITION" don't influence visiting a schema or table.
> On Dec. 31, 2014, 7:31 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java, line 87
> > <https://reviews.apache.org/r/29343/diff/2/?file=799730#file799730line87>
> >
> > Why do you add this check?
> >
> > Is it possible multiple filters could be pushed down seperately? For instance,
> >
> > SELECT ...
> > From ( SELECT T1.COL1, T1.COL2, T1.COL3 from T1 where T1.COL2 = 'abc') X1
> > JOIN
> > (Select T2.COL4, T2.COL5, T2.COL6 from T2 where T2.COL5 = 'def') X2
> > ON X1.col1 = X2.col4 and X1.col3 = 'hij';
> >
> > Here, scan of T1 might have filter pushed from its local condition (T1.COL2 = 'abc'), and from the join ON condition, although I guess such use case might not be typical for INFORMATION_SCHEMA tables.
It is to avoid transforming the already transformed filter -> scan sequence where we could not push the complete filter into scan. We want to avoid re-processing the "filter -> scan" sequence.
- Venki
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29343/#review66480
-----------------------------------------------------------
On Dec. 23, 2014, 9:06 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29343/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2014, 9:06 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Bugs: DRILL-1786
> https://issues.apache.org/jira/browse/DRILL-1786
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Please see the JIRA DRILL-1786
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java e61e0fe
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java 21d563c
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java ba901a8
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java cc9ee78
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaConstants.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilterBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java 5a18033
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaStoragePlugin.java f73bc05
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaSubScan.java 70e1258
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaTable.java 0cf0481
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/RecordGenerator.java 5ef8ad2
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/SelectedTable.java 5f56221
> exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestInfoSchemaFilterPushDown.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29343/diff/
>
>
> Testing
> -------
>
> Added unit test to test the filter pushdown.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 29343: DRILL-1786: Pushdown filter into InfoSchema
POJO Record Generator to avoid visiting unnecessary schemas
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29343/#review66480
-----------------------------------------------------------
Ship it!
+1.
Overall looks good to me. Just have the following minor comments.
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java
<https://reviews.apache.org/r/29343/#comment110053>
Here, we assume all the Constants have to be string type? I see some column might be numeric, like "NUMERIC_PRECISION", "ORDINAL_POSITION".
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java
<https://reviews.apache.org/r/29343/#comment110052>
Why do you add this check?
Is it possible multiple filters could be pushed down seperately? For instance,
SELECT ...
From ( SELECT T1.COL1, T1.COL2, T1.COL3 from T1 where T1.COL2 = 'abc') X1
JOIN
(Select T2.COL4, T2.COL5, T2.COL6 from T2 where T2.COL5 = 'def') X2
ON X1.col1 = X2.col4 and X1.col3 = 'hij';
Here, scan of T1 might have filter pushed from its local condition (T1.COL2 = 'abc'), and from the join ON condition, although I guess such use case might not be typical for INFORMATION_SCHEMA tables.
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java
<https://reviews.apache.org/r/29343/#comment110051>
After "partial" filter is pushed into scan, we copy the orignal filter in the new FilterRel. This means we will evaluate the pushed-filter twice (one in SCAN, one in Filter)?
I guess probably it does not impact a lot for InfoSchemaGroupScan performance, since it's relative small. But in general, we would like to avoid such duplicate evaluation.
- Jinfeng Ni
On Dec. 23, 2014, 1:06 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29343/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2014, 1:06 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Bugs: DRILL-1786
> https://issues.apache.org/jira/browse/DRILL-1786
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Please see the JIRA DRILL-1786
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java e61e0fe
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java 21d563c
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java ba901a8
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java cc9ee78
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaConstants.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilterBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java 5a18033
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaStoragePlugin.java f73bc05
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaSubScan.java 70e1258
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaTable.java 0cf0481
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/RecordGenerator.java 5ef8ad2
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/SelectedTable.java 5f56221
> exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestInfoSchemaFilterPushDown.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29343/diff/
>
>
> Testing
> -------
>
> Added unit test to test the filter pushdown.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 29343: DRILL-1786: Pushdown filter into InfoSchema
POJO Record Generator to avoid visiting unnecessary schemas
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29343/#review66094
-----------------------------------------------------------
Ship it!
Looks good
- Mehant Baid
On Dec. 23, 2014, 9:06 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29343/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2014, 9:06 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Bugs: DRILL-1786
> https://issues.apache.org/jira/browse/DRILL-1786
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Please see the JIRA DRILL-1786
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java e61e0fe
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java 21d563c
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java ba901a8
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java cc9ee78
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaConstants.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilterBuilder.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java 5a18033
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaStoragePlugin.java f73bc05
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaSubScan.java 70e1258
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaTable.java 0cf0481
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/RecordGenerator.java 5ef8ad2
> exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/SelectedTable.java 5f56221
> exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestInfoSchemaFilterPushDown.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29343/diff/
>
>
> Testing
> -------
>
> Added unit test to test the filter pushdown.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 29343: DRILL-1786: Pushdown filter into InfoSchema
POJO Record Generator to avoid visiting unnecessary schemas
Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29343/
-----------------------------------------------------------
(Updated Dec. 23, 2014, 9:06 p.m.)
Review request for drill and Mehant Baid.
Bugs: DRILL-1786
https://issues.apache.org/jira/browse/DRILL-1786
Repository: drill-git
Description
-------
Please see the JIRA DRILL-1786
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeTableHandler.java e61e0fe
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowSchemasHandler.java 21d563c
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java ba901a8
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java cc9ee78
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaConstants.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilterBuilder.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java 5a18033
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaPushFilterIntoRecordGenerator.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaStoragePlugin.java f73bc05
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaSubScan.java 70e1258
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaTable.java 0cf0481
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/RecordGenerator.java 5ef8ad2
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/SelectedTable.java 5f56221
exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestInfoSchemaFilterPushDown.java PRE-CREATION
Diff: https://reviews.apache.org/r/29343/diff/
Testing
-------
Added unit test to test the filter pushdown.
Thanks,
Venki Korukanti