From 606473e7799ede9035b6f62689040ba23a8403fc Mon Sep 17 00:00:00 2001 From: jiefli_LinkedIn Date: Mon, 12 Aug 2024 15:23:53 -0400 Subject: [PATCH] Address comments --- .../VersionedSqlUserDefinedFunction.java | 20 ------------------- .../hive/hive2rel/HiveToRelConverterTest.java | 2 +- .../coral/hive/hive2rel/TestUtils.java | 6 +++--- .../transformers/HiveUDFTransformer.java | 8 ++++---- .../linkedin/coral/spark/CoralSparkTest.java | 8 ++++---- .../com/linkedin/coral/spark/TestUtils.java | 8 ++++---- .../rel2trino/HiveToTrinoConverterTest.java | 4 ++-- .../coral/trino/rel2trino/TestUtils.java | 8 ++++---- 8 files changed, 22 insertions(+), 42 deletions(-) diff --git a/coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/functions/VersionedSqlUserDefinedFunction.java b/coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/functions/VersionedSqlUserDefinedFunction.java index d876ff892..e3fac8596 100644 --- a/coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/functions/VersionedSqlUserDefinedFunction.java +++ b/coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/functions/VersionedSqlUserDefinedFunction.java @@ -6,8 +6,6 @@ package com.linkedin.coral.hive.hive2rel.functions; import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import com.google.common.collect.ImmutableList; @@ -72,24 +70,6 @@ public String getViewDependentFunctionName() { return viewDependentFunctionName; } - /** - * Get the versioned function name based on the `viewDependentFunctionName` and UDF class name. - * For example, if the function name is "myFunction" and the class name is "coral_udf_version_1_0_0.com.linkedin.MyClass", - * the versioned function name will be "myFunction_1_0_0". If the class name is not versioned, such as "com.linkedin.MyClass", - * the versioned function name will be "myFunction". - */ - public String getVersionedViewDependentFunctionName() { - String className = getName(); - String versionedPrefix = className.substring(0, className.indexOf('.')); - Matcher matcher = Pattern.compile(CORAL_VERSIONED_UDF_PREFIX).matcher(versionedPrefix); - if (matcher.find()) { - return String.join("_", - ImmutableList.of(viewDependentFunctionName, matcher.group(1), matcher.group(2), matcher.group(3))); - } else { - return viewDependentFunctionName; - } - } - // This method is called during SQL validation. The super-class implementation resets the call's sqlOperator to one // that is looked up from the StaticHiveFunctionRegistry or inferred dynamically if it's a Dali UDF. Since UDFs in the StaticHiveFunctionRegistry are not // versioned, this method overrides the super-class implementation to properly restore the call's operator as diff --git a/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/HiveToRelConverterTest.java b/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/HiveToRelConverterTest.java index 31b0069d1..dd2837f42 100644 --- a/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/HiveToRelConverterTest.java +++ b/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/HiveToRelConverterTest.java @@ -357,7 +357,7 @@ public void testDaliUDFCall() { } @Test - public void testUDFWithShadedClassName() { + public void testUDFWithVersioningClassName() { RelNode rel = converter.convertView("test", "tableOneViewShadePrefixUDF"); String expectedPlan = "LogicalProject(EXPR$0=[coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF($0)])\n" diff --git a/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/TestUtils.java b/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/TestUtils.java index 2b1550dd0..32a4572a7 100644 --- a/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/TestUtils.java +++ b/coral-hive/src/test/java/com/linkedin/coral/hive/hive2rel/TestUtils.java @@ -195,9 +195,9 @@ public static TestHive setupDefaultHive(HiveConf conf) throws IOException { } driver.run( - "create function lessThanHundred_with_shade_prefix as 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); + "create function lessThanHundred_with_versioning_prefix as 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); response = driver.run( - "CREATE VIEW IF NOT EXISTS test.tableOneViewShadePrefixUDF as SELECT lessThanHundred_with_shade_prefix(a) from test.tableOne"); + "CREATE VIEW IF NOT EXISTS test.tableOneViewShadePrefixUDF as SELECT lessThanHundred_with_versioning_prefix(a) from test.tableOne"); if (response.getResponseCode() != 0) { throw new RuntimeException("Failed to setup view"); } @@ -243,7 +243,7 @@ public static TestHive setupDefaultHive(HiveConf conf) throws IOException { Table tableOneViewLateralUDTF = msc.getTable("test", "tableOneViewLateralUDTF"); setOrUpdateDaliFunction(tableOneViewLateralUDTF, "CountOfRow", "com.linkedin.coral.hive.hive2rel.CoralTestUDTF"); Table tableOneViewShadePrefixUDF = msc.getTable("test", "tableOneViewShadePrefixUDF"); - setOrUpdateDaliFunction(tableOneViewShadePrefixUDF, "lessThanHundred_with_shade_prefix", + setOrUpdateDaliFunction(tableOneViewShadePrefixUDF, "lessThanHundred_with_versioning_prefix", "coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF"); msc.alter_table("test", "tableOneView", tableOneView); msc.alter_table("test", "tableOneViewLateralUDTF", tableOneViewLateralUDTF); diff --git a/coral-spark/src/main/java/com/linkedin/coral/spark/transformers/HiveUDFTransformer.java b/coral-spark/src/main/java/com/linkedin/coral/spark/transformers/HiveUDFTransformer.java index 5fa3b3413..5bcd76030 100644 --- a/coral-spark/src/main/java/com/linkedin/coral/spark/transformers/HiveUDFTransformer.java +++ b/coral-spark/src/main/java/com/linkedin/coral/spark/transformers/HiveUDFTransformer.java @@ -65,16 +65,16 @@ protected SqlCall transform(SqlCall sqlCall) { if (UNSUPPORTED_HIVE_UDFS.contains(operatorName)) { throw new UnsupportedUDFException(operatorName); } - final String versionedViewDependentFunctionName = operator.getVersionedViewDependentFunctionName(); + final String viewDependentFunctionName = operator.getViewDependentFunctionName(); final List dependencies = operator.getIvyDependencies(); List listOfUris = dependencies.stream().map(URI::create).collect(Collectors.toList()); LOG.info("Function: {} is not a Builtin UDF or Transport UDF. We fall back to its Hive " + "function with ivy dependency: {}", operatorName, String.join(",", dependencies)); - final SparkUDFInfo sparkUDFInfo = new SparkUDFInfo(operatorName, versionedViewDependentFunctionName, listOfUris, - SparkUDFInfo.UDFTYPE.HIVE_CUSTOM_UDF); + final SparkUDFInfo sparkUDFInfo = + new SparkUDFInfo(operatorName, viewDependentFunctionName, listOfUris, SparkUDFInfo.UDFTYPE.HIVE_CUSTOM_UDF); sparkUDFInfos.add(sparkUDFInfo); final SqlOperator convertedFunction = - createSqlOperator(versionedViewDependentFunctionName, operator.getReturnTypeInference()); + createSqlOperator(viewDependentFunctionName, operator.getReturnTypeInference()); return convertedFunction.createCall(sqlCall.getParserPosition(), sqlCall.getOperandList()); } } diff --git a/coral-spark/src/test/java/com/linkedin/coral/spark/CoralSparkTest.java b/coral-spark/src/test/java/com/linkedin/coral/spark/CoralSparkTest.java index ca4328cd1..40e4a6cf2 100644 --- a/coral-spark/src/test/java/com/linkedin/coral/spark/CoralSparkTest.java +++ b/coral-spark/src/test/java/com/linkedin/coral/spark/CoralSparkTest.java @@ -165,11 +165,11 @@ public void testHiveUDFTransformer() { } @Test - public void testUDFWithShadedClassName() { + public void testUDFWithVersioningClassName() { // After registering unversioned UDF "com.linkedin.coral.hive.hive2rel.CoralTestUDF", // Coral should be able to translate the view with the corresponding versioned UDF class // "coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF" - RelNode relNode = TestUtils.toRelNode("default", "foo_udf_with_shade_prefix"); + RelNode relNode = TestUtils.toRelNode("default", "foo_udf_with_versioning_prefix"); CoralSpark coralSpark = createCoralSpark(relNode); List udfJars = coralSpark.getSparkUDFInfoList(); @@ -180,7 +180,7 @@ public void testUDFWithShadedClassName() { assertEquals(udfClassName, targetClassName); String udfFunctionName = udfJars.get(0).getFunctionName(); - String targetFunctionName = "LessThanHundred_shade_prefix_0_1_x"; + String targetFunctionName = "LessThanHundred_versioning_prefix_0_1_x"; assertEquals(udfFunctionName, targetFunctionName); List listOfUriStrings = convertToListOfUriStrings(udfJars.get(0).getArtifactoryUrls()); @@ -192,7 +192,7 @@ public void testUDFWithShadedClassName() { assertEquals(testUdfType, targetUdfType); String sparkSqlStmt = coralSpark.getSparkSql(); - String targetSqlStmt = "SELECT LessThanHundred_shade_prefix_0_1_x(foo.a)\n" + "FROM default.foo foo"; + String targetSqlStmt = "SELECT LessThanHundred_versioning_prefix_0_1_x(foo.a)\n" + "FROM default.foo foo"; assertEquals(sparkSqlStmt, targetSqlStmt); } diff --git a/coral-spark/src/test/java/com/linkedin/coral/spark/TestUtils.java b/coral-spark/src/test/java/com/linkedin/coral/spark/TestUtils.java index 911decb57..585fdfb80 100644 --- a/coral-spark/src/test/java/com/linkedin/coral/spark/TestUtils.java +++ b/coral-spark/src/test/java/com/linkedin/coral/spark/TestUtils.java @@ -84,7 +84,7 @@ public static void initializeViews(HiveConf conf) throws HiveException, MetaExce run(driver, "CREATE FUNCTION LessThanHundred as 'com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); run(driver, - "CREATE FUNCTION LessThanHundred_shade_prefix as 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); + "CREATE FUNCTION LessThanHundred_versioning_prefix_0_1_x as 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); run(driver, String.join("\n", "", "CREATE VIEW IF NOT EXISTS foo_view", "AS", "SELECT b AS bcol, sum(c) AS sum_c", "FROM foo", "GROUP BY b")); @@ -239,10 +239,10 @@ public static void initializeViews(HiveConf conf) throws HiveException, MetaExce "SELECT default_foo_duplicate_udf_LessThanHundred(a), default_foo_duplicate_udf_LessThanHundred(a)", "FROM foo")); - run(driver, String.join("\n", "", "CREATE VIEW IF NOT EXISTS foo_udf_with_shade_prefix", - "tblproperties('functions' = 'LessThanHundred_shade_prefix:coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF',", + run(driver, String.join("\n", "", "CREATE VIEW IF NOT EXISTS foo_udf_with_versioning_prefix", + "tblproperties('functions' = 'LessThanHundred_versioning_prefix_0_1_x:coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF',", " 'dependencies' = 'ivy://com.linkedin:udf-shaded:1.0')", "AS", - "SELECT LessThanHundred_shade_prefix(a)", "FROM foo")); + "SELECT LessThanHundred_versioning_prefix_0_1_x(a)", "FROM foo")); } private static void executeCreateTableQuery(Driver driver, String dbName, String tableName, String schema) { diff --git a/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/HiveToTrinoConverterTest.java b/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/HiveToTrinoConverterTest.java index 1a6f8a924..73b7de248 100644 --- a/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/HiveToTrinoConverterTest.java +++ b/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/HiveToTrinoConverterTest.java @@ -1003,8 +1003,8 @@ public void testSqlSelectAliasAppenderTransformerWithoutTableAliasPrefix() { } @Test - public void testUDFWithShadedClassName() { - RelNode relNode = TestUtils.getHiveToRelConverter().convertView("test", "udf_with_shade_prefix"); + public void testUDFWithVersioningClassName() { + RelNode relNode = TestUtils.getHiveToRelConverter().convertView("test", "udf_with_versioning_prefix"); RelToTrinoConverter relToTrinoConverter = TestUtils.getRelToTrinoConverter(); String expandedSql = relToTrinoConverter.convert(relNode); diff --git a/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/TestUtils.java b/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/TestUtils.java index 0486af9d8..2bb368e13 100644 --- a/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/TestUtils.java +++ b/coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/TestUtils.java @@ -414,12 +414,12 @@ public static void initializeTablesAndViews(HiveConf conf) throws HiveException, + "SELECT a_tinyint, a_smallint, a_integer, a_bigint, a_float FROM test.table_with_mixed_columns"); run(driver, - "CREATE FUNCTION LessThanHundred_shade_prefix AS 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); + "CREATE FUNCTION LessThanHundred_versioning_prefix AS 'coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF'"); - run(driver, String.join("\n", "CREATE VIEW IF NOT EXISTS test.udf_with_shade_prefix", - "tblproperties('functions' = 'LessThanHundred_shade_prefix:coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF',", + run(driver, String.join("\n", "CREATE VIEW IF NOT EXISTS test.udf_with_versioning_prefix", + "tblproperties('functions' = 'LessThanHundred_versioning_prefix:coral_udf_version_0_1_x.com.linkedin.coral.hive.hive2rel.CoralTestUDF',", " 'dependencies' = 'ivy://com.linkedin:udf-shaded:1.0')", "AS", - "SELECT LessThanHundred_shade_prefix(a)", "FROM test.tableA")); + "SELECT LessThanHundred_versioning_prefix(a)", "FROM test.tableA")); // Tables used in RelToTrinoConverterTest run(driver,