Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ljfgem committed Aug 12, 2024
1 parent 944ec2b commit 606473e
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> dependencies = operator.getIvyDependencies();
List<URI> 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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<SparkUDFInfo> udfJars = coralSpark.getSparkUDFInfoList();

Expand All @@ -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<String> listOfUriStrings = convertToListOfUriStrings(udfJars.get(0).getArtifactoryUrls());
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 606473e

Please sign in to comment.