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