okumin commented on code in PR #4653:
URL: https://github.com/apache/hive/pull/4653#discussion_r1413142929
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/AbstractGenericUDFArrayBase.java:
##########
@@ -123,4 +127,55 @@ ObjectInspector initListOI(ObjectInspector[] arguments) {
return
ObjectInspectorFactory.getStandardListObjectInspector(initOI(arguments));
}
+ void checkValueAndListElementTypes(ObjectInspector arrayElementOI, String
functionName, ObjectInspector valueOI,
+ int elementIndex) throws UDFArgumentTypeException {
+ // Check if list element and value are of same type
+ if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) {
+ if
(arrayElementOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
arrayElementOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ if
(valueOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
valueOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ return;
+ }
+ }
Review Comment:
I'm curious why we need to make string and varchar comparable here. I
understand some types are comparable.
```
0: jdbc:hive2://hive-hiveserver2:10000/defaul> select cast('abc' as string)
== cast('abcd' as varchar(3));
+-------+
| _c0 |
+-------+
| true |
+-------+
0: jdbc:hive2://hive-hiveserver2:10000/defaul> select cast(2 as int) =
cast(2 as bigint);
+-------+
| _c0 |
+-------+
| true |
+-------+
```
Is there a case where the string <-> varchar comparison gives us convenience?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/AbstractGenericUDFArrayBase.java:
##########
@@ -123,4 +127,55 @@ ObjectInspector initListOI(ObjectInspector[] arguments) {
return
ObjectInspectorFactory.getStandardListObjectInspector(initOI(arguments));
}
+ void checkValueAndListElementTypes(ObjectInspector arrayElementOI, String
functionName, ObjectInspector valueOI,
+ int elementIndex) throws UDFArgumentTypeException {
+ // Check if list element and value are of same type
+ if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) {
+ if
(arrayElementOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
arrayElementOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ if
(valueOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
valueOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ return;
+ }
+ }
+ throw new UDFArgumentTypeException(elementIndex,
+ String.format("%s type element is expected at function
%s(array<%s>,%s), but %s is found",
+ arrayElementOI.getTypeName(), functionName,
arrayElementOI.getTypeName(),
+ arrayElementOI.getTypeName(), valueOI.getTypeName()));
+ }
+ }
+
+ boolean isConversionSupported(ObjectInspector oi1, ObjectInspector oi2) {
+ if (oi1.getCategory() == oi2.getCategory()) {
+ if (oi1.getCategory() == ObjectInspector.Category.PRIMITIVE) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector)
oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector)
oi2);
+ return
(poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING))
|| (
+
poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR));
+ }
+ }
+ return false;
+ }
+
+ int compareConvertibleElements(ObjectInspector oi1, Object o1,
ObjectInspector oi2, Object o2) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector) oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector) oi2);
+ if (String.valueOf(poi1.getPrimitiveWritableObject(o1))
+ .equals(String.valueOf(poi2.getPrimitiveWritableObject(o2)))) {
+ return 0;
+ } else {
+ return -1;
+ }
+ }
+
+ int compareElements(Object value, ObjectInspector valueOI, Object
listElement) {
+ if ((listElement != null) && (ObjectInspectorUtils.compare(value,
valueOI, listElement, arrayElementOI) == 0
+ || (isConversionSupported(valueOI, arrayElementOI)
+ && compareConvertibleElements(valueOI, value, arrayElementOI,
listElement) == 0))) {
+ return 0;
+ }
+ return -1;
Review Comment:
I think we should follow the convention of `Comparator#compare` if we return
an integer here. If we really need only -1 and 0, the type of the return value
may be boolean.
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/AbstractGenericUDFArrayBase.java:
##########
@@ -123,4 +127,55 @@ ObjectInspector initListOI(ObjectInspector[] arguments) {
return
ObjectInspectorFactory.getStandardListObjectInspector(initOI(arguments));
}
+ void checkValueAndListElementTypes(ObjectInspector arrayElementOI, String
functionName, ObjectInspector valueOI,
+ int elementIndex) throws UDFArgumentTypeException {
+ // Check if list element and value are of same type
+ if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) {
+ if
(arrayElementOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
arrayElementOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ if
(valueOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
valueOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ return;
+ }
+ }
+ throw new UDFArgumentTypeException(elementIndex,
+ String.format("%s type element is expected at function
%s(array<%s>,%s), but %s is found",
+ arrayElementOI.getTypeName(), functionName,
arrayElementOI.getTypeName(),
+ arrayElementOI.getTypeName(), valueOI.getTypeName()));
+ }
+ }
+
+ boolean isConversionSupported(ObjectInspector oi1, ObjectInspector oi2) {
+ if (oi1.getCategory() == oi2.getCategory()) {
+ if (oi1.getCategory() == ObjectInspector.Category.PRIMITIVE) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector)
oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector)
oi2);
+ return
(poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING))
|| (
+
poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR));
+ }
+ }
+ return false;
+ }
+
+ int compareConvertibleElements(ObjectInspector oi1, Object o1,
ObjectInspector oi2, Object o2) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector) oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector) oi2);
+ if (String.valueOf(poi1.getPrimitiveWritableObject(o1))
+ .equals(String.valueOf(poi2.getPrimitiveWritableObject(o2)))) {
Review Comment:
I guess this should be private since this is applicable only when o1 and o2
are either string or varchar.
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/AbstractGenericUDFArrayBase.java:
##########
@@ -123,4 +127,55 @@ ObjectInspector initListOI(ObjectInspector[] arguments) {
return
ObjectInspectorFactory.getStandardListObjectInspector(initOI(arguments));
}
+ void checkValueAndListElementTypes(ObjectInspector arrayElementOI, String
functionName, ObjectInspector valueOI,
+ int elementIndex) throws UDFArgumentTypeException {
+ // Check if list element and value are of same type
+ if (!ObjectInspectorUtils.compareTypes(arrayElementOI, valueOI)) {
+ if
(arrayElementOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
arrayElementOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ if
(valueOI.getTypeName().contains(serdeConstants.VARCHAR_TYPE_NAME) ||
valueOI.getTypeName()
+ .equals(serdeConstants.STRING_TYPE_NAME)) {
+ return;
+ }
+ }
+ throw new UDFArgumentTypeException(elementIndex,
+ String.format("%s type element is expected at function
%s(array<%s>,%s), but %s is found",
+ arrayElementOI.getTypeName(), functionName,
arrayElementOI.getTypeName(),
+ arrayElementOI.getTypeName(), valueOI.getTypeName()));
+ }
+ }
+
+ boolean isConversionSupported(ObjectInspector oi1, ObjectInspector oi2) {
+ if (oi1.getCategory() == oi2.getCategory()) {
+ if (oi1.getCategory() == ObjectInspector.Category.PRIMITIVE) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector)
oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector)
oi2);
+ return
(poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING))
|| (
+
poi1.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.STRING)
+ &&
poi2.getPrimitiveCategory().equals(PrimitiveObjectInspector.PrimitiveCategory.VARCHAR));
+ }
+ }
+ return false;
+ }
+
+ int compareConvertibleElements(ObjectInspector oi1, Object o1,
ObjectInspector oi2, Object o2) {
+ PrimitiveObjectInspector poi1 = ((PrimitiveObjectInspector) oi1);
+ PrimitiveObjectInspector poi2 = ((PrimitiveObjectInspector) oi2);
+ if (String.valueOf(poi1.getPrimitiveWritableObject(o1))
+ .equals(String.valueOf(poi2.getPrimitiveWritableObject(o2)))) {
Review Comment:
I may guess it is more robust to use a Converter. I think this is OK if we
only support STRING <-> VARCHAR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
