> On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, > > line 675 > > <https://reviews.apache.org/r/1586/diff/2/?file=41673#file41673line675> > > > > I thought we decided an explicit cast would be required even for > > string<-->binary?
Correct. Accidentally left. Will remove it in the new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFConcat.java, line 36 > > <https://reviews.apache.org/r/1586/diff/2/?file=41678#file41678line36> > > > > @Description annotation for this class needs to be updated. Will update in new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFLength.java, line 55 > > <https://reviews.apache.org/r/1586/diff/2/?file=41679#file41679line55> > > > > @Description annotation for this class needs to be updated. Will update in new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java, > > line 29 > > <https://reviews.apache.org/r/1586/diff/2/?file=41682#file41682line29> > > > > This UDF class needs an @Description annotation. Will update in new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFSubstr.java, line 43 > > <https://reviews.apache.org/r/1586/diff/2/?file=41680#file41680line43> > > > > @Description annotation for this class needs to be updated. Will update in new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java, > > line 67 > > <https://reviews.apache.org/r/1586/diff/2/?file=41682#file41682line67> > > > > The CAST should have parentheses, so > > > > CAST(x AS BINARY) > > > > (this applies to the existing code in GenericUDFTimestamp too) > > Will update in new patch. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/ql/src/test/queries/clientpositive/ba_table1.q, line 9 > > <https://reviews.apache.org/r/1586/diff/2/?file=41683#file41683line9> > > > > Use ORDER BY on a key for all queries to guarantee test determinism. > > Also, since test output keeps getting bigger and bigger, it's best to > > filter down to just a few rows to keep it easier to review and manage. > > > > (This comment applies to all tests in this patch.) Added order-by to guarantee determinism and limit to keep number of rows small in all the tests. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/SerDeUtils.java, line 267 > > <https://reviews.apache.org/r/1586/diff/2/?file=41696#file41696line267> > > > > Is there a test for this somewhere? It seems like a case where we need > > base64. Missed the changes in DelimitedJSONserde so it was not getting used. DelimitedJSONserde now uses this method to print, so its gets used by all the tests. > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarSerDe.java, > > line 83 > > <https://reviews.apache.org/r/1586/diff/2/?file=41698#file41698line83> > > > > I'm not sure about the original connection between UTF and strings here > > (was something forcing a UTF encoding), but is it valid for arbitrary > > binary data? You are correct, this may break when there is a binary data of exactly one byte long and with that bit pattern, it will return null instead of that bit pattern. But thats a pretty corner-case. We can document something along the line "If there is a possibility that your binary data may contain exactly one byte of 10111111, don't use LazyBinaryColumnarSerde with binary data type." > On 2011-09-27 00:43:03, John Sichi wrote: > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java, > > line 393 > > <https://reviews.apache.org/r/1586/diff/2/?file=41716#file41716line393> > > > > Since we decided we're not going to support implicit conversions, these > > don't belong here, right? Correct. Removed. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1586/#review2076 ----------------------------------------------------------- On 2011-09-18 05:52:41, Ashutosh Chauhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1586/ > ----------------------------------------------------------- > > (Updated 2011-09-18 05:52:41) > > > Review request for hive. > > > Summary > ------- > > This patch adds bytearray as a new datatype in Hive. > > > This addresses bug HIVE-2380. > https://issues.apache.org/jira/browse/HIVE-2380 > > > Diffs > ----- > > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java > 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java > 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFConcat.java 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFLength.java 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFSubstr.java 1172168 > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToString.java 1172168 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java > PRE-CREATION > trunk/ql/src/test/queries/clientpositive/ba_table1.q PRE-CREATION > trunk/ql/src/test/queries/clientpositive/ba_table2.q PRE-CREATION > trunk/ql/src/test/queries/clientpositive/ba_table_udfs.q PRE-CREATION > trunk/ql/src/test/results/clientpositive/ba_table1.q.out PRE-CREATION > trunk/ql/src/test/results/clientpositive/ba_table2.q.out PRE-CREATION > trunk/ql/src/test/results/clientpositive/ba_table_udfs.q.out PRE-CREATION > trunk/serde/if/serde.thrift 1172168 > trunk/serde/src/gen/thrift/gen-cpp/serde_constants.h 1172168 > trunk/serde/src/gen/thrift/gen-cpp/serde_constants.cpp 1172168 > > trunk/serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/Constants.java > 1172168 > trunk/serde/src/gen/thrift/gen-php/serde/serde_constants.php 1172168 > trunk/serde/src/gen/thrift/gen-py/org_apache_hadoop_hive_serde/constants.py > 1172168 > trunk/serde/src/gen/thrift/gen-rb/serde_constants.rb 1172168 > trunk/serde/src/java/org/apache/hadoop/hive/serde2/SerDeUtils.java 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/binarysortable/BinarySortableSerDe.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarSerDe.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarStruct.java > 1172168 > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyBinary.java > PRE-CREATION > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyFactory.java > 1172168 > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyUtils.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyBinaryObjectInspector.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyPrimitiveObjectInspectorFactory.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryBinary.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryFactory.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinarySerDe.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryUtils.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/PrimitiveObjectInspector.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/BinaryObjectInspector.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/JavaBinaryObjectInspector.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorConverter.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorFactory.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java > 1172168 > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/SettableBinaryObjectInspector.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableBinaryObjectInspector.java > PRE-CREATION > > trunk/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java > 1172168 > trunk/serde/src/test/org/apache/hadoop/hive/serde2/TestStatsSerde.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/binarysortable/MyTestClass.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/binarysortable/TestBinarySortableSerDe.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/columnar/TestLazyBinaryColumnarSerDe.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazy/TestLazyPrimitive.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazy/TestLazySimpleSerDe.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazybinary/MyTestClassBigger.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazybinary/TestLazyBinarySerDe.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java > 1172168 > > trunk/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestStandardObjectInspectors.java > 1172168 > > Diff: https://reviews.apache.org/r/1586/diff > > > Testing > ------- > > Added and updated unit tests. > Added new system tests. > > > Thanks, > > Ashutosh > >
