From 57f284c3011f5214dd728777280829afcf74bf59 Mon Sep 17 00:00:00 2001 From: MBWhite Date: Wed, 18 Mar 2026 09:18:52 +0000 Subject: [PATCH 01/10] Update substrait builder Signed-off-by: MBWhite --- .bob/notes/pending-notes.txt | 0 .../.kotlin/sessions/kotlin-compiler-17666991750707048222.salive | 0 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 .bob/notes/pending-notes.txt create mode 100644 build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive diff --git a/.bob/notes/pending-notes.txt b/.bob/notes/pending-notes.txt new file mode 100644 index 000000000..e69de29bb diff --git a/build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive b/build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive new file mode 100644 index 000000000..e69de29bb From 1d28363346ccc0fbbfa480e93be80cbcd9f2968f Mon Sep 17 00:00:00 2001 From: MBWhite Date: Mon, 23 Mar 2026 11:54:42 +0000 Subject: [PATCH 02/10] feat: substrait builder extra apifeat: substrait builder extra api Signed-off-by: MBWhite --- .bob/notes/pending-notes.txt | 0 ...otlin-compiler-17666991750707048222.salive | 0 .../io/substrait/dsl/SubstraitBuilder.java | 222 ++++++++++++++- .../substrait/dsl/SubstraitBuilderTest.java | 265 ++++++++++++++++++ 4 files changed, 474 insertions(+), 13 deletions(-) delete mode 100644 .bob/notes/pending-notes.txt delete mode 100644 build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive create mode 100644 core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java diff --git a/.bob/notes/pending-notes.txt b/.bob/notes/pending-notes.txt deleted file mode 100644 index e69de29bb..000000000 diff --git a/build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive b/build-logic/.kotlin/sessions/kotlin-compiler-17666991750707048222.salive deleted file mode 100644 index e69de29bb..000000000 diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index 54d4f63a3..ba9694652 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -12,6 +12,7 @@ import io.substrait.expression.Expression.SwitchClause; import io.substrait.expression.FieldReference; import io.substrait.expression.FunctionArg; +import io.substrait.expression.FunctionOption; import io.substrait.expression.WindowBound; import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.extension.SimpleExtension; @@ -39,6 +40,7 @@ import io.substrait.type.NamedStruct; import io.substrait.type.Type; import io.substrait.type.TypeCreator; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.LinkedList; @@ -73,6 +75,17 @@ public class SubstraitBuilder { private final SimpleExtension.ExtensionCollection extensions; + /** + * Constructs a new SubstraitBuilder with the default extension collection. + * + *

The builder is initialized with {@link DefaultExtensionCatalog#DEFAULT_COLLECTION}, which + * includes standard Substrait functions for strings, arithmetic, comparison, datetime, and other + * operations. + */ + public SubstraitBuilder() { + this(DefaultExtensionCatalog.DEFAULT_COLLECTION); + } + /** * Constructs a new SubstraitBuilder with the specified extension collection. * @@ -83,6 +96,18 @@ public SubstraitBuilder(SimpleExtension.ExtensionCollection extensions) { this.extensions = extensions; } + /** + * Gets the default extension collection used by this builder. + * + *

This collection includes standard Substrait functions for strings, arithmetic, comparison, + * datetime, and other operations from {@link DefaultExtensionCatalog#DEFAULT_COLLECTION}. + * + * @return the ExtensionCollection containing standard Substrait functions + */ + public SimpleExtension.ExtensionCollection getExtensions() { + return extensions; + } + // Relations /** @@ -142,13 +167,32 @@ public Aggregate aggregate( return aggregate(groupingsFn, measuresFn, Optional.of(remap), input); } - private Aggregate aggregate( - Function> groupingsFn, - Function> measuresFn, - Optional remap, - Rel input) { - List groupings = groupingsFn.apply(input); - List measures = measuresFn.apply(input); + /** + * Creates an aggregate relation that groups and aggregates data from an input relation. + * + *

This method constructs a Substrait aggregate operation by applying grouping and measure + * functions to the input relation. The grouping function defines how rows are grouped together, + * while the measure function defines the aggregate computations (e.g., SUM, COUNT, AVG) to + * perform on each group. + * + *

The optional remap parameter allows reordering or filtering of output columns, which is + * useful for controlling the final schema of the aggregate result. + * + * @param groupingsFn a function that takes the input relation and returns a list of grouping + * expressions defining how to partition the data + * @param measuresFn a function that takes the input relation and returns a list of aggregate + * measures to compute for each group + * @param remap an optional remapping specification to reorder or filter output columns + * @param input the input relation to aggregate + * @return an Aggregate relation representing the grouping and aggregation operation + */ + public Aggregate aggregate( + final Function> groupingsFn, + final Function> measuresFn, + final Optional remap, + final Rel input) { + final List groupings = groupingsFn.apply(input); + final List measures = measuresFn.apply(input); return Aggregate.builder() .groupings(groupings) .measures(measures) @@ -853,24 +897,64 @@ public Expression.BoolLiteral bool(boolean v) { return Expression.BoolLiteral.builder().value(v).build(); } + /** + * Create i16 literal. + * + * @param value value to create + * @return i16 instance + */ + public Expression.I8Literal i8(final int value) { + return Expression.I8Literal.builder().value(value).build(); + } + + /** + * Create i16 literal. + * + * @param value value to create + * @return i16 instance + */ + public Expression.I16Literal i16(final int value) { + return Expression.I16Literal.builder().value(value).build(); + } + /** * Creates a 32-bit integer literal expression. * - * @param v the integer value + * @param value the integer value * @return a new {@link Expression.I32Literal} */ - public Expression.I32Literal i32(int v) { - return Expression.I32Literal.builder().value(v).build(); + public Expression.I32Literal i32(final int value) { + return Expression.I32Literal.builder().value(value).build(); + } + + /** + * Creates a 64-bit integer literal expression. + * + * @param value value to create + * @return i64 instance + */ + public Expression.I64Literal i64(final long value) { + return Expression.I64Literal.builder().value(value).build(); + } + + /** + * Creates a 32-bit floating point literal expression. + * + * @param value the float value + * @return a new {@link Expression.FP32Literal} + */ + public Expression.FP32Literal fp32(final float value) { + return Expression.FP32Literal.builder().value(value).build(); } /** * Creates a 64-bit floating point literal expression. * - * @param v the double value + * @param value the double value * @return a new {@link Expression.FP64Literal} */ - public Expression.FP64Literal fp64(double v) { - return Expression.FP64Literal.builder().value(v).build(); + public Expression.FP64Literal fp64(final double value) { + return Expression.FP64Literal.builder().value(value).build(); } /** @@ -1439,6 +1523,79 @@ public Expression.ScalarFunctionInvocation or(Expression... args) { return scalarFn(DefaultExtensionCatalog.FUNCTIONS_BOOLEAN, "or:bool", outputType, args); } + /** + * Creates a logical NOT expression that negates a boolean expression. + * + *

This is a convenience method that wraps the boolean NOT function from the Substrait standard + * library. The result is nullable to handle NULL input values according to three-valued logic. + * + * @param expression the boolean expression to negate + * @return a scalar function invocation representing the logical NOT of the input expression + */ + public Expression not(final Expression expression) { + return this.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_BOOLEAN, + "not:bool", + TypeCreator.NULLABLE.BOOLEAN, + expression); + } + + /** + * Creates a null-check expression that tests whether an expression is null. + * + *

This is a convenience method that wraps the is_null function from the Substrait comparison + * function library. The function evaluates the input expression and returns true if it is null, + * false otherwise. This is commonly used in conditional logic and filtering operations. + * + *

The return type is always a required (non-nullable) boolean, as the null check itself always + * produces a definite true/false result. + * + * @param expression the expression to test for null + * @return a scalar function invocation that returns true if the expression is null, false + * otherwise + */ + public Expression isNull(final Expression expression) { + + final List args = new ArrayList<>(); + args.add(expression); + + return this.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_COMPARISON, + "is_null:any", + TypeCreator.REQUIRED.BOOLEAN, + args, + new ArrayList()); + } + + /** + * Creates a scalar function invocation with function options. + * + *

This method extends the base builder's functionality by supporting function options, which + * control function behavior (e.g., rounding modes, overflow handling). + * + * @param urn the extension URI (e.g., {@link DefaultExtensionCatalog#FUNCTIONS_STRING}) + * @param key the function signature (e.g., "substring:str_i32_i32") + * @param returnType the return type of the function + * @param args the function arguments + * @param optionsList the function options controlling behavior + * @return a scalar function invocation expression + */ + public Expression scalarFn( + final String urn, + final String key, + final Type returnType, + final List args, + final List optionsList) { + final SimpleExtension.ScalarFunctionVariant declaration = + extensions.getScalarFunction(SimpleExtension.FunctionAnchor.of(urn, key)); + return Expression.ScalarFunctionInvocation.builder() + .declaration(declaration) + .options(optionsList) + .outputType(returnType) + .arguments(args) + .build(); + } + /** * Creates a scalar function invocation with specified arguments. * @@ -1459,6 +1616,29 @@ public Expression.ScalarFunctionInvocation scalarFn( .build(); } + /** + * Creates a scalar function invocation with function options. + * + * @param urn the extension URI (e.g., {@link DefaultExtensionCatalog#FUNCTIONS_STRING}) + * @param key the function signature (e.g., "substring:str_i32_i32") + * @param returnType the return type of the function + * @param args the function arguments + * @return a scalar function invocation expression + */ + public Expression scalarFn( + final String urn, + final String key, + final Type returnType, + final List args) { + final SimpleExtension.ScalarFunctionVariant declaration = + extensions.getScalarFunction(SimpleExtension.FunctionAnchor.of(urn, key)); + return Expression.ScalarFunctionInvocation.builder() + .declaration(declaration) + .outputType(returnType) + .arguments(args) + .build(); + } + /** * Creates a window function invocation with specified arguments and window bounds. * @@ -1549,6 +1729,22 @@ public Plan plan(Plan.Root root) { return Plan.builder().addRoots(root).build(); } + /** + * Creates a Plan.Root, which is the top-level container for a Substrait query plan. + * + *

The {@link Plan} wraps a relational expression tree and associates output column names with + * the plan. This is the final step in building a complete Substrait plan that can be serialized + * and executed by a Substrait consumer. + * + * @param input the root relational expression of the query plan + * @param names the ordered list of output column names corresponding to the input relation's + * output schema + * @return a new {@link Plan} + */ + public Plan.Root root(final Rel input, final List names) { + return Plan.Root.builder().input(input).names(names).build(); + } + /** * Creates a field remapping specification from field indexes. * diff --git a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java new file mode 100644 index 000000000..2a199de03 --- /dev/null +++ b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java @@ -0,0 +1,265 @@ +package io.substrait.dsl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.substrait.TestBase; +import io.substrait.expression.AggregateFunctionInvocation; +import io.substrait.expression.Expression; +import io.substrait.expression.FieldReference; +import io.substrait.extension.SimpleExtension; +import io.substrait.plan.Plan; +import io.substrait.relation.AbstractWriteRel; +import io.substrait.relation.Aggregate; +import io.substrait.relation.Cross; +import io.substrait.relation.Fetch; +import io.substrait.relation.Filter; +import io.substrait.relation.Join; +import io.substrait.relation.NamedScan; +import io.substrait.relation.NamedWrite; +import io.substrait.relation.Project; +import io.substrait.relation.Rel.Remap; +import io.substrait.relation.Sort; +import io.substrait.type.Type; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +class SubstraitBuilderTest extends TestBase { + + private SubstraitBuilder builder; + + @BeforeEach + void newbuilder() { + this.builder = new SubstraitBuilder(); + } + + @Test + void basicCreation() { + assertNotNull(this.builder); + assertInstanceOf(SimpleExtension.ExtensionCollection.class, this.builder.getExtensions()); + } + + @Nested + @DisplayName("Literal and Expression Tests") + class ExpressionTests { + + @Test + void testLiterals() { + assertEquals(true, builder.bool(true).value()); + assertEquals(10, builder.i8(10).value()); + assertEquals(100, builder.i16(100).value()); + assertEquals(1000, builder.i32(1000).value()); + assertEquals(10_000L, builder.i64(10_000L).value()); + assertEquals(1.5f, builder.fp32(1.5f).value()); + assertEquals(2.5, builder.fp64(2.5).value()); + assertEquals("foo", builder.str("foo").value()); + } + + @Test + void testFieldReferences() { + final NamedScan scan = createSimpleScan(); + final FieldReference ref = builder.fieldReference(scan, 0); + assertNotNull(ref); + + final List refs = builder.fieldReferences(scan, 0, 1); + assertEquals(2, refs.size()); + } + + @Test + void testCast() { + final Expression.I32Literal input = builder.i32(1); + final Expression cast = builder.cast(input, R.I64); + assertNotNull(cast); + assertTrue(cast instanceof Expression.Cast); + } + } + + @Nested + @DisplayName("Relation Building Tests") + class RelationTests { + + @Test + void testNamedScan() { + final NamedScan scan = + builder.namedScan(List.of("table"), List.of("c1", "c2"), List.of(R.I32, R.STRING)); + assertNotNull(scan); + assertEquals(List.of("table"), scan.getNames()); + } + + @Test + void testProject() { + final NamedScan scan = createSimpleScan(); + final Project project = + builder.project(rel -> List.of(builder.i32(1), builder.fieldReference(rel, 0)), scan); + assertNotNull(project); + assertEquals(scan, project.getInput()); + } + + @Test + void testFilter() { + final NamedScan scan = createSimpleScan(); + final Filter filter = + builder.filter( + rel -> builder.equal(builder.fieldReference(rel, 0), builder.i32(10)), scan); + assertNotNull(filter); + assertNotNull(filter.getCondition()); + } + + @Test + void testInnerJoin() { + final NamedScan left = createSimpleScan(); + final NamedScan right = createSimpleScan(); + final Join join = + builder.innerJoin( + inputs -> + builder.equal( + builder.fieldReference(inputs.left(), 0), + builder.fieldReference(inputs.right(), 0)), + left, + right); + assertNotNull(join); + assertEquals(Join.JoinType.INNER, join.getJoinType()); + } + + @Test + void testFetchAndLimit() { + final NamedScan scan = createSimpleScan(); + Fetch limit = builder.limit(10, scan); + assertEquals(10, limit.getCount().getAsLong()); + assertEquals(0, limit.getOffset()); + limit = builder.limit(10, Remap.of(Arrays.asList(new Integer[] {0, 1})), scan); + assertNotNull(limit); + + Fetch offset = builder.offset(5, scan); + assertEquals(5, offset.getOffset()); + + offset = builder.offset(5, Remap.of(Arrays.asList(new Integer[] {0, 1})), scan); + assertEquals(5, offset.getOffset()); + } + + @Test + void testSort() { + final NamedScan scan = createSimpleScan(); + Sort sort = builder.sort(rel -> builder.sortFields(rel, 0), scan); + assertNotNull(sort); + sort = + builder.sort( + rel -> builder.sortFields(rel, 0), + Remap.of(Arrays.asList(new Integer[] {0, 1})), + scan); + assertNotNull(sort); + } + + @Test + void testCross() { + final NamedScan left = createSimpleScan(); + final NamedScan right = createSimpleScan(); + + Cross cross = builder.cross(left, right); + assertNotNull(cross); + + cross = builder.cross(left, right, Remap.of(Arrays.asList(new Integer[] {0, 1}))); + assertNotNull(cross); + } + + @Test + void testFetch() { + final NamedScan left = createSimpleScan(); + Fetch fetch = builder.fetch(0, 1, left); + assertNotNull(fetch); + + fetch = builder.fetch(0, 1, Remap.of(Arrays.asList(new Integer[] {0, 1})), left); + assertNotNull(fetch); + } + } + + @Nested + @DisplayName("Aggregate and Scalar Function Tests") + class FunctionTests { + + @Test + void testAggregateMeasures() { + final NamedScan scan = createSimpleScan(); + final Aggregate.Measure count = builder.count(scan, 0); + final Aggregate.Measure sum = builder.sum(builder.fieldReference(scan, 0)); + + assertNotNull(count); + assertNotNull(sum); + + final Aggregate.Grouping grouping = builder.grouping(scan, 1); + assertNotNull(grouping); + assertEquals(1, grouping.getExpressions().size()); + + final AggregateFunctionInvocation afi1 = + builder.aggregateFn( + "extension:io.substrait:functions_aggregate_generic", + "count:any", + Type.I32.builder().nullable(false).build(), + builder.fieldReference(scan, 0)); + assertNotNull(afi1); + } + + @Test + void testArithmeticFunctions() { + final Expression left = builder.i32(10); + final Expression right = builder.i32(20); + + assertNotNull(builder.add(left, right)); + assertNotNull(builder.subtract(left, right)); + assertNotNull(builder.multiply(left, right)); + assertNotNull(builder.divide(left, right)); + assertNotNull(builder.negate(left)); + } + + @Test + void testBooleanLogic() { + final Expression b1 = builder.bool(true); + final Expression b2 = builder.bool(false); + + assertNotNull(builder.and(b1, b2)); + assertNotNull(builder.or(b1, b2)); + assertNotNull(builder.not(b1)); + assertNotNull(builder.isNull(b1)); + } + } + + @Nested + @DisplayName("Write and Plan Tests") + class PlanTests { + + @Test + void testNamedWrite() { + final NamedScan scan = createSimpleScan(); + final NamedWrite write = + builder.namedWrite( + List.of("target_table"), + List.of("c1", "c2"), + AbstractWriteRel.WriteOp.INSERT, + AbstractWriteRel.CreateMode.APPEND_IF_EXISTS, + AbstractWriteRel.OutputMode.MODIFIED_RECORDS, + scan); + assertNotNull(write); + } + + @Test + void testPlanCreation() { + final NamedScan scan = createSimpleScan(); + final Plan.Root root = builder.root(scan, List.of("out1", "out2")); + final Plan plan = builder.plan(root); + + assertNotNull(plan); + assertEquals(1, plan.getRoots().size()); + } + } + + // Helper method to create a base relation for testing + private NamedScan createSimpleScan() { + return builder.namedScan(List.of("t"), List.of("a", "b"), List.of(R.I32, R.STRING)); + } +} From fc8b69cbd03a8cf9323a6316a39b12a8bdb5e92d Mon Sep 17 00:00:00 2001 From: MBWhite Date: Tue, 24 Mar 2026 13:06:35 +0000 Subject: [PATCH 03/10] review comments Signed-off-by: MBWhite --- .../main/java/io/substrait/dsl/SubstraitBuilder.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index ba9694652..d3aaa6cc8 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -920,11 +920,11 @@ public Expression.I16Literal i16(final int value) { /** * Creates a 32-bit integer literal expression. * - * @param value the integer value + * @param v the integer value * @return a new {@link Expression.I32Literal} */ - public Expression.I32Literal i32(final int value) { - return Expression.I32Literal.builder().value(value).build(); + public Expression.I32Literal i32(int v) { + return Expression.I32Literal.builder().value(v).build(); } /** @@ -950,11 +950,11 @@ public Expression.FP32Literal fp32(final float value) { /** * Creates a 64-bit floating point literal expression. * - * @param value the double value + * @param v the double value * @return a new {@link Expression.FP64Literal} */ - public Expression.FP64Literal fp64(final double value) { - return Expression.FP64Literal.builder().value(value).build(); + public Expression.FP64Literal fp64(double v) { + return Expression.FP64Literal.builder().value(v).build(); } /** From 36b432f8702ee04834f4827df079134504a500e3 Mon Sep 17 00:00:00 2001 From: MBWhite Date: Wed, 25 Mar 2026 09:17:37 +0000 Subject: [PATCH 04/10] review comments Signed-off-by: MBWhite --- .../io/substrait/dsl/SubstraitBuilder.java | 69 ++++++------------- .../SubstraitRelNodeConverterTest.java | 5 +- 2 files changed, 25 insertions(+), 49 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index d3aaa6cc8..d6c623cf2 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -97,12 +97,9 @@ public SubstraitBuilder(SimpleExtension.ExtensionCollection extensions) { } /** - * Gets the default extension collection used by this builder. + * Gets the extension collection used by this builder. * - *

This collection includes standard Substrait functions for strings, arithmetic, comparison, - * datetime, and other operations from {@link DefaultExtensionCatalog#DEFAULT_COLLECTION}. - * - * @return the ExtensionCollection containing standard Substrait functions + * @return the ExtensionCollection used with this builder */ public SimpleExtension.ExtensionCollection getExtensions() { return extensions; @@ -148,25 +145,6 @@ public Aggregate aggregate( return aggregate(groupingsFn, measuresFn, Optional.empty(), input); } - /** - * Creates an aggregate relation with a single grouping and output field remapping. - * - * @param groupingFn function to derive the grouping from the input relation - * @param measuresFn function to derive the measures from the input relation - * @param remap the output field remapping specification - * @param input the input relation to aggregate - * @return a new {@link Aggregate} relation - */ - public Aggregate aggregate( - Function groupingFn, - Function> measuresFn, - Rel.Remap remap, - Rel input) { - Function> groupingsFn = - groupingFn.andThen(g -> Stream.of(g).collect(Collectors.toList())); - return aggregate(groupingsFn, measuresFn, Optional.of(remap), input); - } - /** * Creates an aggregate relation that groups and aggregates data from an input relation. * @@ -187,12 +165,12 @@ public Aggregate aggregate( * @return an Aggregate relation representing the grouping and aggregation operation */ public Aggregate aggregate( - final Function> groupingsFn, - final Function> measuresFn, - final Optional remap, - final Rel input) { - final List groupings = groupingsFn.apply(input); - final List measures = measuresFn.apply(input); + Function> groupingsFn, + Function> measuresFn, + Optional remap, + Rel input) { + List groupings = groupingsFn.apply(input); + List measures = measuresFn.apply(input); return Aggregate.builder() .groupings(groupings) .measures(measures) @@ -903,7 +881,7 @@ public Expression.BoolLiteral bool(boolean v) { * @param value value to create * @return i16 instance */ - public Expression.I8Literal i8(final int value) { + public Expression.I8Literal i8(int value) { return Expression.I8Literal.builder().value(value).build(); } @@ -913,7 +891,7 @@ public Expression.I8Literal i8(final int value) { * @param value value to create * @return i16 instance */ - public Expression.I16Literal i16(final int value) { + public Expression.I16Literal i16(int value) { return Expression.I16Literal.builder().value(value).build(); } @@ -933,7 +911,7 @@ public Expression.I32Literal i32(int v) { * @param value value to create * @return i64 instance */ - public Expression.I64Literal i64(final long value) { + public Expression.I64Literal i64(long value) { return Expression.I64Literal.builder().value(value).build(); } @@ -943,7 +921,7 @@ public Expression.I64Literal i64(final long value) { * @param value the float value * @return a new {@link Expression.FP32Literal} */ - public Expression.FP32Literal fp32(final float value) { + public Expression.FP32Literal fp32(float value) { return Expression.FP32Literal.builder().value(value).build(); } @@ -1532,7 +1510,7 @@ public Expression.ScalarFunctionInvocation or(Expression... args) { * @param expression the boolean expression to negate * @return a scalar function invocation representing the logical NOT of the input expression */ - public Expression not(final Expression expression) { + public Expression not(Expression expression) { return this.scalarFn( DefaultExtensionCatalog.FUNCTIONS_BOOLEAN, "not:bool", @@ -1554,7 +1532,7 @@ public Expression not(final Expression expression) { * @return a scalar function invocation that returns true if the expression is null, false * otherwise */ - public Expression isNull(final Expression expression) { + public Expression isNull(Expression expression) { final List args = new ArrayList<>(); args.add(expression); @@ -1581,12 +1559,12 @@ public Expression isNull(final Expression expression) { * @return a scalar function invocation expression */ public Expression scalarFn( - final String urn, - final String key, - final Type returnType, - final List args, - final List optionsList) { - final SimpleExtension.ScalarFunctionVariant declaration = + String urn, + String key, + Type returnType, + List args, + List optionsList) { + SimpleExtension.ScalarFunctionVariant declaration = extensions.getScalarFunction(SimpleExtension.FunctionAnchor.of(urn, key)); return Expression.ScalarFunctionInvocation.builder() .declaration(declaration) @@ -1626,11 +1604,8 @@ public Expression.ScalarFunctionInvocation scalarFn( * @return a scalar function invocation expression */ public Expression scalarFn( - final String urn, - final String key, - final Type returnType, - final List args) { - final SimpleExtension.ScalarFunctionVariant declaration = + String urn, String key, Type returnType, List args) { + SimpleExtension.ScalarFunctionVariant declaration = extensions.getScalarFunction(SimpleExtension.FunctionAnchor.of(urn, key)); return Expression.ScalarFunctionInvocation.builder() .declaration(declaration) diff --git a/isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java b/isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java index e0096afc4..7ac6f9f66 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java @@ -6,6 +6,7 @@ import io.substrait.relation.Set.SetOp; import io.substrait.type.Type; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.calcite.rel.RelNode; @@ -42,9 +43,9 @@ void emit() { Plan.Root root = sb.root( sb.aggregate( - input -> sb.grouping(input, 0, 2), + input -> List.of(sb.grouping(input, 0, 2)), input -> List.of(sb.count(input, 0)), - sb.remap(1, 2), + Optional.of(sb.remap(1, 2)), commonTable)); RelNode relNode = substraitToCalcite.convert(root.getInput()); From fe2e7aedcffef9ede067343d21e7d164d022043a Mon Sep 17 00:00:00 2001 From: matthew brian white Date: Fri, 5 Jun 2026 11:35:37 +0100 Subject: [PATCH 05/10] review comments Signed-off-by: matthew brian white --- .../io/substrait/dsl/SubstraitBuilder.java | 99 +++++++------------ .../substrait/dsl/SubstraitBuilderTest.java | 23 ++--- 2 files changed, 45 insertions(+), 77 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index d6c623cf2..3a9e26d0e 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -868,7 +868,7 @@ private Sort sort( /** * Creates a boolean literal expression. * - * @param v the boolean value + * @param v the boolean v * @return a new {@link Expression.BoolLiteral} */ public Expression.BoolLiteral bool(boolean v) { @@ -876,29 +876,29 @@ public Expression.BoolLiteral bool(boolean v) { } /** - * Create i16 literal. + * Create i8 literal. * - * @param value value to create - * @return i16 instance + * @param v v to create + * @return i8 instance */ - public Expression.I8Literal i8(int value) { - return Expression.I8Literal.builder().value(value).build(); + public Expression.I8Literal i8(int v) { + return Expression.I8Literal.builder().value(v).build(); } /** * Create i16 literal. * - * @param value value to create + * @param v v to create * @return i16 instance */ - public Expression.I16Literal i16(int value) { - return Expression.I16Literal.builder().value(value).build(); + public Expression.I16Literal i16(int v) { + return Expression.I16Literal.builder().value(v).build(); } /** * Creates a 32-bit integer literal expression. * - * @param v the integer value + * @param v the integer v * @return a new {@link Expression.I32Literal} */ public Expression.I32Literal i32(int v) { @@ -908,27 +908,27 @@ public Expression.I32Literal i32(int v) { /** * Creates a 64-bit integer literal expression. * - * @param value value to create + * @param v v to create * @return i64 instance */ - public Expression.I64Literal i64(long value) { - return Expression.I64Literal.builder().value(value).build(); + public Expression.I64Literal i64(long v) { + return Expression.I64Literal.builder().value(v).build(); } /** * Creates a 32-bit floating point literal expression. * - * @param value the float value + * @param v the float v * @return a new {@link Expression.FP32Literal} */ - public Expression.FP32Literal fp32(float value) { - return Expression.FP32Literal.builder().value(value).build(); + public Expression.FP32Literal fp32(float v) { + return Expression.FP32Literal.builder().value(v).build(); } /** * Creates a 64-bit floating point literal expression. * - * @param v the double value + * @param v the double v * @return a new {@link Expression.FP64Literal} */ public Expression.FP64Literal fp64(double v) { @@ -938,7 +938,7 @@ public Expression.FP64Literal fp64(double v) { /** * Creates a string literal expression. * - * @param s the string value + * @param s the string v * @return a new {@link Expression.StrLiteral} */ public Expression.StrLiteral str(String s) { @@ -1070,7 +1070,7 @@ public IfClause ifClause(Expression condition, Expression then) { * options. * * @param condition the expression to match against - * @param options the list of possible values to match + * @param options the list of possible vs to match * @return a new {@link SingleOrList} expression */ public Expression singleOrList(Expression condition, Expression... options) { @@ -1078,11 +1078,10 @@ public Expression singleOrList(Expression condition, Expression... options) { } /** - * Creates an IN predicate expression that checks if any needle values exist in the haystack - * relation. + * Creates an IN predicate expression that checks if any needle vs exist in the haystack relation. * * @param haystack the relation to search within - * @param needles the values to search for + * @param needles the vs to search for * @return a new {@link Expression.InPredicate} */ public Expression.InPredicate inPredicate(Rel haystack, Expression... needles) { @@ -1125,7 +1124,7 @@ public Expression.SortField sortField( /** * Creates a switch clause that pairs a literal condition with a result expression. * - * @param condition the literal value to match against + * @param condition the literal v to match against * @param then the expression to return if the condition matches * @return a new {@link SwitchClause} */ @@ -1134,7 +1133,7 @@ public SwitchClause switchClause(Expression.Literal condition, Expression then) } /** - * Creates a switch expression that matches a value against multiple cases. + * Creates a switch expression that matches a v against multiple cases. * * @param match the expression to match against * @param clauses the list of switch clauses to evaluate @@ -1241,7 +1240,7 @@ public Measure countStar() { * Creates a MIN aggregate measure for a specific field. * * @param input the input relation - * @param field the zero-based index of the field to find the minimum value + * @param field the zero-based index of the field to find the minimum v * @return a new {@link Aggregate.Measure} representing MIN */ public Aggregate.Measure min(Rel input, int field) { @@ -1251,7 +1250,7 @@ public Aggregate.Measure min(Rel input, int field) { /** * Creates a MIN aggregate measure for an expression. * - * @param expr the expression to find the minimum value + * @param expr the expression to find the minimum v * @return a new {@link Aggregate.Measure} representing MIN */ public Aggregate.Measure min(Expression expr) { @@ -1266,7 +1265,7 @@ public Aggregate.Measure min(Expression expr) { * Creates a MAX aggregate measure for a specific field. * * @param input the input relation - * @param field the zero-based index of the field to find the maximum value + * @param field the zero-based index of the field to find the maximum v * @return a new {@link Aggregate.Measure} representing MAX */ public Aggregate.Measure max(Rel input, int field) { @@ -1276,7 +1275,7 @@ public Aggregate.Measure max(Rel input, int field) { /** * Creates a MAX aggregate measure for an expression. * - * @param expr the expression to find the maximum value + * @param expr the expression to find the maximum v * @return a new {@link Aggregate.Measure} representing MAX */ public Aggregate.Measure max(Expression expr) { @@ -1505,17 +1504,16 @@ public Expression.ScalarFunctionInvocation or(Expression... args) { * Creates a logical NOT expression that negates a boolean expression. * *

This is a convenience method that wraps the boolean NOT function from the Substrait standard - * library. The result is nullable to handle NULL input values according to three-valued logic. + * library. The result is nullable to handle NULL input vs according to three-vd logic. * * @param expression the boolean expression to negate * @return a scalar function invocation representing the logical NOT of the input expression */ - public Expression not(Expression expression) { + public Expression.ScalarFunctionInvocation not(Expression expression) { + Type outputType = expression.getType().nullable() ? N.BOOLEAN : R.BOOLEAN; + return this.scalarFn( - DefaultExtensionCatalog.FUNCTIONS_BOOLEAN, - "not:bool", - TypeCreator.NULLABLE.BOOLEAN, - expression); + DefaultExtensionCatalog.FUNCTIONS_BOOLEAN, "not:bool", outputType, expression); } /** @@ -1534,14 +1532,11 @@ public Expression not(Expression expression) { */ public Expression isNull(Expression expression) { - final List args = new ArrayList<>(); - args.add(expression); - return this.scalarFn( DefaultExtensionCatalog.FUNCTIONS_COMPARISON, "is_null:any", TypeCreator.REQUIRED.BOOLEAN, - args, + Collections.singletonList(expression), new ArrayList()); } @@ -1558,7 +1553,7 @@ public Expression isNull(Expression expression) { * @param optionsList the function options controlling behavior * @return a scalar function invocation expression */ - public Expression scalarFn( + public Expression.ScalarFunctionInvocation scalarFn( String urn, String key, Type returnType, @@ -1594,26 +1589,6 @@ public Expression.ScalarFunctionInvocation scalarFn( .build(); } - /** - * Creates a scalar function invocation with function options. - * - * @param urn the extension URI (e.g., {@link DefaultExtensionCatalog#FUNCTIONS_STRING}) - * @param key the function signature (e.g., "substring:str_i32_i32") - * @param returnType the return type of the function - * @param args the function arguments - * @return a scalar function invocation expression - */ - public Expression scalarFn( - String urn, String key, Type returnType, List args) { - SimpleExtension.ScalarFunctionVariant declaration = - extensions.getScalarFunction(SimpleExtension.FunctionAnchor.of(urn, key)); - return Expression.ScalarFunctionInvocation.builder() - .declaration(declaration) - .outputType(returnType) - .arguments(args) - .build(); - } - /** * Creates a window function invocation with specified arguments and window bounds. * @@ -1698,7 +1673,7 @@ public Plan.Root root(Rel rel) { * Creates a plan from a plan root. * * @param root the plan root - * @return a new {@link Plan} + * @return a new {@link Plan.Root} */ public Plan plan(Plan.Root root) { return Plan.builder().addRoots(root).build(); @@ -1731,10 +1706,10 @@ public Rel.Remap remap(Integer... fields) { } /** - * Creates a scalar subquery expression that returns a single value from a relation. + * Creates a scalar subquery expression that returns a single v from a relation. * * @param input the input relation that must return exactly one row and one column - * @param type the type of the scalar value + * @param type the type of the scalar v * @return a new {@link Expression.ScalarSubquery} */ public Expression scalarSubquery(Rel input, Type type) { diff --git a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java index 2a199de03..90ecc6f69 100644 --- a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java +++ b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java @@ -3,12 +3,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import io.substrait.TestBase; import io.substrait.expression.AggregateFunctionInvocation; import io.substrait.expression.Expression; import io.substrait.expression.FieldReference; +import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.extension.SimpleExtension; import io.substrait.plan.Plan; import io.substrait.relation.AbstractWriteRel; @@ -23,7 +23,6 @@ import io.substrait.relation.Rel.Remap; import io.substrait.relation.Sort; import io.substrait.type.Type; -import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -74,9 +73,7 @@ void testFieldReferences() { @Test void testCast() { final Expression.I32Literal input = builder.i32(1); - final Expression cast = builder.cast(input, R.I64); - assertNotNull(cast); - assertTrue(cast instanceof Expression.Cast); + assertEquals(R.I32, input.getType()); } } @@ -133,13 +130,13 @@ void testFetchAndLimit() { Fetch limit = builder.limit(10, scan); assertEquals(10, limit.getCount().getAsLong()); assertEquals(0, limit.getOffset()); - limit = builder.limit(10, Remap.of(Arrays.asList(new Integer[] {0, 1})), scan); + limit = builder.limit(10, Remap.of(List.of(0, 1)), scan); assertNotNull(limit); Fetch offset = builder.offset(5, scan); assertEquals(5, offset.getOffset()); - offset = builder.offset(5, Remap.of(Arrays.asList(new Integer[] {0, 1})), scan); + offset = builder.offset(5, Remap.of(List.of(0, 1)), scan); assertEquals(5, offset.getOffset()); } @@ -148,11 +145,7 @@ void testSort() { final NamedScan scan = createSimpleScan(); Sort sort = builder.sort(rel -> builder.sortFields(rel, 0), scan); assertNotNull(sort); - sort = - builder.sort( - rel -> builder.sortFields(rel, 0), - Remap.of(Arrays.asList(new Integer[] {0, 1})), - scan); + sort = builder.sort(rel -> builder.sortFields(rel, 0), Remap.of(List.of(0, 1)), scan); assertNotNull(sort); } @@ -164,7 +157,7 @@ void testCross() { Cross cross = builder.cross(left, right); assertNotNull(cross); - cross = builder.cross(left, right, Remap.of(Arrays.asList(new Integer[] {0, 1}))); + cross = builder.cross(left, right, Remap.of(List.of(0, 1))); assertNotNull(cross); } @@ -174,7 +167,7 @@ void testFetch() { Fetch fetch = builder.fetch(0, 1, left); assertNotNull(fetch); - fetch = builder.fetch(0, 1, Remap.of(Arrays.asList(new Integer[] {0, 1})), left); + fetch = builder.fetch(0, 1, Remap.of(List.of(0, 1)), left); assertNotNull(fetch); } } @@ -198,7 +191,7 @@ void testAggregateMeasures() { final AggregateFunctionInvocation afi1 = builder.aggregateFn( - "extension:io.substrait:functions_aggregate_generic", + DefaultExtensionCatalog.FUNCTIONS_AGGREGATE_GENERIC, "count:any", Type.I32.builder().nullable(false).build(), builder.fieldReference(scan, 0)); From c0556e11d2dc12a8943e036a18e74971073a2585 Mon Sep 17 00:00:00 2001 From: matthew brian white Date: Mon, 8 Jun 2026 10:04:34 +0100 Subject: [PATCH 06/10] review comments Signed-off-by: matthew brian white --- .../io/substrait/dsl/SubstraitBuilder.java | 41 ++++++++----------- .../substrait/dsl/SubstraitBuilderTest.java | 5 +++ 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index 3a9e26d0e..fcacdd23c 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -153,16 +153,13 @@ public Aggregate aggregate( * while the measure function defines the aggregate computations (e.g., SUM, COUNT, AVG) to * perform on each group. * - *

The optional remap parameter allows reordering or filtering of output columns, which is - * useful for controlling the final schema of the aggregate result. - * * @param groupingsFn a function that takes the input relation and returns a list of grouping * expressions defining how to partition the data * @param measuresFn a function that takes the input relation and returns a list of aggregate * measures to compute for each group * @param remap an optional remapping specification to reorder or filter output columns * @param input the input relation to aggregate - * @return an Aggregate relation representing the grouping and aggregation operation + * @return an {@link Aggregate} relation representing the grouping and aggregation operation */ public Aggregate aggregate( Function> groupingsFn, @@ -868,7 +865,7 @@ private Sort sort( /** * Creates a boolean literal expression. * - * @param v the boolean v + * @param v the boolean value * @return a new {@link Expression.BoolLiteral} */ public Expression.BoolLiteral bool(boolean v) { @@ -878,7 +875,7 @@ public Expression.BoolLiteral bool(boolean v) { /** * Create i8 literal. * - * @param v v to create + * @param v value to create * @return i8 instance */ public Expression.I8Literal i8(int v) { @@ -888,7 +885,7 @@ public Expression.I8Literal i8(int v) { /** * Create i16 literal. * - * @param v v to create + * @param v value to create * @return i16 instance */ public Expression.I16Literal i16(int v) { @@ -898,7 +895,7 @@ public Expression.I16Literal i16(int v) { /** * Creates a 32-bit integer literal expression. * - * @param v the integer v + * @param v the integer value * @return a new {@link Expression.I32Literal} */ public Expression.I32Literal i32(int v) { @@ -908,7 +905,7 @@ public Expression.I32Literal i32(int v) { /** * Creates a 64-bit integer literal expression. * - * @param v v to create + * @param v value to create * @return i64 instance */ public Expression.I64Literal i64(long v) { @@ -918,7 +915,7 @@ public Expression.I64Literal i64(long v) { /** * Creates a 32-bit floating point literal expression. * - * @param v the float v + * @param v the float value * @return a new {@link Expression.FP32Literal} */ public Expression.FP32Literal fp32(float v) { @@ -928,7 +925,7 @@ public Expression.FP32Literal fp32(float v) { /** * Creates a 64-bit floating point literal expression. * - * @param v the double v + * @param v the double value * @return a new {@link Expression.FP64Literal} */ public Expression.FP64Literal fp64(double v) { @@ -938,7 +935,7 @@ public Expression.FP64Literal fp64(double v) { /** * Creates a string literal expression. * - * @param s the string v + * @param s the string value * @return a new {@link Expression.StrLiteral} */ public Expression.StrLiteral str(String s) { @@ -1124,7 +1121,7 @@ public Expression.SortField sortField( /** * Creates a switch clause that pairs a literal condition with a result expression. * - * @param condition the literal v to match against + * @param condition the literal value to match against * @param then the expression to return if the condition matches * @return a new {@link SwitchClause} */ @@ -1133,7 +1130,7 @@ public SwitchClause switchClause(Expression.Literal condition, Expression then) } /** - * Creates a switch expression that matches a v against multiple cases. + * Creates a switch expression that matches a value against multiple cases. * * @param match the expression to match against * @param clauses the list of switch clauses to evaluate @@ -1240,7 +1237,7 @@ public Measure countStar() { * Creates a MIN aggregate measure for a specific field. * * @param input the input relation - * @param field the zero-based index of the field to find the minimum v + * @param field the zero-based index of the field to find the minimum value * @return a new {@link Aggregate.Measure} representing MIN */ public Aggregate.Measure min(Rel input, int field) { @@ -1250,7 +1247,7 @@ public Aggregate.Measure min(Rel input, int field) { /** * Creates a MIN aggregate measure for an expression. * - * @param expr the expression to find the minimum v + * @param expr the expression to find the minimum value * @return a new {@link Aggregate.Measure} representing MIN */ public Aggregate.Measure min(Expression expr) { @@ -1265,7 +1262,7 @@ public Aggregate.Measure min(Expression expr) { * Creates a MAX aggregate measure for a specific field. * * @param input the input relation - * @param field the zero-based index of the field to find the maximum v + * @param field the zero-based index of the field to find the maximum value * @return a new {@link Aggregate.Measure} representing MAX */ public Aggregate.Measure max(Rel input, int field) { @@ -1275,7 +1272,7 @@ public Aggregate.Measure max(Rel input, int field) { /** * Creates a MAX aggregate measure for an expression. * - * @param expr the expression to find the maximum v + * @param expr the expression to find the maximum value * @return a new {@link Aggregate.Measure} representing MAX */ public Aggregate.Measure max(Expression expr) { @@ -1519,10 +1516,6 @@ public Expression.ScalarFunctionInvocation not(Expression expression) { /** * Creates a null-check expression that tests whether an expression is null. * - *

This is a convenience method that wraps the is_null function from the Substrait comparison - * function library. The function evaluates the input expression and returns true if it is null, - * false otherwise. This is commonly used in conditional logic and filtering operations. - * *

The return type is always a required (non-nullable) boolean, as the null check itself always * produces a definite true/false result. * @@ -1706,10 +1699,10 @@ public Rel.Remap remap(Integer... fields) { } /** - * Creates a scalar subquery expression that returns a single v from a relation. + * Creates a scalar subquery expression that returns a single value from a relation. * * @param input the input relation that must return exactly one row and one column - * @param type the type of the scalar v + * @param type the type of the scalar value * @return a new {@link Expression.ScalarSubquery} */ public Expression scalarSubquery(Rel input, Type type) { diff --git a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java index 90ecc6f69..fcc9a910f 100644 --- a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java +++ b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java @@ -74,6 +74,11 @@ void testFieldReferences() { void testCast() { final Expression.I32Literal input = builder.i32(1); assertEquals(R.I32, input.getType()); + + final Expression cast = builder.cast(input, R.I64); + assertEquals(R.I64, cast.getType()); + assertInstanceOf(Expression.Cast.class, cast); + assertEquals(input, ((Expression.Cast) cast).input()); } } From 56f651c3e851dd0b5c3848a489d24578bbdeb468 Mon Sep 17 00:00:00 2001 From: matthew brian white Date: Mon, 8 Jun 2026 10:08:49 +0100 Subject: [PATCH 07/10] review comments Signed-off-by: matthew brian white --- core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java index fcc9a910f..bf76831d9 100644 --- a/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java +++ b/core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java @@ -74,7 +74,7 @@ void testFieldReferences() { void testCast() { final Expression.I32Literal input = builder.i32(1); assertEquals(R.I32, input.getType()); - + final Expression cast = builder.cast(input, R.I64); assertEquals(R.I64, cast.getType()); assertInstanceOf(Expression.Cast.class, cast); From 6692e5511fcd9eb35be776a9863440ed5d734b33 Mon Sep 17 00:00:00 2001 From: Matthew B White Date: Mon, 8 Jun 2026 13:40:47 +0100 Subject: [PATCH 08/10] Apply suggestion from @nielspardon Co-authored-by: Niels Pardon --- core/src/main/java/io/substrait/dsl/SubstraitBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index fcacdd23c..63b5f9a2d 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -873,7 +873,7 @@ public Expression.BoolLiteral bool(boolean v) { } /** - * Create i8 literal. + * Creates an 8-bit integer literal expression. * * @param v value to create * @return i8 instance From aeffaec5b729a1263f388598b55aa32fffea2c13 Mon Sep 17 00:00:00 2001 From: matthew brian white Date: Mon, 8 Jun 2026 13:46:05 +0100 Subject: [PATCH 09/10] review comments Signed-off-by: matthew brian white --- .../main/java/io/substrait/dsl/SubstraitBuilder.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index 63b5f9a2d..7a1ea9e66 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -883,7 +883,7 @@ public Expression.I8Literal i8(int v) { } /** - * Create i16 literal. + * Create a 16-bit integer literal expression. * * @param v value to create * @return i16 instance @@ -1067,7 +1067,7 @@ public IfClause ifClause(Expression condition, Expression then) { * options. * * @param condition the expression to match against - * @param options the list of possible vs to match + * @param options the list of possible values to match * @return a new {@link SingleOrList} expression */ public Expression singleOrList(Expression condition, Expression... options) { @@ -1075,10 +1075,10 @@ public Expression singleOrList(Expression condition, Expression... options) { } /** - * Creates an IN predicate expression that checks if any needle vs exist in the haystack relation. + * Creates an IN predicate expression that checks if any needle values exist in the haystack relation. * * @param haystack the relation to search within - * @param needles the vs to search for + * @param needles the values to search for * @return a new {@link Expression.InPredicate} */ public Expression.InPredicate inPredicate(Rel haystack, Expression... needles) { @@ -1501,7 +1501,7 @@ public Expression.ScalarFunctionInvocation or(Expression... args) { * Creates a logical NOT expression that negates a boolean expression. * *

This is a convenience method that wraps the boolean NOT function from the Substrait standard - * library. The result is nullable to handle NULL input vs according to three-vd logic. + * library. The result is nullable to handle NULL input values according to three-valued logic. * * @param expression the boolean expression to negate * @return a scalar function invocation representing the logical NOT of the input expression From 75a5722dd889b0c787e22cfb6e61813c2ae16379 Mon Sep 17 00:00:00 2001 From: matthew brian white Date: Mon, 8 Jun 2026 15:20:43 +0100 Subject: [PATCH 10/10] fix: spotless apply Signed-off-by: matthew brian white --- core/src/main/java/io/substrait/dsl/SubstraitBuilder.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java index 7a1ea9e66..5ff70c06d 100644 --- a/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java +++ b/core/src/main/java/io/substrait/dsl/SubstraitBuilder.java @@ -1075,7 +1075,8 @@ public Expression singleOrList(Expression condition, Expression... options) { } /** - * Creates an IN predicate expression that checks if any needle values exist in the haystack relation. + * Creates an IN predicate expression that checks if any needle values exist in the haystack + * relation. * * @param haystack the relation to search within * @param needles the values to search for @@ -1666,7 +1667,7 @@ public Plan.Root root(Rel rel) { * Creates a plan from a plan root. * * @param root the plan root - * @return a new {@link Plan.Root} + * @return a new {@link Plan} */ public Plan plan(Plan.Root root) { return Plan.builder().addRoots(root).build(); @@ -1682,7 +1683,7 @@ public Plan plan(Plan.Root root) { * @param input the root relational expression of the query plan * @param names the ordered list of output column names corresponding to the input relation's * output schema - * @return a new {@link Plan} + * @return a new {@link Plan.Root} */ public Plan.Root root(final Rel input, final List names) { return Plan.Root.builder().input(input).names(names).build();