hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sichi" <jsi...@fb.com>
Subject Re: Review Request: HIVE-1262
Date Fri, 24 Dec 2010 00:04:02 GMT

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



http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
<https://reviews.apache.org/r/192/#comment120>

    You're missing the Apache header on this file.



http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
<https://reviews.apache.org/r/192/#comment122>

    This method formats rather than generates, so it should be named accordingly.



http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
<https://reviews.apache.org/r/192/#comment121>

    Do the call to key_string.getBytes only once rather than over and over in this method.



http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
<https://reviews.apache.org/r/192/#comment123>

    You could use System.arraycopy here



http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
<https://reviews.apache.org/r/192/#comment124>

    Here we may be chopping off a UTF-8 multibyte sequence in the middle.  Won't that lead
to an invalid UTF-8?
    
    Also, I don't think you can use StringBuffer.append(byte []) like this.  Won't that call
the append(Object) method, which will dump the array address?
    
    Anyway, couldn't this just return byte [] since that's what we need later on anyway?
    
    (Aside:  anywhere you really do need StringBuffer, you should be using StringBuilder instead.)
    



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java
<https://reviews.apache.org/r/192/#comment129>

    "computer?"



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java
<https://reviews.apache.org/r/192/#comment125>

    Shouldn't this be deterministic=true?
    



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java
<https://reviews.apache.org/r/192/#comment126>

    whitespace
    



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java
<https://reviews.apache.org/r/192/#comment127>

    Shouldn't we throw these fatals rather than just logging?



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesEncrypt.java
<https://reviews.apache.org/r/192/#comment128>

    Same comments as decrypt



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCrc32.java
<https://reviews.apache.org/r/192/#comment130>

    Some useful variants would be UDTF (row-wise) and UDAF (over an entire table or group).



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMD5.java
<https://reviews.apache.org/r/192/#comment131>

    For performance, initialize this only once (in initialize method) and then reset per row.



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSha.java
<https://reviews.apache.org/r/192/#comment132>

    See MD5 comment regarding initializing only once.



http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSha.java
<https://reviews.apache.org/r/192/#comment133>

    This code is all the same as MD5, so might as well create a common base class and share
it.



http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/udf_aes.q
<https://reviews.apache.org/r/192/#comment134>

    For test determinism, add ORDER BY on queries which return multiple rows.


- John


On 2010-12-23 16:03:56, John Sichi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/192/
> -----------------------------------------------------------
> 
> (Updated 2010-12-23 16:03:56)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Review by JVS
> 
> 
> This addresses bug HIVE-1262.
>     https://issues.apache.org/jira/browse/HIVE-1262
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
1038444 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesEncrypt.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCrc32.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMD5.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSha.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/udf_aes.q
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/udf_crc32.q
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/udf_md5.q
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/udf_sha.q
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/show_functions.q.out
1038444 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/udf_aes.q.out
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/udf_crc32.q.out
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/udf_md5.q.out
PRE-CREATION 
>   http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/udf_sha.q.out
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/192/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> John
> 
>


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