> On July 30, 2015, 11:50 p.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HivePartitionDescriptor.java,
> >  line 104
> > <https://reviews.apache.org/r/36809/diff/2/?file=1023977#file1023977line104>
> >
> >     Do you want this to be case-sensitive comparison ?

The new files are essentially a subset of the old files so case-sensitive 
comparison should suffice.


> On July 30, 2015, 11:50 p.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HivePartitionLocation.java,
> >  line 38
> > <https://reviews.apache.org/r/36809/diff/2/?file=1023978#file1023978line38>
> >
> >     This will throw IOBE if supplied max nesting level is less than 
> > mostDirs.length.

This should not be the case, since in the case of hive, we get max partition 
hierarchy from the partition metadata itself as opposed to in the case of dfs 
where we set a maximum hierarchy of 10. I have added an assert.


> On July 30, 2015, 11:50 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java,
> >  line 89
> > <https://reviews.apache.org/r/36809/diff/2/?file=1023985#file1023985line89>
> >
> >     It wasn't clear why the Hive changes affected this.. is this based on a 
> > different patch from before ?

While creating a patch I missed this minor change, it belongs in 3503. Moved it.


> On July 30, 2015, 11:50 p.m., Aman Sinha wrote:
> > contrib/storage-hive/core/src/test/java/org/apache/drill/exec/TestHivePartitionPruning.java,
> >  line 68
> > <https://reviews.apache.org/r/36809/diff/2/?file=1023984#file1023984line68>
> >
> >     Are there any Hive PP unit tests that reference subdirectories ? Would 
> > be good to add couple of tests.

In testRangeFilter() and testRangeFilterWithDisjunct() column 'c' is the top 
level partition and column 'd' is the next level partition. Is that what you 
were referring to or am I missing something?


- Mehant


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36809/#review93660
-----------------------------------------------------------


On July 31, 2015, 3:32 a.m., Mehant Baid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36809/
> -----------------------------------------------------------
> 
> (Updated July 31, 2015, 3:32 a.m.)
> 
> 
> Review request for drill and Aman Sinha.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Add support for interpreter based partition pruning for hive tables. Also 
> removes the old partition pruning logic.
> 
> 
> Diffs
> -----
> 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HivePartitionDescriptor.java
>  8307dff 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HivePartitionLocation.java
>  PRE-CREATION 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/logical/HivePushPartitionFilterIntoScan.java
>  6ab1a78 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveDataTypeUtility.java
>  PRE-CREATION 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
>  088fb74 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveStoragePlugin.java
>  fb827cc 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTable.java
>  99101cc 
>   
> contrib/storage-hive/core/src/test/java/org/apache/drill/exec/TestHivePartitionPruning.java
>  c846328 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DirPathBuilder.java
>  892e8cb 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushPartitionFilterIntoScan.java
>  b83cedd 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/PartitionPruningUtil.java
>  05ccfb9 
> 
> Diff: https://reviews.apache.org/r/36809/diff/
> 
> 
> Testing
> -------
> 
> Pending unit tests.
> 
> 
> Thanks,
> 
> Mehant Baid
> 
>

Reply via email to