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