From 3cc4bc79e19d0a975c4d476f87994894c9e582a6 Mon Sep 17 00:00:00 2001 From: MBWhite Date: Wed, 18 Mar 2026 13:42:08 +0000 Subject: [PATCH 1/2] fix(javadoc): partial isthmus Signed-off-by: MBWhite --- .../isthmus/SimpleExtensionToSqlOperator.java | 40 ++++++++++ .../substrait/isthmus/SqlConverterBase.java | 21 ++++++ .../isthmus/SqlExpressionToSubstrait.java | 73 +++++++++++++++++-- .../io/substrait/isthmus/SqlToSubstrait.java | 8 ++ .../isthmus/SubstraitRelNodeConverter.java | 51 +++++++++++++ 5 files changed, 185 insertions(+), 8 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java b/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java index 3c61acd94..fa622c7f1 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java @@ -23,6 +23,21 @@ import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; +/** + * Utility class for converting Substrait {@link SimpleExtension} function definitions (scalar and + * aggregate) into Calcite {@link SqlOperator}s. + * + *

This enables Calcite to recognize and use Substrait-defined functions during query planning + * and execution. Conversion includes: + * + *

+ * + *

Currently supports scalar and aggregate functions; window functions are not yet implemented. + */ public final class SimpleExtensionToSqlOperator { private static final RelDataTypeFactory DEFAULT_TYPE_FACTORY = @@ -32,15 +47,40 @@ public final class SimpleExtensionToSqlOperator { private SimpleExtensionToSqlOperator() {} + /** + * Converts all functions in a Substrait {@link SimpleExtension.ExtensionCollection} (scalar and + * aggregate) into Calcite {@link SqlOperator}s using the default type factory. + * + * @param collection The Substrait extension collection containing function definitions. + * @return A list of Calcite {@link SqlOperator}s corresponding to the Substrait functions. + */ public static List from(SimpleExtension.ExtensionCollection collection) { return from(collection, DEFAULT_TYPE_FACTORY); } + /** + * Converts all functions in a Substrait {@link SimpleExtension.ExtensionCollection} (scalar and + * aggregate) into Calcite {@link SqlOperator}s using a provided type factory. + * + * @param collection The Substrait extension collection containing function definitions. + * @param typeFactory Calcite {@link RelDataTypeFactory} for type creation and inference. + * @return A list of Calcite {@link SqlOperator}s corresponding to the Substrait functions. + */ public static List from( SimpleExtension.ExtensionCollection collection, RelDataTypeFactory typeFactory) { return from(collection, typeFactory, TypeConverter.DEFAULT); } + /** + * Converts all functions in a Substrait {@link SimpleExtension.ExtensionCollection} (scalar and + * aggregate) into Calcite {@link SqlOperator}s with a custom type factory and {@link + * TypeConverter}. + * + * @param collection The Substrait extension collection containing function definitions. + * @param typeFactory Calcite {@link RelDataTypeFactory} for type creation and inference. + * @param typeConverter Converter for Substrait/Calcite type mappings. + * @return A list of Calcite {@link SqlOperator}s corresponding to the Substrait functions. + */ public static List from( SimpleExtension.ExtensionCollection collection, RelDataTypeFactory typeFactory, diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlConverterBase.java b/isthmus/src/main/java/io/substrait/isthmus/SqlConverterBase.java index 170dc4f94..c7dc54ec6 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlConverterBase.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlConverterBase.java @@ -14,20 +14,41 @@ import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql2rel.SqlToRelConverter; +/** + * Base class for Substrait SQL conversion pipelines. + * + *

Configures Calcite parser, connection, planner, and cluster. Holds the Substrait extensions + * and feature flags. Subclasses can build conversions from SQL to Calcite/Substrait using this + * shared setup. + */ public class SqlConverterBase { protected final ConverterProvider converterProvider; + /** Default Calcite connection config (case-insensitive). */ public static final CalciteConnectionConfig CONNECTION_CONFIG = CalciteConnectionConfig.DEFAULT.set( CalciteConnectionProperty.CASE_SENSITIVE, Boolean.FALSE.toString()); + /** Calcite type factory using the Substrait type system. */ final RelDataTypeFactory factory; + + /** Calcite optimization cluster with planner, type factory, and RexBuilder. */ final RelOptCluster relOptCluster; + + /** Connection configuration used for SQL parsing and validation. */ final CalciteConnectionConfig config; + + /** Configuration for SQL-to-Rel conversion. */ final SqlToRelConverter.Config converterConfig; + /** Parser configuration, including casing and DDL parser factory. */ final SqlParser.Config parserConfig; + /** + * Creates a converter base with explicit features and extensions. + * + * @param converterProvider Converter Provider for configuration + */ protected SqlConverterBase(ConverterProvider converterProvider) { this.converterProvider = converterProvider; this.factory = converterProvider.getTypeFactory(); diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java index b43b3753f..898c9dcd8 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java @@ -27,25 +27,47 @@ import org.apache.calcite.sql2rel.SqlToRelConverter; import org.apache.calcite.sql2rel.StandardConvertletTable; +/** + * Converts SQL expressions to Substrait {@link io.substrait.proto.ExtendedExpression} payloads. + * + *

Supports optional CREATE TABLE statements to provide schema and column bindings for expression + * validation and Rex conversion. + */ public class SqlExpressionToSubstrait extends SqlConverterBase { + /** Converter for RexNodes to Substrait expressions. */ protected final RexExpressionConverter rexConverter; + /** Creates a converter with default features and the default extension catalog. */ public SqlExpressionToSubstrait() { this(new ConverterProvider()); } + /** + * Creates a converter with the given feature board and extension collection. + * + * @param converterProvider Converter Provider to use for configuration + */ public SqlExpressionToSubstrait(ConverterProvider converterProvider) { super(converterProvider); this.rexConverter = new RexExpressionConverter(converterProvider.getScalarFunctionConverter()); } + /** Bundled result carrying validator, catalog reader, and name/type and name/node maps. */ private static final class Result { final SqlValidator validator; final CalciteCatalogReader catalogReader; final Map nameToTypeMap; final Map nameToNodeMap; + /** + * Creates a result bundle. + * + * @param validator SQL validator + * @param catalogReader Calcite catalog reader + * @param nameToTypeMap mapping from column name to Calcite type + * @param nameToNodeMap mapping from column name to Rex input ref + */ Result( SqlValidator validator, CalciteCatalogReader catalogReader, @@ -59,12 +81,12 @@ private static final class Result { } /** - * Converts the given SQL expression to an {@link io.substrait.proto.ExtendedExpression } + * Converts a single SQL expression to a Substrait {@link io.substrait.proto.ExtendedExpression}. * * @param sqlExpression a SQL expression * @param createStatements table creation statements defining fields referenced by the expression - * @return a {@link io.substrait.proto.ExtendedExpression } - * @throws SqlParseException + * @return the Substrait extended expression proto + * @throws SqlParseException if parsing or validation fails */ public io.substrait.proto.ExtendedExpression convert( String sqlExpression, List createStatements) throws SqlParseException { @@ -72,12 +94,12 @@ public io.substrait.proto.ExtendedExpression convert( } /** - * Converts the given SQL expressions to an {@link io.substrait.proto.ExtendedExpression } + * Converts multiple SQL expressions to a Substrait {@link io.substrait.proto.ExtendedExpression}. * - * @param sqlExpressions an array of SQL expressions - * @param createStatements table creation statements defining fields referenced by the expression - * @return a {@link io.substrait.proto.ExtendedExpression } - * @throws SqlParseException + * @param sqlExpressions array of SQL expressions + * @param createStatements table creation statements defining fields referenced by the expressions + * @return the Substrait extended expression proto + * @throws SqlParseException if parsing or validation fails */ public io.substrait.proto.ExtendedExpression convert( String[] sqlExpressions, List createStatements) throws SqlParseException { @@ -90,6 +112,17 @@ public io.substrait.proto.ExtendedExpression convert( result.nameToNodeMap); } + /** + * Converts the given SQL expressions using the provided validator/catalog and column bindings. + * + * @param sqlExpressions array of SQL expressions + * @param validator SQL validator + * @param catalogReader Calcite catalog reader + * @param nameToTypeMap mapping from column name to Calcite type + * @param nameToNodeMap mapping from column name to Rex input ref + * @return the Substrait extended expression proto + * @throws SqlParseException if parsing or validation fails + */ private io.substrait.proto.ExtendedExpression executeInnerSQLExpressions( String[] sqlExpressions, SqlValidator validator, @@ -120,6 +153,17 @@ private io.substrait.proto.ExtendedExpression executeInnerSQLExpressions( return new ExtendedExpressionProtoConverter().toProto(extendedExpression.build()); } + /** + * Parses and validates a SQL expression, then converts it to a {@link RexNode}. + * + * @param sql SQL expression string + * @param validator SQL validator + * @param catalogReader Calcite catalog reader + * @param nameToTypeMap mapping from column name to Calcite type + * @param nameToNodeMap mapping from column name to Rex input ref + * @return the converted RexNode + * @throws SqlParseException if parsing or validation fails + */ private RexNode sqlToRexNode( String sql, SqlValidator validator, @@ -141,6 +185,13 @@ private RexNode sqlToRexNode( return converter.convertExpression(validSqlNode, nameToNodeMap); } + /** + * Registers tables from CREATE statements and prepares validator, catalog, and column bindings. + * + * @param tables list of CREATE TABLE statements; may be null + * @return result bundle containing validator, catalog reader, and name/type and name/node maps + * @throws SqlParseException if any CREATE statement is invalid + */ private Result registerCreateTablesForExtendedExpression(List tables) throws SqlParseException { Map nameToTypeMap = new LinkedHashMap<>(); @@ -177,6 +228,12 @@ private Result registerCreateTablesForExtendedExpression(List tables) return new Result(validator, catalogReader, nameToTypeMap, nameToNodeMap); } + /** + * Converts a name-to-type map into a {@link NamedStruct} in Substrait types. + * + * @param nameToTypeMap mapping from column name to Calcite type + * @return a {@link NamedStruct} with non-nullable struct type + */ private NamedStruct toNamedStruct(Map nameToTypeMap) { ArrayList names = new ArrayList(); ArrayList types = new ArrayList(); diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java index a76567f36..53f4b37cc 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java @@ -19,10 +19,18 @@ public class SqlToSubstrait extends SqlConverterBase { private final SqlOperatorTable operatorTable; + /** + * Creates a SQL-to-Substrait converter using the default extension catalog and default features. + */ public SqlToSubstrait() { this(new ConverterProvider()); } + /** + * Creates a SQL-to-Substrait converter with explicit extensions and features. + * + * @param converterProvider Converter Provider for the configuration + */ public SqlToSubstrait(ConverterProvider converterProvider) { super(converterProvider); this.operatorTable = converterProvider.getSqlOperatorTable(); diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java index f07a87a78..67262132b 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java @@ -90,18 +90,37 @@ public class SubstraitRelNodeConverter extends AbstractRelVisitor { + /** Calcite type factory used to construct row and field types. */ protected final RelDataTypeFactory typeFactory; + /** Converter for Substrait scalar functions to Calcite operators. */ protected final ScalarFunctionConverter scalarFunctionConverter; + + /** Converter for Substrait aggregate functions to Calcite operators. */ protected final AggregateFunctionConverter aggregateFunctionConverter; + + /** Converts Substrait {@code Expression}s into Calcite {@code RexNode}s. */ protected final ExpressionRexConverter expressionRexConverter; + /** Calcite {@link RelBuilder} used to construct relational expressions during conversion. */ protected final RelBuilder relBuilder; + + /** Calcite {@link RexBuilder} used to build Rex nodes (e.g., input refs, literals). */ protected final RexBuilder rexBuilder; + + /** Type converter to translate between Calcite and Substrait type systems. */ private final TypeConverter typeConverter; /** Use {@link #SubstraitRelNodeConverter(RelBuilder, ConverterProvider)} instead */ @Deprecated + /** + * Creates a new SubstraitRelNodeConverter with the specified extensions, type factory, and + * relation builder. + * + * @param extensions the Substrait extension collection + * @param typeFactory the Calcite type factory + * @param relBuilder the Calcite relation builder + */ public SubstraitRelNodeConverter( SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory, @@ -109,6 +128,13 @@ public SubstraitRelNodeConverter( this(relBuilder, new ConverterProvider(extensions, typeFactory)); } + /** + * Creates a new SubstraitRelNodeConverter with the specified relation builder and converter + * provider. + * + * @param relBuilder the Calcite relation builder + * @param converterProvider the converter provider containing configuration and converters + */ public SubstraitRelNodeConverter(RelBuilder relBuilder, ConverterProvider converterProvider) { this.typeFactory = converterProvider.getTypeFactory(); this.typeConverter = converterProvider.getTypeConverter(); @@ -119,6 +145,18 @@ public SubstraitRelNodeConverter(RelBuilder relBuilder, ConverterProvider conver this.expressionRexConverter = converterProvider.getExpressionRexConverter(this); } + /** + * Converts a Substrait {@link Rel} plan to a Calcite {@link RelNode} using default feature + * settings. + * + *

This method creates a {@link RelBuilder} configured with the provided cluster and catalog, + * then delegates to the converter with default features. + * + * @param relRoot the root Substrait relation to convert + * @param catalogReader the Calcite catalog reader for schema resolution + * @param converterProvider the converter provider containing configuration and converters + * @return the converted Calcite {@link RelNode} + */ public static RelNode convert( Rel relRoot, Prepare.CatalogReader catalogReader, ConverterProvider converterProvider) { RelBuilder relBuilder = @@ -736,6 +774,16 @@ public RelNode visitFallback(Rel rel, Context context) throws RuntimeException { rel, rel.getClass().getCanonicalName(), this.getClass().getCanonicalName())); } + /** + * Applies an optional field remap to the given node. + * + *

If {@code remap} is present, the node is projected according to the provided indices; + * otherwise the original node is returned unchanged. + * + * @param relNode the node to remap + * @param remap optional field index remap + * @return remapped node or original node if no remap is present + */ protected RelNode applyRemap(RelNode relNode, Optional remap) { if (remap.isPresent()) { return applyRemap(relNode, remap.get()); @@ -759,8 +807,10 @@ private RelNode applyRemap(RelNode relNode, Rel.Remap remap) { /** A shared context for the Substrait to RelNode conversion. */ public static class Context implements VisitationContext { + /** Stack of outer row type range maps used to resolve correlated references. */ protected final Stack> outerRowTypes = new Stack<>(); + /** Stack of correlation ids collected while visiting subqueries. */ protected final Stack> correlationIds = new Stack<>(); private int subqueryDepth; @@ -798,6 +848,7 @@ public void pushOuterRowType(final RelDataType... inputs) { this.correlationIds.push(new HashSet<>()); } + /** Pops the most recent outer row type from the stack. */ public void popOuterRowType() { outerRowTypes.pop(); } From e9c769c6bec9b6772b4b5dfe41d26302ea30098c Mon Sep 17 00:00:00 2001 From: MBWhite Date: Thu, 19 Mar 2026 09:11:34 +0000 Subject: [PATCH 2/2] review comments Signed-off-by: MBWhite --- .../io/substrait/isthmus/SqlExpressionToSubstrait.java | 4 ++-- .../main/java/io/substrait/isthmus/SqlToSubstrait.java | 4 +--- .../substrait/isthmus/SubstraitRelNodeConverter.java | 10 +++------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java index 898c9dcd8..5c020d5d1 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java @@ -38,13 +38,13 @@ public class SqlExpressionToSubstrait extends SqlConverterBase { /** Converter for RexNodes to Substrait expressions. */ protected final RexExpressionConverter rexConverter; - /** Creates a converter with default features and the default extension catalog. */ + /** Creates a converter with default configuration. */ public SqlExpressionToSubstrait() { this(new ConverterProvider()); } /** - * Creates a converter with the given feature board and extension collection. + * Creates a converter with the given converter provider. * * @param converterProvider Converter Provider to use for configuration */ diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java index 53f4b37cc..0fb7cf1c3 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java @@ -19,9 +19,7 @@ public class SqlToSubstrait extends SqlConverterBase { private final SqlOperatorTable operatorTable; - /** - * Creates a SQL-to-Substrait converter using the default extension catalog and default features. - */ + /** Creates a SQL-to-Substrait converter using the default configuration. */ public SqlToSubstrait() { this(new ConverterProvider()); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java index 67262132b..52849f96f 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java @@ -111,8 +111,6 @@ public class SubstraitRelNodeConverter /** Type converter to translate between Calcite and Substrait type systems. */ private final TypeConverter typeConverter; - /** Use {@link #SubstraitRelNodeConverter(RelBuilder, ConverterProvider)} instead */ - @Deprecated /** * Creates a new SubstraitRelNodeConverter with the specified extensions, type factory, and * relation builder. @@ -120,7 +118,9 @@ public class SubstraitRelNodeConverter * @param extensions the Substrait extension collection * @param typeFactory the Calcite type factory * @param relBuilder the Calcite relation builder + * @deprecated Use {@link #SubstraitRelNodeConverter(RelBuilder, ConverterProvider)} instead */ + @Deprecated public SubstraitRelNodeConverter( SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory, @@ -146,11 +146,7 @@ public SubstraitRelNodeConverter(RelBuilder relBuilder, ConverterProvider conver } /** - * Converts a Substrait {@link Rel} plan to a Calcite {@link RelNode} using default feature - * settings. - * - *

This method creates a {@link RelBuilder} configured with the provided cluster and catalog, - * then delegates to the converter with default features. + * Converts a Substrait {@link Rel} plan to a Calcite {@link RelNode}. * * @param relRoot the root Substrait relation to convert * @param catalogReader the Calcite catalog reader for schema resolution