hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Swarnim Kulkarni" <kulkarni.swar...@gmail.com>
Subject Re: Review Request 21138: Support more generic way of using composite key for HBaseHandler
Date Thu, 08 May 2014 15:40:33 GMT


> On May 7, 2014, 11:27 p.m., Xuefu Zhang wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/CompositeHBaseKeyFactory.java,
line 132
> > <https://reviews.apache.org/r/21138/diff/1/?file=575776#file575776line132>
> >
> >     Is FamilyFilter appropriate here?

That's exactly what I had asked Navis in the previous review s this wasn't part of my patch.
The only reason I let this pass in my latest patch is because this seems like a default implementation
of the HBaseKeyFactory that supports composite keys. So consumers can choose to extend this
to override the implementation as they see fit. 

One thing that we can probably change on this is to convert the setupFilter() method to be
protected instead of private. So in that way we can provide a capability to simply override
the filter on the factory implementation.

Thoughts?


> On May 7, 2014, 11:27 p.m., Xuefu Zhang wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/CompositeHBaseKeyFactory.java,
line 120
> > <https://reviews.apache.org/r/21138/diff/1/?file=575776#file575776line120>
> >
> >     Is the comment meant for setupFilter()?

I'll move this comment over the Validator#validate.


> On May 7, 2014, 11:27 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java, line 265
> > <https://reviews.apache.org/r/21138/diff/1/?file=575804#file575804line265>
> >
> >     Can we have some comments here? I had difficulty understanding this.

Added.


> On May 7, 2014, 11:27 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java, line 281
> > <https://reviews.apache.org/r/21138/diff/1/?file=575804#file575804line281>
> >
> >     Same as above.

Added.


> On May 7, 2014, 11:27 p.m., Xuefu Zhang wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java, line 174
> > <https://reviews.apache.org/r/21138/diff/1/?file=575814#file575814line174>
> >
> >     I don't see any use of this method.

Cleaned up.


- Swarnim


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


On May 6, 2014, 11:26 p.m., Swarnim Kulkarni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21138/
> -----------------------------------------------------------
> 
> (Updated May 6, 2014, 11:26 p.m.)
> 
> 
> Review request for hive.
> 
> 
> 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/AbstractHBaseKeyFactory.java PRE-CREATION

>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/ColumnMappings.java PRE-CREATION

>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/CompositeHBaseKeyFactory.java PRE-CREATION

>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/DefaultHBaseKeyFactory.java PRE-CREATION

>   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/HBaseRowSerializer.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 5fe35a5 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSerDeParameters.java b64590d

>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 4fe1b1b

>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHBaseTableInputFormat.java
142bfd8 
>   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/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java 7c4fc9f

>   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 113227d 
>   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 4921966 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcStruct.java 293b74e 
>   ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ArrayWritableObjectInspector.java
2a7fdf9 
>   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 ecb82d7 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java c0a8269 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java 5f32f2d 
>   serde/src/java/org/apache/hadoop/hive/serde2/BaseStructObjectInspector.java PRE-CREATION

>   serde/src/java/org/apache/hadoop/hive/serde2/NullStructSerDe.java dba5e33 
>   serde/src/java/org/apache/hadoop/hive/serde2/StructObject.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/LazySimpleSerDe.java 82c1263 
>   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

>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ColumnarStructObjectInspector.java
7d0d91c 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/DelegatedStructObjectInspector.java
5e1a369 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ReflectionStructObjectInspector.java
bd3cdd4 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StructField.java 67827d6

>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/UnionStructObjectInspector.java
60e55ec 
> 
> Diff: https://reviews.apache.org/r/21138/diff/
> 
> 
> Testing
> -------
> 
> Tests added as a part of the patch.
> 
> 
> Thanks,
> 
> Swarnim Kulkarni
> 
>


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