From dd545a1f0a35fc68136ed366c4febcaaa92d6051 Mon Sep 17 00:00:00 2001 From: "zor@zurich.ibm.com" Date: Wed, 17 Dec 2025 22:55:43 +0100 Subject: [PATCH 1/2] feat(isthmus): configurable fallback to dynamic function mapping --- isthmus/build.gradle.kts | 1 + .../io/substrait/isthmus/FeatureBoard.java | 53 +++++ .../isthmus/SimpleExtensionToSqlOperator.java | 40 +++- .../isthmus/expression/FunctionConverter.java | 55 ++++- .../IgnoreNullableAndParameters.java | 39 +++- ...oFallbackDynamicFunctionRoundtripTest.java | 194 ++++++++++++++++++ 6 files changed, 372 insertions(+), 10 deletions(-) create mode 100644 isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java create mode 100644 isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java diff --git a/isthmus/build.gradle.kts b/isthmus/build.gradle.kts index ad6ce6230..37a5d8fd1 100644 --- a/isthmus/build.gradle.kts +++ b/isthmus/build.gradle.kts @@ -105,6 +105,7 @@ dependencies { testImplementation(platform(libs.junit.bom)) testImplementation(libs.junit.jupiter) testRuntimeOnly(libs.junit.platform.launcher) + testRuntimeOnly(libs.slf4j.jdk14) implementation(libs.guava) implementation(libs.protobuf.java.util) { exclude("com.google.guava", "guava") diff --git a/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java b/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java new file mode 100644 index 000000000..c303ebbfb --- /dev/null +++ b/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java @@ -0,0 +1,53 @@ +package io.substrait.isthmus; + +import org.apache.calcite.avatica.util.Casing; +import org.immutables.value.Value; + +/** + * A feature board is a collection of flags that are enabled or configurations that control the + * handling of a request to convert query [batch] to Substrait plans. + */ +@Value.Immutable +public abstract class FeatureBoard { + + /** + * @return Calcite's identifier casing policy for unquoted identifiers. + */ + @Value.Default + public Casing unquotedCasing() { + return Casing.TO_UPPER; + } + + /** + * Controls whether to support dynamic user-defined functions (UDFs) during SQL to Substrait plan + * conversion. + * + *

When enabled, custom functions defined in extension YAML files are available for use in SQL + * queries. These functions will be dynamically converted to SQL operators during plan conversion. + * This feature must be explicitly enabled by users and is disabled by default. + * + * @return true if dynamic UDFs should be supported; false otherwise (default) + */ + @Value.Default + public boolean allowDynamicUdfs() { + return false; + } + + /** + * Controls whether to automatically create mappings for all unmapped functions using + * SimpleExtensionToSqlOperator. + * + *

When enabled, functions from extension YAML files that are not explicitly mapped in + * FunctionMappings will be automatically mapped to Calcite SqlOperators. This allows custom and + * dynamic functions to be used in SQL queries without manual mapping configuration. + * + *

This feature is disabled by default for backward compatibility. + * + * @return true if automatic fallback to dynamic function mapping should be enabled; false + * otherwise (default) + */ + @Value.Default + public boolean autoFallbackToDynamicFunctionMapping() { + return false; + } +} diff --git a/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java b/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java index 3c61acd94..fbafc78b8 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java @@ -45,9 +45,45 @@ public static List from( SimpleExtension.ExtensionCollection collection, RelDataTypeFactory typeFactory, TypeConverter typeConverter) { - // TODO: add support for windows functions return Stream.concat( - collection.scalarFunctions().stream(), collection.aggregateFunctions().stream()) + Stream.concat( + collection.scalarFunctions().stream(), collection.aggregateFunctions().stream()), + collection.windowFunctions().stream()) + .map(function -> toSqlFunction(function, typeFactory, typeConverter)) + .collect(Collectors.toList()); + } + + /** + * Converts a list of functions to SqlOperators. Handles scalar, aggregate, and window functions. + * + * @param functions list of functions to convert + * @param typeFactory the Calcite type factory + * @return list of SqlOperators + */ + public static List from( + List functions, RelDataTypeFactory typeFactory) { + return from(functions, typeFactory, TypeConverter.DEFAULT); + } + + /** + * Converts a list of functions to SqlOperators. Handles scalar, aggregate, and window functions. + * + *

Each function variant is converted to a separate SqlOperator. Functions with the same base + * name but different type signatures (e.g., strftime:ts_str, strftime:ts_string) are ALL added to + * the operator table. Calcite will try to match the function call arguments against all available + * operators and select the one that matches. This allows functions with multiple signatures to be + * used correctly without explicit deduplication. + * + * @param functions list of functions to convert + * @param typeFactory the Calcite type factory + * @param typeConverter the type converter + * @return list of SqlOperators + */ + public static List from( + List functions, + RelDataTypeFactory typeFactory, + TypeConverter typeConverter) { + return functions.stream() .map(function -> toSqlFunction(function, typeFactory, typeConverter)) .collect(Collectors.toList()); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java index b5604d4d9..303399fd4 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java @@ -240,9 +240,30 @@ private Optional signatureMatch(List inputTypes, Type outputType) { for (F function : functions) { List args = function.requiredArguments(); // Make sure that arguments & return are within bounds and match the types - if (function.returnType() instanceof ParameterizedType - && isMatch(outputType, (ParameterizedType) function.returnType()) - && inputTypesMatchDefinedArguments(inputTypes, args)) { + boolean returnTypeMatches; + Object funcReturnType = function.returnType(); + + if (funcReturnType instanceof ParameterizedType) { + returnTypeMatches = isMatch(outputType, (ParameterizedType) funcReturnType); + } else if (funcReturnType instanceof Type) { + // For non-parameterized return types, check if they match + Type targetType = (Type) funcReturnType; + if (outputType instanceof ParameterizedType) { + // outputType is parameterized but targetType is not - use visitor pattern + returnTypeMatches = + ((ParameterizedType) outputType) + .accept(new IgnoreNullableAndParameters(targetType)); + } else { + // Both are non-parameterized types - compare them directly by using the visitor + // Create a simple visitor that just checks class equality + returnTypeMatches = outputType.getClass().equals(targetType.getClass()); + } + } else { + // If function.returnType() is neither Type nor ParameterizedType, skip it + returnTypeMatches = false; + } + + if (returnTypeMatches && inputTypesMatchDefinedArguments(inputTypes, args)) { return Optional.of(function); } } @@ -476,6 +497,13 @@ public Optional attemptMatch(C call, Function topLevelCo if (leastRestrictive.isPresent()) { return leastRestrictive; } + } else { + // Fallback: try matchCoerced even if singularInputType is empty + // This handles functions with mixed argument types like strftime(timestamp, string) + Optional coerced = matchCoerced(call, outputType, operands); + if (coerced.isPresent()) { + return coerced; + } } return Optional.empty(); } @@ -565,4 +593,25 @@ private static boolean isMatch(ParameterizedType actualType, ParameterizedType t } return actualType.accept(new IgnoreNullableAndParameters(targetType)); } + + /** + * Identifies functions that are not mapped in the provided Sig list. + * + * @param functions the list of function variants to check + * @param sigs the list of mapped Sig signatures + * @return a list of functions that are not found in the Sig mappings (case-insensitive name + * comparison) + */ + public static List getUnmappedFunctions( + List functions, ImmutableList sigs) { + Set mappedNames = + sigs.stream() + .map(FunctionMappings.Sig::name) + .map(name -> name.toLowerCase(Locale.ROOT)) + .collect(Collectors.toSet()); + + return functions.stream() + .filter(fn -> !mappedNames.contains(fn.name().toLowerCase(Locale.ROOT))) + .collect(Collectors.toList()); + } } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java b/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java index f8b4be1dd..7ad7380cd 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java @@ -50,7 +50,12 @@ public Boolean visit(Type.FP64 type) { @Override public Boolean visit(Type.Str type) { - return typeToMatch instanceof Type.Str; + // Treat all string types as compatible: Str, VarChar, and FixedChar + return typeToMatch instanceof Type.Str + || typeToMatch instanceof Type.VarChar + || typeToMatch instanceof Type.FixedChar + || typeToMatch instanceof ParameterizedType.VarChar + || typeToMatch instanceof ParameterizedType.FixedChar; } @Override @@ -108,13 +113,22 @@ public Boolean visit(Type.UserDefined type) throws RuntimeException { @Override public Boolean visit(Type.FixedChar type) { + // Treat all string types as compatible: Str, VarChar, and FixedChar return typeToMatch instanceof Type.FixedChar - || typeToMatch instanceof ParameterizedType.FixedChar; + || typeToMatch instanceof ParameterizedType.FixedChar + || typeToMatch instanceof Type.Str + || typeToMatch instanceof Type.VarChar + || typeToMatch instanceof ParameterizedType.VarChar; } @Override public Boolean visit(Type.VarChar type) { - return typeToMatch instanceof Type.VarChar || typeToMatch instanceof ParameterizedType.VarChar; + // Treat all string types as compatible: Str, VarChar, and FixedChar + return typeToMatch instanceof Type.VarChar + || typeToMatch instanceof ParameterizedType.VarChar + || typeToMatch instanceof Type.Str + || typeToMatch instanceof Type.FixedChar + || typeToMatch instanceof ParameterizedType.FixedChar; } @Override @@ -131,18 +145,21 @@ public Boolean visit(Type.Decimal type) { @Override public Boolean visit(Type.PrecisionTime type) { return typeToMatch instanceof Type.PrecisionTime + || typeToMatch instanceof Type.Time || typeToMatch instanceof ParameterizedType.PrecisionTime; } @Override public Boolean visit(Type.PrecisionTimestamp type) { return typeToMatch instanceof Type.PrecisionTimestamp + || typeToMatch instanceof Type.Timestamp || typeToMatch instanceof ParameterizedType.PrecisionTimestamp; } @Override public Boolean visit(Type.PrecisionTimestampTZ type) { return typeToMatch instanceof Type.PrecisionTimestampTZ + || typeToMatch instanceof Type.TimestampTZ || typeToMatch instanceof ParameterizedType.PrecisionTimestampTZ; } @@ -164,13 +181,22 @@ public Boolean visit(Type.Map type) { @Override public Boolean visit(ParameterizedType.FixedChar expr) throws RuntimeException { + // Treat all string types as compatible: Str, VarChar, and FixedChar return typeToMatch instanceof Type.FixedChar - || typeToMatch instanceof ParameterizedType.FixedChar; + || typeToMatch instanceof ParameterizedType.FixedChar + || typeToMatch instanceof Type.Str + || typeToMatch instanceof Type.VarChar + || typeToMatch instanceof ParameterizedType.VarChar; } @Override public Boolean visit(ParameterizedType.VarChar expr) throws RuntimeException { - return typeToMatch instanceof Type.VarChar || typeToMatch instanceof ParameterizedType.VarChar; + // Treat all string types as compatible: Str, VarChar, and FixedChar + return typeToMatch instanceof Type.VarChar + || typeToMatch instanceof ParameterizedType.VarChar + || typeToMatch instanceof Type.Str + || typeToMatch instanceof Type.FixedChar + || typeToMatch instanceof ParameterizedType.FixedChar; } @Override @@ -199,18 +225,21 @@ public Boolean visit(ParameterizedType.IntervalCompound expr) throws RuntimeExce @Override public Boolean visit(ParameterizedType.PrecisionTime expr) throws RuntimeException { return typeToMatch instanceof Type.PrecisionTime + || typeToMatch instanceof Type.Time || typeToMatch instanceof ParameterizedType.PrecisionTime; } @Override public Boolean visit(ParameterizedType.PrecisionTimestamp expr) throws RuntimeException { return typeToMatch instanceof Type.PrecisionTimestamp + || typeToMatch instanceof Type.Timestamp || typeToMatch instanceof ParameterizedType.PrecisionTimestamp; } @Override public Boolean visit(ParameterizedType.PrecisionTimestampTZ expr) throws RuntimeException { return typeToMatch instanceof Type.PrecisionTimestampTZ + || typeToMatch instanceof Type.TimestampTZ || typeToMatch instanceof ParameterizedType.PrecisionTimestampTZ; } diff --git a/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java b/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java new file mode 100644 index 000000000..00b1c12d3 --- /dev/null +++ b/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java @@ -0,0 +1,194 @@ +package io.substrait.isthmus; + +import io.substrait.expression.ExpressionCreator; +import io.substrait.extension.DefaultExtensionCatalog; +import io.substrait.plan.Plan; +import io.substrait.relation.NamedScan; +import io.substrait.relation.Project; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Roundtrip test for the autoFallbackToDynamicFunctionMapping feature. + * + *

This test verifies that: 1. Substrait plans using unmapped functions (like strftime from + * extensions) are built with SubstraitBuilder 2. With autoFallbackToDynamicFunctionMapping enabled, + * these unmapped functions are dynamically mapped to Calcite operators 3. The roundtrip conversion + * (Substrait → Calcite → Substrait) is stable, including for SQL queries + * + *

The test uses unmapped datetime functions like strftime that are defined in extension YAML but + * not in FunctionMappings. + */ +class AutoFallbackDynamicFunctionRoundtripTest extends PlanTestBase { + + AutoFallbackDynamicFunctionRoundtripTest() { + super(DefaultExtensionCatalog.DEFAULT_COLLECTION); + } + + /** + * Test roundtrip with unmapped strftime function using SubstraitBuilder. + * + *

This test builds a Substrait plan using SubstraitBuilder that calls the unmapped + * strftime:ts_str function (which exists in functions_datetime.yaml but NOT in FunctionMappings). + */ + @Test + void testUnmappedStrftimeRoundtrip() { + + // Build table scan + NamedScan table = substraitBuilder.namedScan(List.of("t"), List.of("ts"), List.of(R.TIMESTAMP)); + + // Create project with unmapped strftime function call + Project project = + substraitBuilder.project( + input -> + List.of( + // Call unmapped strftime function: strftime(ts, '%Y-%m-%d') + substraitBuilder.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_DATETIME, + "strftime:ts_str", + R.STRING, + substraitBuilder.fieldReference(input, 0), + ExpressionCreator.string(false, "%Y-%m-%d"))), + table); + + // Build plan with output field names + // Note: Project creates a struct with 2 fields internally, so we need 2 names + Plan plan = + Plan.builder() + .roots( + List.of( + Plan.Root.builder() + .input(project) + .names(List.of("formatted_date", "_ignored")) + .build())) + .build(); + + // Enable autoFallbackToDynamicFunctionMapping + FeatureBoard featureBoard = + ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); + + // Use PlanTestBase helper method for comprehensive roundtrip testing with FeatureBoard + assertFullRoundTrip(plan.getRoots().get(0), featureBoard); + } + + /** + * Test roundtrip with multiple unmapped functions from default extensions. + * + *

This test builds Substrait plans with multiple unmapped functions from the default + * extensions (functions_datetime.yaml) using SubstraitBuilder, then verifies that + * autoFallbackToDynamicFunctionMapping enables dynamic mapping of these functions during + * Substrait → Calcite → Substrait roundtrip conversions. + */ + @Test + void testMultipleUnmappedFunctionsRoundtrip() { + // Build table scan + NamedScan table = substraitBuilder.namedScan(List.of("t"), List.of("ts"), List.of(R.TIMESTAMP)); + + // Create project with multiple unmapped strftime function calls from default extensions + Project project = + substraitBuilder.project( + input -> + List.of( + // First unmapped strftime call: strftime(ts, '%Y-%m-%d') + substraitBuilder.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_DATETIME, + "strftime:ts_str", + R.STRING, + substraitBuilder.fieldReference(input, 0), + ExpressionCreator.string(false, "%Y-%m-%d")), + // Second unmapped strftime call: strftime(ts, '%H:%M:%S') + substraitBuilder.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_DATETIME, + "strftime:ts_str", + R.STRING, + substraitBuilder.fieldReference(input, 0), + ExpressionCreator.string(false, "%H:%M:%S"))), + table); + + // Build plan with output field names + // Project creates a struct with 3 fields internally (input field + 2 output fields) + Plan plan = + Plan.builder() + .roots( + List.of( + Plan.Root.builder() + .input(project) + .names(List.of("date_str", "time_str", "_ignored")) + .build())) + .build(); + + // Enable autoFallbackToDynamicFunctionMapping + FeatureBoard featureBoard = + ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); + + // Use PlanTestBase helper method for comprehensive roundtrip testing with FeatureBoard + assertFullRoundTrip(plan.getRoots().get(0), featureBoard); + } + + /** + * Test roundtrip with SQL query using unmapped strftime function. + * + *

This test verifies that SQL queries with unmapped functions (strftime from + * functions_datetime.yaml) can be parsed, validated, and converted to Substrait and back when + * autoFallbackToDynamicFunctionMapping is enabled. The feature flag should enable the + * SqlToSubstrait converter to populate its operator table with unmapped function signatures, + * allowing SqlValidator to accept them during SQL parsing. + */ + @Test + void testUnmappedStrftimeSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; + String query = "SELECT strftime(ts, '%Y-%m-%d') FROM t"; + + // Enable autoFallbackToDynamicFunctionMapping to populate operator table with unmapped + // functions + FeatureBoard featureBoard = + ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); + + // Perform roundtrip: SQL → Substrait → Calcite → Substrait → Calcite → Substrait + // Uses loose POJO comparison since dynamic function mapping may transform the structure + assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); + } + + /** + * Test roundtrip with SQL query using multiple unmapped functions. + * + *

This test verifies that SQL queries with multiple unmapped function calls can be handled + * when autoFallbackToDynamicFunctionMapping is enabled. The operator table is populated with + * unmapped function signatures, allowing all occurrences of the unmapped functions to be + * recognized during SQL parsing and conversion. + */ + @Test + void testMultipleUnmappedFunctionsSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (date_str VARCHAR, ts_str VARCHAR)"; + String query = + "SELECT strptime_date(date_str, '%Y-%m-%d') AS parsed_date, strptime_timestamp(ts_str, '%Y-%m-%d %H:%M:%S') AS parsed_ts FROM t"; + + // Enable autoFallbackToDynamicFunctionMapping + FeatureBoard featureBoard = + ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); + + // Perform roundtrip with multiple unmapped function calls + assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); + } + + /** + * Test roundtrip with multiple calls to the same unmapped function. + * + *

This test verifies that an unmapped function can be called multiple times in the same query + * and all calls are properly converted and matched. This ensures the function matching logic + * works correctly even with repeated function calls. + */ + @Test + void testMultipleCallsToSameFunctionSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; + String query = + "SELECT strftime(ts, '%Y-%m-%d') AS formatted1, strftime(ts, '%H:%M:%S') AS formatted2 FROM t"; + + // Enable autoFallbackToDynamicFunctionMapping + FeatureBoard featureBoard = + ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); + + // Perform roundtrip with multiple calls to the same function + assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); + } +} From 9314af1c7b0361417e4129225efc1ec890c8fe17 Mon Sep 17 00:00:00 2001 From: "zor@zurich.ibm.com" Date: Thu, 5 Mar 2026 17:03:09 +0100 Subject: [PATCH 2/2] chore(isthmus): adjust automatic dynamic function mapping to new way of conversion config --- ...namicFunctionMappingConverterProvider.java | 157 ++++++++++++++ .../io/substrait/isthmus/FeatureBoard.java | 53 ----- ...oFallbackDynamicFunctionRoundtripTest.java | 194 ------------------ ...icDynamicFunctionMappingRoundtripTest.java | 133 ++++++++++++ .../io/substrait/isthmus/PlanTestBase.java | 14 +- 5 files changed, 297 insertions(+), 254 deletions(-) create mode 100644 isthmus/src/main/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingConverterProvider.java delete mode 100644 isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java delete mode 100644 isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java create mode 100644 isthmus/src/test/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingRoundtripTest.java diff --git a/isthmus/src/main/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingConverterProvider.java new file mode 100644 index 000000000..5f36c4389 --- /dev/null +++ b/isthmus/src/main/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingConverterProvider.java @@ -0,0 +1,157 @@ +package io.substrait.isthmus; + +import io.substrait.extension.DefaultExtensionCatalog; +import io.substrait.extension.SimpleExtension; +import io.substrait.isthmus.expression.AggregateFunctionConverter; +import io.substrait.isthmus.expression.FunctionMappings; +import io.substrait.isthmus.expression.ScalarFunctionConverter; +import io.substrait.isthmus.expression.WindowFunctionConverter; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlOperatorTable; +import org.apache.calcite.sql.util.SqlOperatorTables; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class AutomaticDynamicFunctionMappingConverterProvider extends ConverterProvider { + + private static final Logger LOGGER = + LoggerFactory.getLogger(AutomaticDynamicFunctionMappingConverterProvider.class); + + public AutomaticDynamicFunctionMappingConverterProvider() { + this(DefaultExtensionCatalog.DEFAULT_COLLECTION, SubstraitTypeSystem.TYPE_FACTORY); + } + + public AutomaticDynamicFunctionMappingConverterProvider(SimpleExtension.ExtensionCollection extensions) { + this(extensions, SubstraitTypeSystem.TYPE_FACTORY); + } + + public AutomaticDynamicFunctionMappingConverterProvider( + SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory) { + super(extensions, typeFactory); + this.scalarFunctionConverter = createScalarFunctionConverter(); + this.aggregateFunctionConverter = createAggregateFunctionConverter(); + this.windowFunctionConverter = createWindowFunctionConverter(); + } + + @Override + public SqlOperatorTable getSqlOperatorTable() { + SqlOperatorTable baseOperatorTable = super.getSqlOperatorTable(); + List dynamicOperators = new ArrayList<>(); + + List unmappedScalars = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.scalarFunctions(), + io.substrait.isthmus.expression.FunctionMappings.SCALAR_SIGS); + List unmappedAggregates = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.aggregateFunctions(), + io.substrait.isthmus.expression.FunctionMappings.AGGREGATE_SIGS); + List unmappedWindows = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.windowFunctions(), + io.substrait.isthmus.expression.FunctionMappings.WINDOW_SIGS); + + if (!unmappedScalars.isEmpty()) { + dynamicOperators.addAll(SimpleExtensionToSqlOperator.from(unmappedScalars, typeFactory)); + } + if (!unmappedAggregates.isEmpty()) { + dynamicOperators.addAll(SimpleExtensionToSqlOperator.from(unmappedAggregates, typeFactory)); + } + if (!unmappedWindows.isEmpty()) { + dynamicOperators.addAll(SimpleExtensionToSqlOperator.from(unmappedWindows, typeFactory)); + } + + if (!dynamicOperators.isEmpty()) { + return SqlOperatorTables.chain(baseOperatorTable, SqlOperatorTables.of(dynamicOperators)); + } else { + return baseOperatorTable; + } + } + + protected ScalarFunctionConverter createScalarFunctionConverter() { + List unmappedFunctions = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.scalarFunctions(), FunctionMappings.SCALAR_SIGS); + + List additionalSignatures = new ArrayList<>(); + + if (!unmappedFunctions.isEmpty()) { + LOGGER.info( + "Dynamically mapping {} unmapped scalar functions: {}", + unmappedFunctions.size(), + unmappedFunctions.stream().map(f -> f.name()).collect(Collectors.toList())); + + List dynamicOperators = + SimpleExtensionToSqlOperator.from(unmappedFunctions, typeFactory); + + java.util.Map operatorsByName = new java.util.LinkedHashMap<>(); + for (SqlOperator op : dynamicOperators) { + operatorsByName.put(op.getName().toLowerCase(), op); + } + + additionalSignatures.addAll( + operatorsByName.values().stream() + .map(op -> FunctionMappings.s(op, op.getName().toLowerCase())) + .collect(Collectors.toList())); + } + + return new ScalarFunctionConverter( + extensions.scalarFunctions(), additionalSignatures, typeFactory, typeConverter); + } + + protected AggregateFunctionConverter createAggregateFunctionConverter() { + List additionalSignatures = new ArrayList<>(); + + List unmappedFunctions = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.aggregateFunctions(), FunctionMappings.AGGREGATE_SIGS); + + if (!unmappedFunctions.isEmpty()) { + List dynamicOperators = + SimpleExtensionToSqlOperator.from(unmappedFunctions, typeFactory); + + java.util.Map operatorsByName = new java.util.LinkedHashMap<>(); + for (SqlOperator op : dynamicOperators) { + operatorsByName.put(op.getName().toLowerCase(), op); + } + + additionalSignatures.addAll( + operatorsByName.values().stream() + .map(op -> FunctionMappings.s(op, op.getName().toLowerCase())) + .collect(Collectors.toList())); + } + + return new AggregateFunctionConverter( + extensions.aggregateFunctions(), additionalSignatures, typeFactory, typeConverter); + } + + protected WindowFunctionConverter createWindowFunctionConverter() { + List additionalSignatures = new ArrayList<>(); + + List unmappedFunctions = + io.substrait.isthmus.expression.FunctionConverter.getUnmappedFunctions( + extensions.windowFunctions(), FunctionMappings.WINDOW_SIGS); + + if (!unmappedFunctions.isEmpty()) { + List dynamicOperators = + SimpleExtensionToSqlOperator.from(unmappedFunctions, typeFactory); + + java.util.Map operatorsByName = new java.util.LinkedHashMap<>(); + for (SqlOperator op : dynamicOperators) { + operatorsByName.put(op.getName().toLowerCase(), op); + } + + additionalSignatures.addAll( + operatorsByName.values().stream() + .map(op -> FunctionMappings.s(op, op.getName().toLowerCase())) + .collect(Collectors.toList())); + } + + return new WindowFunctionConverter( + extensions.windowFunctions(), additionalSignatures, typeFactory, typeConverter); + } +} diff --git a/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java b/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java deleted file mode 100644 index c303ebbfb..000000000 --- a/isthmus/src/main/java/io/substrait/isthmus/FeatureBoard.java +++ /dev/null @@ -1,53 +0,0 @@ -package io.substrait.isthmus; - -import org.apache.calcite.avatica.util.Casing; -import org.immutables.value.Value; - -/** - * A feature board is a collection of flags that are enabled or configurations that control the - * handling of a request to convert query [batch] to Substrait plans. - */ -@Value.Immutable -public abstract class FeatureBoard { - - /** - * @return Calcite's identifier casing policy for unquoted identifiers. - */ - @Value.Default - public Casing unquotedCasing() { - return Casing.TO_UPPER; - } - - /** - * Controls whether to support dynamic user-defined functions (UDFs) during SQL to Substrait plan - * conversion. - * - *

When enabled, custom functions defined in extension YAML files are available for use in SQL - * queries. These functions will be dynamically converted to SQL operators during plan conversion. - * This feature must be explicitly enabled by users and is disabled by default. - * - * @return true if dynamic UDFs should be supported; false otherwise (default) - */ - @Value.Default - public boolean allowDynamicUdfs() { - return false; - } - - /** - * Controls whether to automatically create mappings for all unmapped functions using - * SimpleExtensionToSqlOperator. - * - *

When enabled, functions from extension YAML files that are not explicitly mapped in - * FunctionMappings will be automatically mapped to Calcite SqlOperators. This allows custom and - * dynamic functions to be used in SQL queries without manual mapping configuration. - * - *

This feature is disabled by default for backward compatibility. - * - * @return true if automatic fallback to dynamic function mapping should be enabled; false - * otherwise (default) - */ - @Value.Default - public boolean autoFallbackToDynamicFunctionMapping() { - return false; - } -} diff --git a/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java b/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java deleted file mode 100644 index 00b1c12d3..000000000 --- a/isthmus/src/test/java/io/substrait/isthmus/AutoFallbackDynamicFunctionRoundtripTest.java +++ /dev/null @@ -1,194 +0,0 @@ -package io.substrait.isthmus; - -import io.substrait.expression.ExpressionCreator; -import io.substrait.extension.DefaultExtensionCatalog; -import io.substrait.plan.Plan; -import io.substrait.relation.NamedScan; -import io.substrait.relation.Project; -import java.util.List; -import org.junit.jupiter.api.Test; - -/** - * Roundtrip test for the autoFallbackToDynamicFunctionMapping feature. - * - *

This test verifies that: 1. Substrait plans using unmapped functions (like strftime from - * extensions) are built with SubstraitBuilder 2. With autoFallbackToDynamicFunctionMapping enabled, - * these unmapped functions are dynamically mapped to Calcite operators 3. The roundtrip conversion - * (Substrait → Calcite → Substrait) is stable, including for SQL queries - * - *

The test uses unmapped datetime functions like strftime that are defined in extension YAML but - * not in FunctionMappings. - */ -class AutoFallbackDynamicFunctionRoundtripTest extends PlanTestBase { - - AutoFallbackDynamicFunctionRoundtripTest() { - super(DefaultExtensionCatalog.DEFAULT_COLLECTION); - } - - /** - * Test roundtrip with unmapped strftime function using SubstraitBuilder. - * - *

This test builds a Substrait plan using SubstraitBuilder that calls the unmapped - * strftime:ts_str function (which exists in functions_datetime.yaml but NOT in FunctionMappings). - */ - @Test - void testUnmappedStrftimeRoundtrip() { - - // Build table scan - NamedScan table = substraitBuilder.namedScan(List.of("t"), List.of("ts"), List.of(R.TIMESTAMP)); - - // Create project with unmapped strftime function call - Project project = - substraitBuilder.project( - input -> - List.of( - // Call unmapped strftime function: strftime(ts, '%Y-%m-%d') - substraitBuilder.scalarFn( - DefaultExtensionCatalog.FUNCTIONS_DATETIME, - "strftime:ts_str", - R.STRING, - substraitBuilder.fieldReference(input, 0), - ExpressionCreator.string(false, "%Y-%m-%d"))), - table); - - // Build plan with output field names - // Note: Project creates a struct with 2 fields internally, so we need 2 names - Plan plan = - Plan.builder() - .roots( - List.of( - Plan.Root.builder() - .input(project) - .names(List.of("formatted_date", "_ignored")) - .build())) - .build(); - - // Enable autoFallbackToDynamicFunctionMapping - FeatureBoard featureBoard = - ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); - - // Use PlanTestBase helper method for comprehensive roundtrip testing with FeatureBoard - assertFullRoundTrip(plan.getRoots().get(0), featureBoard); - } - - /** - * Test roundtrip with multiple unmapped functions from default extensions. - * - *

This test builds Substrait plans with multiple unmapped functions from the default - * extensions (functions_datetime.yaml) using SubstraitBuilder, then verifies that - * autoFallbackToDynamicFunctionMapping enables dynamic mapping of these functions during - * Substrait → Calcite → Substrait roundtrip conversions. - */ - @Test - void testMultipleUnmappedFunctionsRoundtrip() { - // Build table scan - NamedScan table = substraitBuilder.namedScan(List.of("t"), List.of("ts"), List.of(R.TIMESTAMP)); - - // Create project with multiple unmapped strftime function calls from default extensions - Project project = - substraitBuilder.project( - input -> - List.of( - // First unmapped strftime call: strftime(ts, '%Y-%m-%d') - substraitBuilder.scalarFn( - DefaultExtensionCatalog.FUNCTIONS_DATETIME, - "strftime:ts_str", - R.STRING, - substraitBuilder.fieldReference(input, 0), - ExpressionCreator.string(false, "%Y-%m-%d")), - // Second unmapped strftime call: strftime(ts, '%H:%M:%S') - substraitBuilder.scalarFn( - DefaultExtensionCatalog.FUNCTIONS_DATETIME, - "strftime:ts_str", - R.STRING, - substraitBuilder.fieldReference(input, 0), - ExpressionCreator.string(false, "%H:%M:%S"))), - table); - - // Build plan with output field names - // Project creates a struct with 3 fields internally (input field + 2 output fields) - Plan plan = - Plan.builder() - .roots( - List.of( - Plan.Root.builder() - .input(project) - .names(List.of("date_str", "time_str", "_ignored")) - .build())) - .build(); - - // Enable autoFallbackToDynamicFunctionMapping - FeatureBoard featureBoard = - ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); - - // Use PlanTestBase helper method for comprehensive roundtrip testing with FeatureBoard - assertFullRoundTrip(plan.getRoots().get(0), featureBoard); - } - - /** - * Test roundtrip with SQL query using unmapped strftime function. - * - *

This test verifies that SQL queries with unmapped functions (strftime from - * functions_datetime.yaml) can be parsed, validated, and converted to Substrait and back when - * autoFallbackToDynamicFunctionMapping is enabled. The feature flag should enable the - * SqlToSubstrait converter to populate its operator table with unmapped function signatures, - * allowing SqlValidator to accept them during SQL parsing. - */ - @Test - void testUnmappedStrftimeSqlRoundtrip() throws Exception { - String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; - String query = "SELECT strftime(ts, '%Y-%m-%d') FROM t"; - - // Enable autoFallbackToDynamicFunctionMapping to populate operator table with unmapped - // functions - FeatureBoard featureBoard = - ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); - - // Perform roundtrip: SQL → Substrait → Calcite → Substrait → Calcite → Substrait - // Uses loose POJO comparison since dynamic function mapping may transform the structure - assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); - } - - /** - * Test roundtrip with SQL query using multiple unmapped functions. - * - *

This test verifies that SQL queries with multiple unmapped function calls can be handled - * when autoFallbackToDynamicFunctionMapping is enabled. The operator table is populated with - * unmapped function signatures, allowing all occurrences of the unmapped functions to be - * recognized during SQL parsing and conversion. - */ - @Test - void testMultipleUnmappedFunctionsSqlRoundtrip() throws Exception { - String createStatements = "CREATE TABLE t (date_str VARCHAR, ts_str VARCHAR)"; - String query = - "SELECT strptime_date(date_str, '%Y-%m-%d') AS parsed_date, strptime_timestamp(ts_str, '%Y-%m-%d %H:%M:%S') AS parsed_ts FROM t"; - - // Enable autoFallbackToDynamicFunctionMapping - FeatureBoard featureBoard = - ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); - - // Perform roundtrip with multiple unmapped function calls - assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); - } - - /** - * Test roundtrip with multiple calls to the same unmapped function. - * - *

This test verifies that an unmapped function can be called multiple times in the same query - * and all calls are properly converted and matched. This ensures the function matching logic - * works correctly even with repeated function calls. - */ - @Test - void testMultipleCallsToSameFunctionSqlRoundtrip() throws Exception { - String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; - String query = - "SELECT strftime(ts, '%Y-%m-%d') AS formatted1, strftime(ts, '%H:%M:%S') AS formatted2 FROM t"; - - // Enable autoFallbackToDynamicFunctionMapping - FeatureBoard featureBoard = - ImmutableFeatureBoard.builder().autoFallbackToDynamicFunctionMapping(true).build(); - - // Perform roundtrip with multiple calls to the same function - assertSqlSubstraitRelRoundTripLoosePojoComparison(query, createStatements, featureBoard); - } -} diff --git a/isthmus/src/test/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingRoundtripTest.java b/isthmus/src/test/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingRoundtripTest.java new file mode 100644 index 000000000..da129e504 --- /dev/null +++ b/isthmus/src/test/java/io/substrait/isthmus/AutomaticDynamicFunctionMappingRoundtripTest.java @@ -0,0 +1,133 @@ +package io.substrait.isthmus; + +import io.substrait.expression.ExpressionCreator; +import io.substrait.extension.DefaultExtensionCatalog; +import io.substrait.isthmus.sql.SubstraitCreateStatementParser; +import io.substrait.plan.Plan; +import io.substrait.relation.NamedScan; +import io.substrait.relation.Project; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Roundtrip test for the AutomaticDynamicFunctionMappingConverterProvider feature. + * + *

This test verifies that: 1. Substrait plans using unmapped functions (like strftime or + * regexp_match_substring from extensions) are successfully converted. 2. With + * AutomaticDynamicFunctionMappingConverterProvider enabled, these unmapped functions are dynamically mapped to + * Calcite operators. 3. The roundtrip conversion (Substrait → Calcite → Substrait) is stable, + * including for SQL queries. + * + *

The test uses unmapped functions like strftime and regexp_match_substring that are defined in + * extension YAML but not in FunctionMappings. + */ +class AutomaticDynamicFunctionMappingRoundtripTest extends PlanTestBase { + + AutomaticDynamicFunctionMappingRoundtripTest() { + super(new AutomaticDynamicFunctionMappingConverterProvider()); + } + + /** + * Test roundtrip with multiple variants of the same unmapped function. + * + *

This test builds a Substrait plan with multiple variants of an unmapped function (e.g. + * strftime with a precision timestamp and strftime with a date) using SubstraitBuilder, then + * verifies that the framework correctly maps both to the same generic operator and resolves them + * back to their correct specific variants during roundtrip conversions. + */ + @Test + void testMultipleVariantsOfUnmappedFunctionRoundtrip() { + // Build table scan with a precision timestamp and a date + NamedScan table = + sb.namedScan(List.of("t"), List.of("ts", "dt"), List.of(R.precisionTimestamp(6), R.DATE)); + + // Create project with multiple variants of strftime + Project project = + sb.project( + input -> + List.of( + // First variant: strftime(ts, '%Y-%m-%d') + sb.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_DATETIME, + "strftime:pts_str", + R.STRING, + sb.fieldReference(input, 0), + ExpressionCreator.string(false, "%Y-%m-%d")), + // Second variant: strftime(dt, '%Y-%m-%d') + sb.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_DATETIME, + "strftime:date_str", + R.STRING, + sb.fieldReference(input, 1), + ExpressionCreator.string(false, "%Y-%m-%d"))), + sb.remap(2, 3), // The inputs are indices 0, 1. The two scalarFn outputs are 2, 3. + table); + + // Build plan with output field names + Plan plan = + Plan.builder() + .roots( + List.of( + Plan.Root.builder().input(project).names(List.of("ts_str", "dt_str")).build())) + .build(); + + // Use PlanTestBase helper method for comprehensive roundtrip testing + assertFullRoundTrip(plan.getRoots().get(0)); + } + + /** + * Test roundtrip with SQL query using unmapped strftime function. + * + *

This test verifies that SQL queries with unmapped functions (strftime from + * functions_datetime.yaml) can be parsed, validated, and converted to Substrait and back when + * AutomaticDynamicFunctionMappingConverterProvider is used. The provider populates the operator table with + * unmapped function signatures, allowing SqlValidator to accept them during SQL parsing. + */ + @Test + void testUnmappedStrftimeSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; + String query = "SELECT strftime(ts, '%Y-%m-%d') FROM t"; + + // Perform roundtrip: SQL → Substrait → Calcite → Substrait → Calcite → Substrait + // Uses loose POJO comparison since dynamic function mapping may transform the structure + assertSqlSubstraitRelRoundTripLoosePojoComparison( + query, SubstraitCreateStatementParser.processCreateStatementsToCatalog(createStatements)); + } + + /** + * Test roundtrip with SQL query using multiple unmapped functions. + * + *

This test verifies that SQL queries with multiple unmapped function calls (like + * regexp_match_substring) can be handled when AutomaticDynamicFunctionMappingConverterProvider is used. The + * operator table is populated with unmapped function signatures, allowing occurrences of unmapped + * functions to be recognized during SQL parsing and conversion. + */ + @Test + void testMultipleUnmappedFunctionsSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (date_str VARCHAR, ts_str VARCHAR)"; + String query = + "SELECT regexp_match_substring(date_str, '^[0-9]{4}') AS parsed_date, regexp_match_substring(ts_str, '^[0-9]{4}') AS parsed_ts FROM t"; + + // Perform roundtrip with multiple unmapped function calls + assertSqlSubstraitRelRoundTripLoosePojoComparison( + query, SubstraitCreateStatementParser.processCreateStatementsToCatalog(createStatements)); + } + + /** + * Test roundtrip with multiple calls to the same unmapped function. + * + *

This test verifies that an unmapped function can be called multiple times in the same query + * and all calls are properly converted and matched. This ensures the function matching logic + * works correctly even with repeated function calls. + */ + @Test + void testMultipleCallsToSameFunctionSqlRoundtrip() throws Exception { + String createStatements = "CREATE TABLE t (ts TIMESTAMP)"; + String query = + "SELECT strftime(ts, '%Y-%m-%d') AS formatted1, strftime(ts, '%H:%M:%S') AS formatted2 FROM t"; + + // Perform roundtrip with multiple calls to the same function + assertSqlSubstraitRelRoundTripLoosePojoComparison( + query, SubstraitCreateStatementParser.processCreateStatementsToCatalog(createStatements)); + } +} diff --git a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java index 9d3c8135a..b21b7ce81 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java +++ b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java @@ -247,7 +247,7 @@ protected void assertFullRoundTrip(String sqlQuery, Prepare.CatalogReader catalo RelRoot calcite1 = SubstraitSqlToCalcite.convertQuery(sqlQuery, catalogReader); // Calcite 1 -> Substrait POJO 1 - Plan.Root root1 = SubstraitRelVisitor.convert(calcite1, extensions); + Plan.Root root1 = SubstraitRelVisitor.convert(calcite1, converterProvider); // Substrait Root 1 -> Substrait Proto io.substrait.proto.RelRoot proto = new RelProtoConverter(extensionCollector).toProto(root1); @@ -268,7 +268,7 @@ protected void assertFullRoundTrip(String sqlQuery, Prepare.CatalogReader catalo assertNotNull(calcite2); // Calcite 2 -> Substrait Root 3 - Plan.Root root3 = SubstraitRelVisitor.convert(calcite2, extensions); + Plan.Root root3 = SubstraitRelVisitor.convert(calcite2, converterProvider); // Verify that POJOs are the same assertEquals(root1, root3); @@ -306,7 +306,7 @@ protected void assertFullRoundTripWithIdentityProjectionWorkaround( RelRoot calcite0 = SubstraitSqlToCalcite.convertQuery(sqlQuery, catalogReader); // Calcite 0 -> Substrait POJO 0 - Plan.Root root0 = SubstraitRelVisitor.convert(calcite0, extensions); + Plan.Root root0 = SubstraitRelVisitor.convert(calcite0, converterProvider); // Substrait POJO 0 -> Substrait Proto 0 io.substrait.proto.RelRoot proto0 = new RelProtoConverter(extensionCollector).toProto(root0); @@ -326,7 +326,7 @@ protected void assertFullRoundTripWithIdentityProjectionWorkaround( // End Preparation // Calcite 1 -> Substrait POJO 2 - Plan.Root root2 = SubstraitRelVisitor.convert(calcite1, extensions); + Plan.Root root2 = SubstraitRelVisitor.convert(calcite1, converterProvider); // Substrait POJO 2 -> Substrait Proto 1 io.substrait.proto.RelRoot proto1 = new RelProtoConverter(extensionCollector).toProto(root2); @@ -337,7 +337,7 @@ protected void assertFullRoundTripWithIdentityProjectionWorkaround( // Substrait POJO 3 -> Calcite 2 RelRoot calcite2 = substraitToCalcite.convert(root3); // Calcite 2 -> Substrait POJO 4 - Plan.Root root4 = SubstraitRelVisitor.convert(calcite2, extensions); + Plan.Root root4 = SubstraitRelVisitor.convert(calcite2, converterProvider); // Verify that POJOs are the same assertEquals(root2, root4); @@ -369,7 +369,7 @@ protected void assertFullRoundTrip(Rel pojo1) { RelNode calcite = new SubstraitToCalcite(converterProvider).convert(pojo2); // Calcite -> Substrait POJO 3 - io.substrait.relation.Rel pojo3 = SubstraitRelVisitor.convert(calcite, extensions); + io.substrait.relation.Rel pojo3 = SubstraitRelVisitor.convert(calcite, converterProvider); // Verify that POJOs are the same assertEquals(pojo1, pojo3); @@ -400,7 +400,7 @@ protected void assertFullRoundTrip(Plan.Root pojo1) { RelRoot calcite = new SubstraitToCalcite(converterProvider).convert(pojo2); // Calcite -> Substrait POJO 3 - io.substrait.plan.Plan.Root pojo3 = SubstraitRelVisitor.convert(calcite, extensions); + io.substrait.plan.Plan.Root pojo3 = SubstraitRelVisitor.convert(calcite, converterProvider); // Verify that POJOs are the same assertEquals(pojo1, pojo3);