You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/04/27 13:05:09 UTC

[GitHub] [flink] lirui-apache commented on a change in pull request #11876: [FLINK-17334] HIVE UDF BUGFIX

lirui-apache commented on a change in pull request #11876:
URL: https://github.com/apache/flink/pull/11876#discussion_r415742394



##########
File path: flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/functions/hive/HiveSimpleUDFTest.java
##########
@@ -49,6 +50,30 @@
 public class HiveSimpleUDFTest {
 	private static HiveShim hiveShim = HiveShimLoader.loadHiveShim(HiveShimLoader.getHiveVersion());
 
+	@Test
+	public void testBooleanUDF() {
+		HiveSimpleUDF udf = init(BooleanUDF.class, new DataType[]{ DataTypes.INT()});
+		assertTrue((boolean) udf.eval(1));
+	}
+
+	@Test
+	public void testFloatUDF() {
+		HiveSimpleUDF udf = init(FloatUDF.class, new DataType[]{ DataTypes.FLOAT()});
+		assertEquals((float) udf.eval(3.0f), 3.0f, 1.0);

Review comment:
       Why we allow a 1.0 delta here? And I think `3.0f` should be the expected value and `udf.eval(3.0f)` should be the actual value.

##########
File path: flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/functions/hive/conversion/HiveInspectors.java
##########
@@ -354,25 +354,25 @@ public static ObjectInspector getObjectInspector(HiveShim hiveShim, Class clazz)
 		if (clazz.equals(String.class) || clazz.equals(Text.class)) {
 
 			typeInfo = TypeInfoFactory.stringTypeInfo;
-		} else if (clazz.equals(Boolean.class) || clazz.equals(BooleanWritable.class)) {
+		} else if (clazz.equals(boolean.class) || clazz.equals(Boolean.class) || clazz.equals(BooleanWritable.class)) {

Review comment:
       Why do we still need these changes? Actually I think this method is no longer needed

##########
File path: flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/functions/hive/HiveSimpleUDFTest.java
##########
@@ -230,4 +255,64 @@ protected static HiveSimpleUDF init(Class hiveUdfClass, DataType[] argTypes) {
 
 		return udf;
 	}
+
+	/**
+	 * Boolean Test UDF.
+	 */
+	public static class BooleanUDF extends UDF {
+		/**
+		 *
+		 * @param content input
+		 * @return
+		 */
+		public boolean evaluate(int content) {
+			if (content == 1) {
+				return true;
+			} else {
+				return false;
+			}
+		}
+	}
+
+	/**
+	 * Float Test UDF.
+	 */
+	public static class FloatUDF extends UDF {
+		/**
+		 *
+		 * @param content input
+		 * @return
+		 */

Review comment:
       Please remove these java docs since they're meaningless.

##########
File path: flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/functions/hive/HiveSimpleUDFTest.java
##########
@@ -230,4 +255,64 @@ protected static HiveSimpleUDF init(Class hiveUdfClass, DataType[] argTypes) {
 
 		return udf;
 	}
+
+	/**
+	 * Boolean Test UDF.
+	 */
+	public static class BooleanUDF extends UDF {
+		/**
+		 *
+		 * @param content input
+		 * @return
+		 */
+		public boolean evaluate(int content) {
+			if (content == 1) {
+				return true;
+			} else {
+				return false;
+			}

Review comment:
       just return content == 1




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org