> On 2010-12-23 16:04:03, John Sichi wrote: > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java, > > line 37 > > <https://reviews.apache.org/r/192/diff/1/?file=9231#file9231line37> > > > > Shouldn't this be deterministic=true? > >
The AES algorithm can actually produce multiple intermediates that will decode to the same result. The decode might be deterministic but the encode is definitely not. I figured this was safe. > On 2010-12-23 16:04:03, John Sichi wrote: > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAesDecrypt.java, > > line 85 > > <https://reviews.apache.org/r/192/diff/1/?file=9231#file9231line85> > > > > Shouldn't we throw these fatals rather than just logging? Agreed. What type of Exception should we throw in this case? > On 2010-12-23 16:04:03, John Sichi wrote: > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/AesUtils.java, > > line 16 > > <https://reviews.apache.org/r/192/diff/1/?file=9229#file9229line16> > > > > 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.) > > All other comments are taken. As for the case of cutting apart a UTF-8, that is ok (I think). We need 128 bits for the algorithm regardless of what encoding the source key is in we only need 128 bits of it. - Edward ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/192/#review79 ----------------------------------------------------------- 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 > >
