hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Navis Ryu" <navis....@nexr.com>
Subject Re: Review Request 18179: Support more generic way of using composite key for HBaseHandler
Date Tue, 11 Mar 2014 02:24:21 GMT


> On March 10, 2014, 10:22 p.m., Swarnim Kulkarni wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseScanRange.java, line 100
> > <https://reviews.apache.org/r/18179/diff/4/?file=513316#file513316line100>
> >
> >     I think we are restricting the capability here by limiting the type of filters
that can be provided. This might be difficult to evolve and is less dynamic as consumers cannot
plugin their custom filters. Also the toByteArray() method on line 60 for filter worries me
a little bit a custom filter implementation might not have it properly implemented.
> >     
> >     Any reason we cannot get rid of this FilterDesc class completely and instead
just have List<Filter> instead of List<FilterDesc> on line 41?

Currently, ExprNodeDesc for predicates are serialized into a string and conveyed to InputFormat#getSplits
and there is no other way to provide custom objects to InputFormat except using thread local.
I didn't wanted to add one more thread local so I decided to serialized Filters. But Filter
is not serializable instance. 
I didn't tried to serialize it with kryo, but felt not a safe way. I might extend this part
more generic (serialize FQCN of filter and find static parseFrom() method in the class to
instantiate the Filter).


> On March 10, 2014, 10:22 p.m., Swarnim Kulkarni 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>
> >
> >     One of my primary motivation behind adding this here was to provide the capability
to consumers to be able to plugin any custom filter implementation down to the hbase scan
and tieing it with the custom key implementation looked like an apt place to do so.
> >     
> >     TL;DR I am fine with removing this as long as the factory implementation can
support a similar capability.

Thanks. Then I can do some more refactoring.


- Navis


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


On March 7, 2014, 7:46 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, 7:46 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/HBaseCompositeKeyFactory.java PRE-CREATION

>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message