You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by abdelhakim deneche <ad...@gmail.com> on 2014/11/24 23:33:00 UTC
Review Request 28417: DRILL-1742 Use Hive stats when planning queries
on Hive data sources
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/
-----------------------------------------------------------
Review request for drill.
Bugs: DRILL-1742
https://issues.apache.org/jira/browse/DRILL-1742
Repository: drill-git
Description
-------
HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
Diffs
-----
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
Diff: https://reviews.apache.org/r/28417/diff/
Testing
-------
created several partitioned and non-partitioned tables, loaded data in hive.
querying the tables I checked the logs to make sure the correct number of rows is computed.
Thanks,
abdelhakim deneche
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
> On Nov. 25, 2014, 12:12 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, line 93
> > <https://reviews.apache.org/r/28417/diff/1/?file=774768#file774768line93>
> >
> > This value is not initialized and only assigned to in the copy constructor (other 2 constructors are not doing anything with it).
This value is being computed inside getSplits() which in turn is called by the 2 constructors.
> On Nov. 25, 2014, 12:12 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, line 293
> > <https://reviews.apache.org/r/28417/diff/1/?file=774768#file774768line293>
> >
> > Why does the estRowCount need to be here if it is not being used any more ?
Indeed, but the new patch does use it after all
> On Nov. 25, 2014, 12:12 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, line 162
> > <https://reviews.apache.org/r/28417/diff/1/?file=774768#file774768line162>
> >
> > An assumption here is that the numRows property is always available...you should handle the scenario when it isn't.
I will submit a patch right away to fix this
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review62903
-----------------------------------------------------------
On Nov. 25, 2014, 12:13 a.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 12:13 a.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review62903
-----------------------------------------------------------
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105055>
This value is not initialized and only assigned to in the copy constructor (other 2 constructors are not doing anything with it).
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105061>
An assumption here is that the numRows property is always available...you should handle the scenario when it isn't.
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105062>
Same as above.
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105058>
Why does the estRowCount need to be here if it is not being used any more ?
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105059>
Why is the property NO_EXACT_ROW_COUNT if we are getting the exact row count from Hive ?
- Aman Sinha
On Nov. 24, 2014, 10:33 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 24, 2014, 10:33 p.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> querying the tables I checked the logs to make sure the correct number of rows is computed.
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
> On Nov. 25, 2014, 1:23 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, line 93
> > <https://reviews.apache.org/r/28417/diff/3/?file=775081#file775081line93>
> >
> > I still think this needs to be initialized and not depend on getSplits() since obviously after your latest changes, the rowCount property is not assumed to be available. Also, see my later comment about distinguishing between an empty table (0 rowcount) and one where stats is not available.
The problem is, at least with hive 0.12.0, when a table has rows but the statistics haven't been computed yet, the "numRows" property will be available and will contain the value of 0. So getting a numRows=0 doesn't actually tell us much about the size of the table, that's why leaving rowCount initialized to 0 is correct.
It should be noted that according to hive [documentation](https://cwiki.apache.org/confluence/display/Hive/StatsDev#StatsDev-NewlyCreatedTables) tables should always have their statistics computed, so a missing "numRows" property looks more like a bug in hive 0.12.0 (I will try a more recent version of hive to see if the problem persists).
> On Nov. 25, 2014, 1:23 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, lines 298-300
> > <https://reviews.apache.org/r/28417/diff/3/?file=775081#file775081line298>
> >
> > This is not necessarily true; if you have empty tables, the rowcount will be 0. So you need to distinguish between the case where the stats are not available (maybe use -1 as an indicator) from the case where it is available and has 0 rowcount.
The problem is that when numRows=0 in the stats can actually mean the stats have not been computed yet! so we still need to estimate the row count using the size of the input splits.
I made some tests using empty tables, and the estimated row count for those tables is 0 too, so it's correct.
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review62916
-----------------------------------------------------------
On Nov. 25, 2014, 12:56 a.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 12:56 a.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
> On Nov. 25, 2014, 1:23 a.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java, lines 298-300
> > <https://reviews.apache.org/r/28417/diff/3/?file=775081#file775081line298>
> >
> > This is not necessarily true; if you have empty tables, the rowcount will be 0. So you need to distinguish between the case where the stats are not available (maybe use -1 as an indicator) from the case where it is available and has 0 rowcount.
>
> abdelhakim deneche wrote:
> The problem is that when numRows=0 in the stats can actually mean the stats have not been computed yet! so we still need to estimate the row count using the size of the input splits.
> I made some tests using empty tables, and the estimated row count for those tables is 0 too, so it's correct.
in hive 0.13 numRows will contain -1 when the stats were never computed.
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review62916
-----------------------------------------------------------
On Nov. 25, 2014, 7:46 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 7:46 p.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review62916
-----------------------------------------------------------
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105085>
I still think this needs to be initialized and not depend on getSplits() since obviously after your latest changes, the rowCount property is not assumed to be available. Also, see my later comment about distinguishing between an empty table (0 rowcount) and one where stats is not available.
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
<https://reviews.apache.org/r/28417/#comment105084>
This is not necessarily true; if you have empty tables, the rowcount will be 0. So you need to distinguish between the case where the stats are not available (maybe use -1 as an indicator) from the case where it is available and has 0 rowcount.
- Aman Sinha
On Nov. 25, 2014, 12:56 a.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 12:56 a.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/#review63034
-----------------------------------------------------------
Ship it!
Revised patch looks good. Please see bug comments about tests that were run.
- Aman Sinha
On Nov. 25, 2014, 7:46 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28417/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 7:46 p.m.)
>
>
> Review request for drill.
>
>
> Bugs: DRILL-1742
> https://issues.apache.org/jira/browse/DRILL-1742
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
> We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
>
>
> Diffs
> -----
>
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
>
> Diff: https://reviews.apache.org/r/28417/diff/
>
>
> Testing
> -------
>
> created several partitioned and non-partitioned tables, loaded data in hive.
>
> used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/
-----------------------------------------------------------
(Updated Feb. 12, 2015, 11:53 p.m.)
Review request for drill.
Changes
-------
updated the code to properly handle when numRows=-1
Bugs: DRILL-1742
https://issues.apache.org/jira/browse/DRILL-1742
Repository: drill-git
Description
-------
HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
Diffs (updated)
-----
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java 35db8ef
Diff: https://reviews.apache.org/r/28417/diff/
Testing (updated)
-------
created several partitioned and non-partitioned tables, loaded data in hive.
used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
passes unit tests, along with functional, SF100 and customer regression tests.
Thanks,
abdelhakim deneche
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/
-----------------------------------------------------------
(Updated Nov. 25, 2014, 7:46 p.m.)
Review request for drill.
Changes
-------
updated patch: because hive stats may not be up to date, we use NO_EXACT_ROW_COUNT instead of EXACT_ROW_COUNT
Bugs: DRILL-1742
https://issues.apache.org/jira/browse/DRILL-1742
Repository: drill-git
Description
-------
HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
Diffs (updated)
-----
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
Diff: https://reviews.apache.org/r/28417/diff/
Testing
-------
created several partitioned and non-partitioned tables, loaded data in hive.
used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
Thanks,
abdelhakim deneche
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/
-----------------------------------------------------------
(Updated Nov. 25, 2014, 12:56 a.m.)
Review request for drill.
Changes
-------
handles the case when hive metadata doesn't include a numRows property.
When row count is available we return a GroupScanProperty.EXACT_ROW_COUNT
Bugs: DRILL-1742
https://issues.apache.org/jira/browse/DRILL-1742
Repository: drill-git
Description
-------
HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
Diffs (updated)
-----
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
Diff: https://reviews.apache.org/r/28417/diff/
Testing
-------
created several partitioned and non-partitioned tables, loaded data in hive.
used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
Thanks,
abdelhakim deneche
Re: Review Request 28417: DRILL-1742 Use Hive stats when planning
queries on Hive data sources
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28417/
-----------------------------------------------------------
(Updated Nov. 25, 2014, 12:13 a.m.)
Review request for drill.
Changes
-------
In case hive metadata has 0 as row count, getScanStats() uses the inputSplit size to estimate the number of rows
Bugs: DRILL-1742
https://issues.apache.org/jira/browse/DRILL-1742
Repository: drill-git
Description
-------
HiveScan.getSplits() already gets the table and partitions metadata using MetaStoreUtils.
We compute the total number of rows using the numRows property and store the computed number of rows in rowCount attribute which is later returned by getScanStats().
Diffs (updated)
-----
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java ddbc100
Diff: https://reviews.apache.org/r/28417/diff/
Testing (updated)
-------
created several partitioned and non-partitioned tables, loaded data in hive.
used explain plan to check the number of rows when the whole table is queried and also when specific partitions are queried (to make sure the row count takes hive partition pruning into account)
Thanks,
abdelhakim deneche