[ https://issues.apache.org/jira/browse/HIVE-2380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13118224#comment-13118224 ]
jirapos...@reviews.apache.org commented on HIVE-2380: ----------------------------------------------------- bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 675 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41673#file41673line675> bq. > bq. > I thought we decided an explicit cast would be required even for string<-->binary? Correct. Accidentally left. Will remove it in the new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFConcat.java, line 36 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41678#file41678line36> bq. > bq. > @Description annotation for this class needs to be updated. Will update in new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFLength.java, line 55 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41679#file41679line55> bq. > bq. > @Description annotation for this class needs to be updated. Will update in new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java, line 29 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41682#file41682line29> bq. > bq. > This UDF class needs an @Description annotation. Will update in new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFSubstr.java, line 43 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41680#file41680line43> bq. > bq. > @Description annotation for this class needs to be updated. Will update in new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java, line 67 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41682#file41682line67> bq. > bq. > The CAST should have parentheses, so bq. > bq. > CAST(x AS BINARY) bq. > bq. > (this applies to the existing code in GenericUDFTimestamp too) bq. > Will update in new patch. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/ql/src/test/queries/clientpositive/ba_table1.q, line 9 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41683#file41683line9> bq. > bq. > 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. bq. > bq. > (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. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/serde/src/java/org/apache/hadoop/hive/serde2/SerDeUtils.java, line 267 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41696#file41696line267> bq. > bq. > 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. bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarSerDe.java, line 83 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41698#file41698line83> bq. > bq. > 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." bq. On 2011-09-27 00:43:03, John Sichi wrote: bq. > trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java, line 393 bq. > <https://reviews.apache.org/r/1586/diff/2/?file=41716#file41716line393> bq. > bq. > 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: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1586/ bq. ----------------------------------------------------------- bq. bq. (Updated 2011-09-18 05:52:41) bq. bq. bq. Review request for hive. bq. bq. bq. Summary bq. ------- bq. bq. This patch adds bytearray as a new datatype in Hive. bq. bq. bq. This addresses bug HIVE-2380. bq. https://issues.apache.org/jira/browse/HIVE-2380 bq. bq. bq. Diffs bq. ----- bq. bq. trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFConcat.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFLength.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFSubstr.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToString.java 1172168 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToBinary.java PRE-CREATION bq. trunk/ql/src/test/queries/clientpositive/ba_table1.q PRE-CREATION bq. trunk/ql/src/test/queries/clientpositive/ba_table2.q PRE-CREATION bq. trunk/ql/src/test/queries/clientpositive/ba_table_udfs.q PRE-CREATION bq. trunk/ql/src/test/results/clientpositive/ba_table1.q.out PRE-CREATION bq. trunk/ql/src/test/results/clientpositive/ba_table2.q.out PRE-CREATION bq. trunk/ql/src/test/results/clientpositive/ba_table_udfs.q.out PRE-CREATION bq. trunk/serde/if/serde.thrift 1172168 bq. trunk/serde/src/gen/thrift/gen-cpp/serde_constants.h 1172168 bq. trunk/serde/src/gen/thrift/gen-cpp/serde_constants.cpp 1172168 bq. trunk/serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/Constants.java 1172168 bq. trunk/serde/src/gen/thrift/gen-php/serde/serde_constants.php 1172168 bq. trunk/serde/src/gen/thrift/gen-py/org_apache_hadoop_hive_serde/constants.py 1172168 bq. trunk/serde/src/gen/thrift/gen-rb/serde_constants.rb 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/SerDeUtils.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/binarysortable/BinarySortableSerDe.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarSerDe.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/columnar/LazyBinaryColumnarStruct.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyBinary.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyFactory.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyUtils.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyBinaryObjectInspector.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyPrimitiveObjectInspectorFactory.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryBinary.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryFactory.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinarySerDe.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/lazybinary/LazyBinaryUtils.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/PrimitiveObjectInspector.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/BinaryObjectInspector.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/JavaBinaryObjectInspector.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorConverter.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorFactory.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java 1172168 bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/SettableBinaryObjectInspector.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableBinaryObjectInspector.java PRE-CREATION bq. trunk/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/TestStatsSerde.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/binarysortable/MyTestClass.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/binarysortable/TestBinarySortableSerDe.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/columnar/TestLazyBinaryColumnarSerDe.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazy/TestLazyPrimitive.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazy/TestLazySimpleSerDe.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazybinary/MyTestClassBigger.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/lazybinary/TestLazyBinarySerDe.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java 1172168 bq. trunk/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestStandardObjectInspectors.java 1172168 bq. bq. Diff: https://reviews.apache.org/r/1586/diff bq. bq. bq. Testing bq. ------- bq. bq. Added and updated unit tests. bq. Added new system tests. bq. bq. bq. Thanks, bq. bq. Ashutosh bq. bq. > Add ByteArray Datatype > ---------------------- > > Key: HIVE-2380 > URL: https://issues.apache.org/jira/browse/HIVE-2380 > Project: Hive > Issue Type: New Feature > Components: Serializers/Deserializers > Reporter: Ashutosh Chauhan > Assignee: Ashutosh Chauhan > Attachments: hive-2380.patch, hive-2380_1.patch > > > Add bytearray as a primitive data type. -- 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