Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ljfgem committed Aug 26, 2024
1 parent d3d27d8 commit aafd59d
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* Class to resolve hive function names in SQL to Function.
*/
public class HiveFunctionResolver {
private static final String CORAL_VERSIONED_UDF_PREFIX = "coral_udf_version_(\\d+|x)_(\\d+|x)_(\\d+|x)";
private static final String CLASS_NAME_PREFIX = "coral_udf_version_(\\d+|x)_(\\d+|x)_(\\d+|x)";

public final FunctionRegistry registry;
private final ConcurrentHashMap<String, Function> dynamicFunctionRegistry;
Expand Down Expand Up @@ -246,14 +246,14 @@ public void addDynamicFunctionToTheRegistry(String funcClassName, Function funct
/**
* Removes the versioning prefix from a given UDF class name if it is present.
* A class name is considered versioned if the prefix before the first dot
* follows {@link HiveFunctionResolver#CORAL_VERSIONED_UDF_PREFIX} format
* follows {@link HiveFunctionResolver#CLASS_NAME_PREFIX} format
*/
private String removeVersioningPrefix(String className) {
if (className != null && !className.isEmpty()) {
int firstDotIndex = className.indexOf('.');
if (firstDotIndex != -1) {
String prefix = className.substring(0, firstDotIndex);
if (prefix.matches(CORAL_VERSIONED_UDF_PREFIX)) {
if (prefix.matches(CLASS_NAME_PREFIX)) {
return className.substring(firstDotIndex + 1);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,25 @@ public class VersionedSqlUserDefinedFunction extends SqlUserDefinedFunction {
private final String viewDependentFunctionName;

// The UDF class name value defined in the "functions" property of the view.
// i.e. "functions = <viewDependentFunctionName> : <udfClassName>"
private final String udfClassName;
// i.e. "functions = <viewDependentFunctionName> : <funcClassName>"
private final String funcClassName;

private VersionedSqlUserDefinedFunction(SqlIdentifier opName, SqlReturnTypeInference returnTypeInference,
SqlOperandTypeInference operandTypeInference, SqlOperandTypeChecker operandTypeChecker,
List<RelDataType> paramTypes, Function function, List<String> ivyDependencies, String viewDependentFunctionName,
String udfClassName) {
String funcClassName) {
super(opName, returnTypeInference, operandTypeInference, operandTypeChecker, paramTypes, function,
SqlFunctionCategory.USER_DEFINED_FUNCTION);
this.ivyDependencies = ivyDependencies;
this.viewDependentFunctionName = viewDependentFunctionName;
this.udfClassName = udfClassName;
this.funcClassName = funcClassName;
}

public VersionedSqlUserDefinedFunction(SqlUserDefinedFunction sqlUdf, List<String> ivyDependencies,
String viewDependentFunctionName, String udfClassName) {
String viewDependentFunctionName, String funcClassName) {
this(new SqlIdentifier(ImmutableList.of(sqlUdf.getName()), SqlParserPos.ZERO), sqlUdf.getReturnTypeInference(),
null, sqlUdf.getOperandTypeChecker(), sqlUdf.getParamTypes(), sqlUdf.getFunction(), ivyDependencies,
viewDependentFunctionName, udfClassName);
viewDependentFunctionName, funcClassName);
}

public List<String> getIvyDependencies() {
Expand All @@ -95,6 +95,8 @@ public String getViewDependentFunctionName() {
* the short function name.
*/
public String getShortFunctionName() {
// getName() returns the unversioned function class, which we use to identify the type inference.
// It's just a convention and other naming approaches are valid as long as they identify the type inference.
String unversionedClassName = getName();
if (SHORT_FUNC_NAME_MAP.containsKey(unversionedClassName)) {
return SHORT_FUNC_NAME_MAP.get(unversionedClassName);
Expand All @@ -104,8 +106,8 @@ public String getShortFunctionName() {
return caseConverter.convert(nameSplit[nameSplit.length - 1]);
}

public String getUDFClassName() {
return udfClassName;
public String getFuncClassName() {
return funcClassName;
}

// This method is called during SQL validation. The super-class implementation resets the call's sqlOperator to one
Expand All @@ -117,7 +119,7 @@ public String getUDFClassName() {
public RelDataType deriveType(SqlValidator validator, SqlValidatorScope scope, SqlCall call) {
RelDataType relDataType = super.deriveType(validator, scope, call);
((SqlBasicCall) call).setOperator(new VersionedSqlUserDefinedFunction((SqlUserDefinedFunction) (call.getOperator()),
ivyDependencies, viewDependentFunctionName, udfClassName));
ivyDependencies, viewDependentFunctionName, funcClassName));
return relDataType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public void testDaliUDFCall() {
}

@Test
public void testUDFWithVersioningClassName() {
public void testVersioningUDF() {
RelNode rel = converter.convertView("test", "tableOneViewShadePrefixUDF");
String expectedPlan = "LogicalProject(EXPR$0=[com.linkedin.coral.hive.hive2rel.CoralTestUDF($0)])\n"
+ " LogicalTableScan(table=[[hive, test, tableone]])\n";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected SqlCall transform(SqlCall sqlCall) {
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(operator.getUDFClassName(), viewDependentFunctionName,
final SparkUDFInfo sparkUDFInfo = new SparkUDFInfo(operator.getFuncClassName(), viewDependentFunctionName,
listOfUris, SparkUDFInfo.UDFTYPE.HIVE_CUSTOM_UDF);
sparkUDFInfos.add(sparkUDFInfo);
final SqlOperator convertedFunction =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public void testHiveUDFTransformer() {
}

@Test
public void testUDFWithVersioningClassName() {
public void testVersioningUDF() {
// 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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ public void testSqlSelectAliasAppenderTransformerWithoutTableAliasPrefix() {
}

@Test
public void testUDFWithVersioningClassName() {
public void testVersioningUDF() {
RelNode relNode = TestUtils.getHiveToRelConverter().convertView("test", "udf_with_versioning_prefix");
RelToTrinoConverter relToTrinoConverter = TestUtils.getRelToTrinoConverter();
String expandedSql = relToTrinoConverter.convert(relNode);
Expand Down

0 comments on commit aafd59d

Please sign in to comment.