[ 
https://issues.apache.org/jira/browse/HCATALOG-241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13209098#comment-13209098
 ] 

[email protected] commented on HCATALOG-241:
--------------------------------------------------------


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



/trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
<https://reviews.apache.org/r/3819/#comment11260>

    this is only used in tests. Do we really need this in produciton code?



/trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
<https://reviews.apache.org/r/3819/#comment11254>

    Nit: You're throwing away the serDe once you get the ObjectInspector. Just 
thinking you might change this to getSerDe instead and get the object inspector 
from them. That way you can reuse the serDe if need be.
    
    Just noticed this isn't used anywhere in this patch. Is this really used?



/trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
<https://reviews.apache.org/r/3819/#comment11255>

    Nit: Same here..



/trunk/src/java/org/apache/hcatalog/data/HCatRecord.java
<https://reviews.apache.org/r/3819/#comment11256>

    no longer needed, remove.



/trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java
<https://reviews.apache.org/r/3819/#comment11257>

    I'd return an IllegalArgumentException instead. I believe it's an error and 
not an option for the user to pass data which is null?



/trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java
<https://reviews.apache.org/r/3819/#comment11245>

    Nit: I would throw an IllegalArgumentException instead.



/trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java
<https://reviews.apache.org/r/3819/#comment11258>

    I see, this is no longer really a SerDe as hive would expect it to be, it's 
more a converter. Just thinking if it'd be better for it not to implement 
SerDe. Though we don't have to do that now, since this is already checked in.



/trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
<https://reviews.apache.org/r/3819/#comment11246>

    Nit: why no just catch the specific exception. less noise in the stack 
trace.



/trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
<https://reviews.apache.org/r/3819/#comment11247>

    Nit: IllegalStateException...



/trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
<https://reviews.apache.org/r/3819/#comment11249>

    throw UnsupportedOperationException instead.


- Francis


On 2012-02-15 23:13:50, Sushanth Sowmyan wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3819/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-02-15 23:13:50)
bq.  
bq.  
bq.  Review request for Alan Gates and Francis Liu.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Initial upload.
bq.  
bq.  
bq.  This addresses bug HCATALOG-241.
bq.      https://issues.apache.org/jira/browse/HCATALOG-241
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/DefaultHCatRecord.java 1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/HCatRecord.java 1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java 
1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java 1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/HCatRecordable.java 1244779 
bq.    /trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 
PRE-CREATION 
bq.    /trunk/src/test/org/apache/hcatalog/data/TestDefaultHCatRecord.java 
1244779 
bq.    /trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 
1244779 
bq.    
/trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java 
1244779 
bq.    
/trunk/src/test/org/apache/hcatalog/rcfile/TestRCFileInputStorageDriver.java 
1244779 
bq.  
bq.  Diff: https://reviews.apache.org/r/3819/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Sushanth
bq.  
bq.


                
> Changes to HCatRecord to support switch from StorageDriver to SerDe
> -------------------------------------------------------------------
>
>                 Key: HCATALOG-241
>                 URL: https://issues.apache.org/jira/browse/HCATALOG-241
>             Project: HCatalog
>          Issue Type: Sub-task
>    Affects Versions: 0.4
>            Reporter: Alan Gates
>            Assignee: Sushanth Sowmyan
>             Fix For: 0.4
>
>         Attachments: HCATALOG-241.patch, HCATALOG-241.patch.2, 
> HCATALOG-241.patch.3
>
>
> This JIRA tracks changes to HCatRecord.  See HCATALOG-237 for details and 
> design notes.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to