Re: Review Request: HIVE-1229: take two

[bk...@gmail.com]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/239/#review309
-----------------------------------------------------------



http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSerDe.java
<http://review.hbase.org/r/239/#comment1362>

    I have added a HashMap to HBaseSerDe to cache the String to byte [] mapping. The code looks it up for the serialization path. The map is also passed to LazyHBaseRow and LazyHBaseCellMap for lookup during deserialization. In addition I have moved some function calls to serdeParams and saved their return values as instance variables to reduce the per row calls.



http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSplit.java
<http://review.hbase.org/r/239/#comment1363>

    I have left this in the 3rd patch. Thanks for explaining this. I don't think we have a test case which exposes this or even whether the serde instance and the record reader instance could get out of sync, but good idea to leave it in.
    
    The failing tests were due to an improperly initialized Scan instance.



http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHBaseTableOutputFormat.java
<http://review.hbase.org/r/239/#comment1366>

    This OutputFormat is from the deprecated mapred package - this is needed to keep it compatible with the storage handler which needs it to be compatible with this -- see the getOutputFormat() method.



http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseCellMap.java
<http://review.hbase.org/r/239/#comment1365>

    Actually, parse() should also be called only once. I have added the missing call to set parsed to true. In addition the cached values are now passed in from the serde to lazy row to the lazy cell map in the deserialization path.



http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java
<http://review.hbase.org/r/239/#comment1364>

    Done, reverted these changes.


- bkm


On 2010-06-28 12:13:44, John Sichi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/239/
> -----------------------------------------------------------
> 
> (Updated 2010-06-28 12:13:44)
> 
> 
> Review request for Hive Developers.
> 
> 
> Summary
> -------
> 
> review by JVS (please ignore the previous one I created a few minutes ago with the bad patch by accident)
> 
> 
> This addresses bug HIVE-1229.
>     http://issues.apache.org/jira/browse/HIVE-1229
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSerDe.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseSplit.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHBaseTableInputFormat.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHBaseTableOutputFormat.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/HiveHFileOutputFormat.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseCellMap.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/test/org/apache/hadoop/hive/hbase/HBaseTestSetup.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseSerDe.java 957296 
>   http://svn.apache.org/repos/asf/hadoop/hive/trunk/hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java 957296 
> 
> Diff: http://review.hbase.org/r/239/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> John
> 
>