> On March 7, 2014, 6:22 a.m., Xuefu Zhang wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseWritableKeyFactory.java,
> >  line 26
> > <https://reviews.apache.org/r/18179/diff/4/?file=513319#file513319line26>
> >
> >     Don't we expect this key type handles predicate pushdown?

Firstly it was. But predicate is for reading and it could be implemented for 
read only factories(HBaseKeyFactory). So I thought it would be better to split 
predicate handler from key factory. I'll add some comments.


> On March 7, 2014, 6:22 a.m., Xuefu Zhang wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseCompositeKey.java, 
> > line 116
> > <https://reviews.apache.org/r/18179/diff/4/?file=513313#file513313line116>
> >
> >     I saw the same method, but defined in several different classes. This 
> > seems confusing, which might needs some refactoring.

The fetch in HIVE-6290 makes filter in HBaseCompositeKey. But in here I've 
delegated it to factory implementation. So it is like this. If Swarnim allows 
the filter logic to be moved into factory, this can be removed.


> On March 7, 2014, 6:22 a.m., Xuefu Zhang wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java,
> >  line 407
> > <https://reviews.apache.org/r/18179/diff/4/?file=513318#file513318line407>
> >
> >     Because HBaseCompositeKey has a default implementation (return null) 
> > for decomposePredicate(), it seems this creates a different behavior as 
> > subsquenct code will not be exercised.

Right. I should implement default behavior in HBaseCompositeKey.


- Navis


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


On March 7, 2014, 3:32 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18179/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 3:32 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6411
>     https://issues.apache.org/jira/browse/HIVE-6411
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-2599 introduced using custom object for the row key. But it forces key 
> objects to extend HBaseCompositeKey, which is again extension of LazyStruct. 
> If user provides proper Object and OI, we can replace internal key and keyOI 
> with those. 
> 
> Initial implementation is based on factory interface.
> {code}
> public interface HBaseKeyFactory {
>   void init(SerDeParameters parameters, Properties properties) throws 
> SerDeException;
>   ObjectInspector createObjectInspector(TypeInfo type) throws SerDeException;
>   LazyObjectBase createObject(ObjectInspector inspector) throws 
> SerDeException;
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   hbase-handler/pom.xml 132af43 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseCompositeKey.java 
> 5008f15 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseKeyFactory.java 
> PRE-CREATION 
>   
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseLazyObjectFactory.java
>  PRE-CREATION 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseScanRange.java 
> PRE-CREATION 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSerDe.java 2cd65cb 
>   
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 
> 29e5da5 
>   
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseWritableKeyFactory.java
>  PRE-CREATION 
>   
> hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHBaseTableInputFormat.java
>  704fcb9 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java 
> fc40195 
>   
> hbase-handler/src/test/org/apache/hadoop/hive/hbase/HBaseTestCompositeKey.java
>  13c344b 
>   
> hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseKeyFactory.java 
> PRE-CREATION 
>   
> hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseKeyFactory2.java 
> PRE-CREATION 
>   hbase-handler/src/test/queries/positive/hbase_custom_key.q PRE-CREATION 
>   hbase-handler/src/test/queries/positive/hbase_custom_key2.q PRE-CREATION 
>   hbase-handler/src/test/results/positive/hbase_custom_key.q.out PRE-CREATION 
>   hbase-handler/src/test/results/positive/hbase_custom_key2.q.out 
> PRE-CREATION 
>   itests/util/pom.xml e9720df 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java b966d33 
>   ql/src/java/org/apache/hadoop/hive/ql/index/IndexPredicateAnalyzer.java 
> d39ee2e 
>   ql/src/java/org/apache/hadoop/hive/ql/index/IndexSearchCondition.java 
> 5f1329c 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 647a9a6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStoragePredicateHandler.java
>  9f35575 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java e50026b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 10bae4d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java 40298e1 
>   serde/src/java/org/apache/hadoop/hive/serde2/StructObject.java PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/StructObjectBaseInspector.java 
> PRE-CREATION 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/columnar/ColumnarStructBase.java 
> 1fd6853 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyObject.java 10f4c05 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyObjectBase.java 
> 3334dff 
>   serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyStruct.java 8a1ea46 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazySimpleStructObjectInspector.java
>  8a5386a 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryObject.java 
> 598683f 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryStruct.java 
> caf3517 
> 
> Diff: https://reviews.apache.org/r/18179/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Reply via email to