You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ning Zhang <nz...@fb.com> on 2011/03/11 23:59:46 UTC
Review Request: HIVE-2049. Push down partition pruning to JDO filtering for
a subset of partition predicates
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/
-----------------------------------------------------------
Review request for hive.
Summary
-------
* expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
* only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
Diffs
-----
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1080788
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1080788
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1080788
trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1080788
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1080788
Diff: https://reviews.apache.org/r/489/diff
Testing
-------
Thanks,
Ning
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by M IS <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review325
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
<https://reviews.apache.org/r/489/#comment659>
Here we could have specified the exact size of the ArrayList that is bing created.
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment657>
Here in this case, we really need not catch the HiveException explicitly, as we'll end up creating one more instance of HiveException as part of the catch block that catches Exception block
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment658>
It is always preferred that the reference type is generic and not tied to the implementation, unless it gives access to additional methods. So, changing the type from linkedHashMap to Map would be preferred.
- M
On 2011-03-11 14:59:46, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-11 14:59:46)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1080788
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1080788
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by Ning Zhang <nz...@fb.com>.
> On 2011-03-16 20:19:58, M IS wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1786
> > <https://reviews.apache.org/r/489/diff/2/?file=14162#file14162line1786>
> >
> > Why can't we just do :
> > <code>
> >
> > return (value instanceof String);
> >
> > </code>
Yeah, I like that.
> On 2011-03-16 20:19:58, M IS wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1798
> > <https://reviews.apache.org/r/489/diff/2/?file=14162#file14162line1798>
> >
> > Why can't we just do :
> > <code>
> >
> > return (fs.getType().equals(Constants.STRING_TYPE_NAME));
> >
> > </code>
will do also in the next patch.
- Ning
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review336
-----------------------------------------------------------
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by M IS <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review336
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/489/#comment679>
Why can't we just do :
<code>
return (value instanceof String);
</code>
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/489/#comment680>
Why can't we just do :
<code>
return (fs.getType().equals(Constants.STRING_TYPE_NAME));
</code>
- M
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by namit jain <nj...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review341
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/489/#comment693>
Add comments
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/489/#comment694>
You are assuming here that the expression only
contains partition columns - add it in the comments
for the function
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment696>
Add comments
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment695>
Please add comments
- namit
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by namit jain <nj...@fb.com>.
> On 2011-03-21 13:45:49, namit jain wrote:
> > It would be very useful to have the JDO expression for the table under consideration
in the explain plan (extended).
The client side logs are very difficult to debug with.
- namit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review342
-----------------------------------------------------------
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by namit jain <nj...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review342
-----------------------------------------------------------
It would be very useful to have the JDO expression for the table under consideration
- namit
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/
-----------------------------------------------------------
(Updated 2011-03-21 16:02:55.219077)
Review request for hive.
Changes
-------
The new patch's major change is to move the checkJDOPushDown after compactExpr. Previously checkJDOPushDown checks the original prunerExpr. If prunerExpr contains non-partition columns and there are functions other than {=,AND, OR}, this predicate is not qualified for JDO push down. After moving to applying checkJDOPushDown to compactExpr, expressions are qualified as long as the subexpression that contains only partition columns contains {=, AND, OR}.
Summary
-------
* expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
* only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
Diffs (updated)
-----
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1083890
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1083890
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1083890
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1083890
trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1083890
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1083890
trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/489/diff
Testing
-------
Thanks,
Ning
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/
-----------------------------------------------------------
(Updated 2011-03-16 16:50:15.347839)
Review request for hive.
Changes
-------
Add more changes to suite the current implementation of JDO filtering. Some major changes are:
- ExpressionTree.makeFilterForEquals(): previous '=' are translated to JDO method matches(). This will introduce false positives if the partition value contains regex special characters (e.g., dot). I changed this function to use startWith(), endsWith(), and indexOf() in the case of whether the partition column is at the beginning, end or middle of the partition spec string. Two unit tests files (ppr_pushdown*.q) are added to test these cases.
- ObjectStore.listMPartitions(): added a query.setOrder() to return partitions ordered by their partition names. This is to be backward compatible with the old partition pruning behavior.
- PartitionPruner.prune(): check if the partition pruning expression contains non-partition columns. If so add the resulting partitions to unkn_parts, otherwise to true_parts. This is required by down stream optimizations.
- Utilities.checkJDOPushDown(): return true only if the partition column type is string and constant type is string. This is required by the current implementation of JDO filter (see ExpressionTree.java and Filter.g).
Passed all unit tests.
Summary
-------
* expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
* only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
Diffs (updated)
-----
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/489/diff
Testing
-------
Thanks,
Ning
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by M IS <mi...@gmail.com>.
> On 2011-03-16 16:37:44, Ning Zhang wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java, line 244
> > <https://reviews.apache.org/r/489/diff/1/?file=13887#file13887line244>
> >
> > I agree in general we should use a generic interface in the declaration, but I'd prefer leave the LinkedHashMap here. The reason is that what we need for the partSpec is an ordered map where the order of iterator.getNext() should be the same order of elements being inserted. Unfortunately Java collections doesn't have this interface but just an implementation. We could declare an interface just for that, but that should be a different issue.
Using the generic interface (java.util.Map in this context) in declaration will not affect the expected FIFO functionality.
Further, when this map instance is being passed around to get the partition in the getPartition(...) method, the reference type is generic Map and not implementation type.
Here is a sample program to illustrate that using Map as reference for an LinkedHashMap maintains the order:
public class Sample {
public static void main(String[] args) {
Map<String, String> map = new LinkedHashMap<String, String>();
map.put("A", "A");
map.put("B", "B");
map.put("C", "C");
map.put("D", "D");
map.put("E", "E");
map.put("F", "F");
map.put("G", "G");
Iterator<String> iter = map.keySet().iterator();
while (iter.hasNext()) {
System.out.println(map.get(iter.next()));
}
for (Map.Entry<String, String> entry : map.entrySet()) {
System.out.println(entry.getKey() + " " + entry.getValue());
}
}
}
> On 2011-03-16 16:37:44, Ning Zhang wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java, line 216
> > <https://reviews.apache.org/r/489/diff/1/?file=13887#file13887line216>
> >
> > I think explicitly catching HiveException and throw it away will save the creation of HiveException in the catch(Exception) block, right?
Fine.
- M
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review333
-----------------------------------------------------------
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by Ning Zhang <nz...@fb.com>.
> On 2011-03-16 16:37:44, Ning Zhang wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java, line 244
> > <https://reviews.apache.org/r/489/diff/1/?file=13887#file13887line244>
> >
> > I agree in general we should use a generic interface in the declaration, but I'd prefer leave the LinkedHashMap here. The reason is that what we need for the partSpec is an ordered map where the order of iterator.getNext() should be the same order of elements being inserted. Unfortunately Java collections doesn't have this interface but just an implementation. We could declare an interface just for that, but that should be a different issue.
>
> M IS wrote:
> Using the generic interface (java.util.Map in this context) in declaration will not affect the expected FIFO functionality.
> Further, when this map instance is being passed around to get the partition in the getPartition(...) method, the reference type is generic Map and not implementation type.
> Here is a sample program to illustrate that using Map as reference for an LinkedHashMap maintains the order:
>
> public class Sample {
> public static void main(String[] args) {
> Map<String, String> map = new LinkedHashMap<String, String>();
>
> map.put("A", "A");
> map.put("B", "B");
> map.put("C", "C");
> map.put("D", "D");
> map.put("E", "E");
> map.put("F", "F");
> map.put("G", "G");
>
> Iterator<String> iter = map.keySet().iterator();
>
> while (iter.hasNext()) {
> System.out.println(map.get(iter.next()));
> }
>
> for (Map.Entry<String, String> entry : map.entrySet()) {
> System.out.println(entry.getKey() + " " + entry.getValue());
> }
> }
> }
I think I didn't make myself clear here. I know if the variable is declared as Map but the implementation is LinkedHashMap, it will preserve the order property. My concern is that if people only look at the declaration of partSpec without look deep inside the implementation of Warehouse.makePartName(), they only know it is a Map type. People may think they can do a set of operations (e.g., inserts) to this data structure in arbitrary order because order is not important/guaranteed in Map. It is easier to introduce bugs. If we declare it as LinkedHashMap, it is a reminder to people to think twice about the order of operations to this data structure.
- Ning
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review333
-----------------------------------------------------------
On 2011-03-16 16:50:15, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-16 16:50:15)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1081948
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1081948
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1081948
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown.q PRE-CREATION
> trunk/ql/src/test/queries/clientpositive/ppr_pushdown2.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown.q.out PRE-CREATION
> trunk/ql/src/test/results/clientpositive/ppr_pushdown2.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>
Re: Review Request: HIVE-2049. Push down partition pruning to JDO filtering
for a subset of partition predicates
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/489/#review333
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
<https://reviews.apache.org/r/489/#comment674>
will do.
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment673>
I think explicitly catching HiveException and throw it away will save the creation of HiveException in the catch(Exception) block, right?
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
<https://reviews.apache.org/r/489/#comment675>
I agree in general we should use a generic interface in the declaration, but I'd prefer leave the LinkedHashMap here. The reason is that what we need for the partSpec is an ordered map where the order of iterator.getNext() should be the same order of elements being inserted. Unfortunately Java collections doesn't have this interface but just an implementation. We could declare an interface just for that, but that should be a different issue.
- Ning
On 2011-03-11 14:59:46, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/489/
> -----------------------------------------------------------
>
> (Updated 2011-03-11 14:59:46)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> * expose HiveMetaStoreClient.listPartitionsByFilter() to Hive.java so that PartitionPruner can use that for certain partition predicates.
> * only allows {=, AND, OR} in the partition predicates that can be pushed down to JDO filtering.
>
>
> Diffs
> -----
>
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1080788
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1080788
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 1080788
>
> Diff: https://reviews.apache.org/r/489/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>