Skip to content

Commit 685de52

Browse files
committed
test: updates scaffolding for UdfSqlToSubstraitTest
1 parent 98e6cea commit 685de52

5 files changed

Lines changed: 47 additions & 10 deletions

File tree

isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public class ConverterProvider {
2121

2222
protected final RelDataTypeFactory typeFactory;
2323

24-
private final ScalarFunctionConverter scalarFunctionConverter;
24+
protected ScalarFunctionConverter scalarFunctionConverter;
2525
private final AggregateFunctionConverter aggregateFunctionConverter;
2626
private final WindowFunctionConverter windowFunctionConverter;
2727

isthmus/src/main/java/io/substrait/isthmus/DynamicConverterProvider.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ public class DynamicConverterProvider extends ConverterProvider {
1616

1717
private final SimpleExtension.ExtensionCollection extensions;
1818

19-
private final ScalarFunctionConverter scalarFunctionConverter;
20-
2119
public DynamicConverterProvider(
2220
RelDataTypeFactory typeFactory, SimpleExtension.ExtensionCollection extensions) {
2321
super(typeFactory, extensions);

isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,35 @@ public static Plan.Root convert(RelRoot relRoot, SimpleExtension.ExtensionCollec
613613
return convert(relRoot, extensions, FEATURES_DEFAULT);
614614
}
615615

616+
/**
617+
* Converts a Calcite {@link RelRoot} to a Substrait {@link Plan.Root} using a custom visitor.
618+
*
619+
* <p>This is the main conversion entry point for a complete plan. It applies the provided {@link
620+
* SubstraitRelVisitor} to the final projected {@link RelNode} from the {@code relRoot}, and wraps
621+
* the resulting {@link Rel} in a {@link Plan.Root}.
622+
*
623+
* <p>This method also correctly extracts the final output field names, paying special attention
624+
* to nested types (structs, maps) via the visitor's type converter, rather than using the names
625+
* from {@code relRoot.validatedRowType} directly.
626+
*
627+
* @param relRoot The Calcite RelRoot to convert. This is expected to be a complete, optimized
628+
* plan.
629+
* @param visitor {@link SubstraitRelVisitor} or its subclass. This allows for custom visitor
630+
* behavior.
631+
* @return The resulting Substrait Plan.Root, containing the converted relational tree and the
632+
* output names.
633+
*/
634+
public static Plan.Root convert(RelRoot relRoot, ConverterProvider converterProvider) {
635+
SubstraitRelVisitor visitor = new SubstraitRelVisitor(converterProvider);
636+
visitor.popFieldAccessDepthMap(relRoot.rel);
637+
Rel rel = visitor.apply(relRoot.project());
638+
639+
// Avoid using the names from relRoot.validatedRowType because if there are
640+
// nested types (i.e ROW, MAP, etc) the typeConverter will pad names correctly
641+
List<String> names = visitor.typeConverter.toNamedStruct(relRoot.validatedRowType).names();
642+
return Plan.Root.builder().input(rel).names(names).build();
643+
}
644+
616645
/**
617646
* Converts a Calcite {@link RelRoot} to a Substrait {@link Plan.Root} using a custom visitor.
618647
*

isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public class PlanTestBase {
5050

5151
protected static final CalciteCatalogReader TPCH_CATALOG;
5252

53+
protected ConverterProvider converterProvider;
54+
5355
static {
5456
try {
5557
String tpchCreateStatements = asString("tpch/schema.sql");
@@ -69,8 +71,14 @@ protected PlanTestBase() {
6971
}
7072

7173
protected PlanTestBase(SimpleExtension.ExtensionCollection extensions) {
74+
this(extensions, new ConverterProvider(extensions));
75+
}
76+
77+
protected PlanTestBase(
78+
SimpleExtension.ExtensionCollection extensions, ConverterProvider converterProvider) {
7279
this.extensions = extensions;
7380
this.substraitBuilder = new SubstraitBuilder(extensions);
81+
this.converterProvider = converterProvider;
7482
}
7583

7684
public static String asString(String resource) throws IOException {
@@ -134,7 +142,8 @@ protected RelRoot assertSqlSubstraitRelRoundTrip(
134142
// Return list of sql -> Substrait rel -> Calcite rel.
135143

136144
SqlToSubstrait s2s = new SqlToSubstrait();
137-
SubstraitToCalcite substraitToCalcite = new SubstraitToCalcite(extensions, typeFactory);
145+
SubstraitToCalcite substraitToCalcite =
146+
new SubstraitToCalcite(converterProvider, catalogReader);
138147

139148
// 1. SQL -> Substrait Plan
140149
Plan plan1 = s2s.convert(query, catalogReader);
@@ -146,7 +155,7 @@ protected RelRoot assertSqlSubstraitRelRoundTrip(
146155
RelRoot relRoot2 = substraitToCalcite.convert(pojo1);
147156

148157
// 4. Calcite RelNode -> Substrait Rel
149-
Plan.Root pojo2 = SubstraitRelVisitor.convert(relRoot2, extensions);
158+
Plan.Root pojo2 = SubstraitRelVisitor.convert(relRoot2, converterProvider);
150159

151160
assertEquals(pojo1, pojo2);
152161
return relRoot2;
@@ -181,11 +190,11 @@ protected RelRoot assertSqlSubstraitRelRoundTripLoosePojoComparison(
181190
featureBoard != null ? featureBoard : ImmutableFeatureBoard.builder().build();
182191

183192
SubstraitToCalcite substraitToCalcite =
184-
new SubstraitToCalcite(extensions, typeFactory, TypeConverter.DEFAULT, null, features);
185-
SqlToSubstrait s = new SqlToSubstrait(extensions, features);
193+
new SubstraitToCalcite(converterProvider, catalogReader);
194+
SqlToSubstrait sqlToSubstrait = new SqlToSubstrait(extensions, converterProvider, featureBoard);
186195

187196
// 1. SQL -> Substrait Plan
188-
Plan plan1 = s.convert(query, catalogReader);
197+
Plan plan1 = sqlToSubstrait.convert(query, catalogReader);
189198

190199
// 2. Substrait Plan -> Substrait Root (POJO 1)
191200
Plan.Root pojo1 = plan1.getRoots().get(0);
@@ -194,7 +203,7 @@ protected RelRoot assertSqlSubstraitRelRoundTripLoosePojoComparison(
194203
RelRoot relRoot2 = substraitToCalcite.convert(pojo1);
195204

196205
// 4. Calcite RelNode -> Substrait Root (POJO 2)
197-
Plan.Root pojo2 = SubstraitRelVisitor.convert(relRoot2, extensions, features);
206+
Plan.Root pojo2 = SubstraitRelVisitor.convert(relRoot2, converterProvider);
198207

199208
// Note: pojo1 and pojo2 may differ due to different optimization strategies applied by:
200209
// - SqlNode->RelRoot conversion during SQL->Substrait conversion
@@ -205,7 +214,7 @@ protected RelRoot assertSqlSubstraitRelRoundTripLoosePojoComparison(
205214
RelRoot relRoot3 = substraitToCalcite.convert(pojo2);
206215

207216
// 6. Calcite RelNode -> Substrait Root (POJO 3)
208-
Plan.Root pojo3 = SubstraitRelVisitor.convert(relRoot3, extensions, features);
217+
Plan.Root pojo3 = SubstraitRelVisitor.convert(relRoot3, converterProvider);
209218

210219
// Verify that subsequent round trips are stable (pojo2 and pojo3 should be identical)
211220
assertEquals(pojo2, pojo3);

isthmus/src/test/java/io/substrait/isthmus/UdfSqlSubstraitTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class UdfSqlSubstraitTest extends PlanTestBase {
1313

1414
UdfSqlSubstraitTest() {
1515
super(loadExtensions(List.of(CUSTOM_FUNCTION_PATH)));
16+
this.converterProvider = new DynamicConverterProvider(typeFactory, extensions);
1617
}
1718

1819
@Test

0 commit comments

Comments
 (0)