From 91ff10d9f0eaf6237ca3ed797ea3716ee6a6e7eb Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Wed, 5 Aug 2026 14:02:29 +0200 Subject: [PATCH 1/2] feat(core)!: support nested map expressions and import nested structs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proto `Expression.Nested` has three arms — struct, list and map — but only `list` round-tripped. `Expression.NestedMap` did not exist at all, so the `map` arm was unreachable, and `ProtoExpressionConverter.from(Nested)` handled only `case LIST`: a plan carrying a nested struct failed on import with `UnsupportedOperationException: Unimplemented nested type: STRUCT`, even though the POJO to proto direction emitted one. (`VirtualTableScan` rows were unaffected — they use the separate `from(Nested.Struct)` overload.) Adds the `NestedMap` POJO with the usual visitor and converter wiring, and imports all three nested kinds on the way back from proto. A nested map holds its pairs as an ordered `List`, mirroring the proto `repeated KeyValue key_values`: a map expression may repeat a key, and a `Map`-based representation would silently drop one of the pairs on import. `NestedMap.check()` rejects an empty map and heterogeneous key or value types with `IllegalArgumentException`. Isthmus still cannot convert nested structs or maps to and from Calcite; that follows separately. Part of #375 BREAKING CHANGE: `ExpressionVisitor` gains `visit(Expression.NestedMap)`. Direct implementors must add it; implementors extending `AbstractExpressionVisitor` inherit the `visitFallback` default and need no change. --- .../expression/AbstractExpressionVisitor.java | 13 ++ .../io/substrait/expression/Expression.java | 98 +++++++++++++ .../expression/ExpressionCreator.java | 16 ++ .../expression/ExpressionVisitor.java | 10 ++ .../proto/ExpressionProtoConverter.java | 17 +++ .../proto/ProtoExpressionConverter.java | 20 ++- .../ExpressionCopyOnWriteVisitor.java | 21 +++ .../type/proto/NestedMapExpressionTest.java | 137 ++++++++++++++++++ .../proto/NestedStructExpressionTest.java | 68 +++++++++ .../examples/util/ExpressionStringify.java | 6 + 10 files changed, 404 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java create mode 100644 core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java diff --git a/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java b/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java index eeb3b0954..f39a5e164 100644 --- a/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java +++ b/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java @@ -513,6 +513,19 @@ public O visit(Expression.NestedList expr, C context) throws E { return visitFallback(expr, context); } + /** + * Visits a nested map expression. + * + * @param expr the nested map + * @param context the visitation context + * @return the visit result + * @throws E if visitation fails + */ + @Override + public O visit(Expression.NestedMap expr, C context) throws E { + return visitFallback(expr, context); + } + /** * Visits a field reference. * diff --git a/core/src/main/java/io/substrait/expression/Expression.java b/core/src/main/java/io/substrait/expression/Expression.java index e394682b0..3c22ef70d 100644 --- a/core/src/main/java/io/substrait/expression/Expression.java +++ b/core/src/main/java/io/substrait/expression/Expression.java @@ -1819,6 +1819,104 @@ public static ImmutableExpression.NestedList.Builder builder() { } } + /** + * A nested map expression with one or more key-value pairs. + * + *

The pairs are held as an ordered list rather than a {@code Map} because that is what the + * Substrait map expression is: a repeated list of key-value pairs. Two pairs may carry equal + * keys, and a {@code Map} would silently drop one of them. + * + *

Note: This class cannot be used to construct an empty map. To create an empty map, use + * {@link ExpressionCreator#emptyMap(boolean, Type, Type)} which returns an {@link + * EmptyMapLiteral}. + */ + @Value.Immutable + abstract class NestedMap implements Nested { + /** + * Returns the key-value pairs in this nested map, in the order they were added. + * + * @return the key-value pairs + */ + public abstract List keyValues(); + + /** + * Validates that the nested map is not empty and that all keys, and all values, have a single + * common type. + */ + @Value.Check + protected void check() { + if (keyValues().isEmpty()) { + throw new IllegalArgumentException( + "To specify an empty map, use ExpressionCreator.emptyMap"); + } + if (keyValues().stream().map(keyValue -> keyValue.key().getType()).distinct().count() > 1) { + throw new IllegalArgumentException("All keys in a NestedMap must have the same type"); + } + if (keyValues().stream().map(keyValue -> keyValue.value().getType()).distinct().count() > 1) { + throw new IllegalArgumentException("All values in a NestedMap must have the same type"); + } + } + + @Override + public Type getType() { + KeyValue first = keyValues().get(0); + return Type.withNullability(nullable()).map(first.key().getType(), first.value().getType()); + } + + @Override + public R accept( + ExpressionVisitor visitor, C context) throws E { + return visitor.visit(this, context); + } + + /** + * Creates a new builder for constructing a NestedMap. + * + * @return a new builder instance + */ + public static ImmutableExpression.NestedMap.Builder builder() { + return ImmutableExpression.NestedMap.builder(); + } + + /** A single key-value pair of a {@link NestedMap}. */ + @Value.Immutable + public abstract static class KeyValue { + /** + * Returns the key expression of this pair. + * + * @return the key + */ + public abstract Expression key(); + + /** + * Returns the value expression of this pair. + * + * @return the value + */ + public abstract Expression value(); + + /** + * Creates a key-value pair. + * + * @param key the key expression + * @param value the value expression + * @return the key-value pair + */ + public static KeyValue of(Expression key, Expression value) { + return builder().key(key).value(value).build(); + } + + /** + * Creates a new builder for constructing a KeyValue. + * + * @return a new builder instance + */ + public static ImmutableExpression.KeyValue.Builder builder() { + return ImmutableExpression.KeyValue.builder(); + } + } + } + /** Represents a single record (combination of values) in a multi-or-list expression. */ @Value.Immutable abstract class MultiOrListRecord { diff --git a/core/src/main/java/io/substrait/expression/ExpressionCreator.java b/core/src/main/java/io/substrait/expression/ExpressionCreator.java index 714b741be..bd4e819c4 100644 --- a/core/src/main/java/io/substrait/expression/ExpressionCreator.java +++ b/core/src/main/java/io/substrait/expression/ExpressionCreator.java @@ -571,6 +571,22 @@ public static Expression.NestedStruct nestedStruct(boolean nullable, Expression. return Expression.NestedStruct.builder().nullable(nullable).addFields(fields).build(); } + /** + * Creates a nested map expression with one or more key-value pairs. + * + *

Note: This method cannot be used to construct an empty map. To create an empty map, use + * {@link ExpressionCreator#emptyMap(boolean, Type, Type)} which returns an {@link + * Expression.EmptyMapLiteral}. + * + * @param nullable whether the map can be null + * @param keyValues the key-value pairs in the nested map, in the order they should be serialized + * @return a NestedMap expression + */ + public static Expression.NestedMap nestedMap( + boolean nullable, List keyValues) { + return Expression.NestedMap.builder().nullable(nullable).addAllKeyValues(keyValues).build(); + } + /** * Create a UserDefinedAnyLiteral with google.protobuf.Any representation. * diff --git a/core/src/main/java/io/substrait/expression/ExpressionVisitor.java b/core/src/main/java/io/substrait/expression/ExpressionVisitor.java index f9a8f62ba..e9af20c44 100644 --- a/core/src/main/java/io/substrait/expression/ExpressionVisitor.java +++ b/core/src/main/java/io/substrait/expression/ExpressionVisitor.java @@ -401,6 +401,16 @@ public interface ExpressionVisitor { + Expression.Nested.Map.Builder mapBldr = Expression.Nested.Map.newBuilder(); + for (io.substrait.expression.Expression.NestedMap.KeyValue keyValue : expr.keyValues()) { + mapBldr.addKeyValues( + Expression.Nested.Map.KeyValue.newBuilder() + .setKey(toProto(keyValue.key())) + .setValue(toProto(keyValue.value()))); + } + bldr.setMap(mapBldr).setNullable(expr.nullable()); + }); + } + @Override public Expression visit(FieldReference expr, EmptyVisitationContext context) { diff --git a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java index 8c51027d4..d0cfbbd81 100644 --- a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java @@ -481,13 +481,29 @@ private WindowBound toWindowBound(io.substrait.proto.Expression.WindowFunction.B */ public Expression.Nested from(io.substrait.proto.Expression.Nested nested) { switch (nested.getNestedTypeCase()) { + case STRUCT: + List fields = + nested.getStruct().getFieldsList().stream() + .map(this::from) + .collect(Collectors.toList()); + return ExpressionCreator.nestedStruct(nested.getNullable(), fields); case LIST: List list = nested.getList().getValuesList().stream().map(this::from).collect(Collectors.toList()); return ExpressionCreator.nestedList(nested.getNullable(), list); + case MAP: + // The pairs are kept in a list, in the order the producer emitted them, so that a map + // repeating a key keeps both of its pairs. + List keyValues = + nested.getMap().getKeyValuesList().stream() + .map( + keyValue -> + Expression.NestedMap.KeyValue.of( + from(keyValue.getKey()), from(keyValue.getValue()))) + .collect(Collectors.toList()); + return ExpressionCreator.nestedMap(nested.getNullable(), keyValues); default: - throw new UnsupportedOperationException( - "Unimplemented nested type: " + nested.getNestedTypeCase()); + throw new IllegalStateException("Unexpected nested type: " + nested.getNestedTypeCase()); } } diff --git a/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java b/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java index 1a8923083..bc4d0228a 100644 --- a/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java +++ b/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java @@ -10,6 +10,7 @@ import io.substrait.expression.FunctionArg; import io.substrait.expression.ImmutableExpression; import io.substrait.util.EmptyVisitationContext; +import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -402,6 +403,26 @@ public Optional visit(Expression.NestedList expr, EmptyVisitationCon Expression.NestedList.builder().from(expr).values(expressionList).build()); } + @Override + public Optional visit(Expression.NestedMap expr, EmptyVisitationContext context) + throws E { + boolean changed = false; + List keyValues = new ArrayList<>(); + for (Expression.NestedMap.KeyValue keyValue : expr.keyValues()) { + Optional key = keyValue.key().accept(this, context); + Optional value = keyValue.value().accept(this, context); + changed |= !allEmpty(key, value); + keyValues.add( + Expression.NestedMap.KeyValue.of( + key.orElse(keyValue.key()), value.orElse(keyValue.value()))); + } + + if (!changed) { + return Optional.empty(); + } + return Optional.of(Expression.NestedMap.builder().from(expr).keyValues(keyValues).build()); + } + /** * Visits a multi-or-list record. * diff --git a/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java b/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java new file mode 100644 index 000000000..3b2a574be --- /dev/null +++ b/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java @@ -0,0 +1,137 @@ +package io.substrait.type.proto; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.substrait.TestBase; +import io.substrait.expression.Expression; +import io.substrait.expression.ImmutableExpression; +import io.substrait.relation.Project; +import java.util.List; +import org.junit.jupiter.api.Test; + +class NestedMapExpressionTest extends TestBase { + Expression literalExpression = Expression.BoolLiteral.builder().value(true).build(); + Expression.ScalarFunctionInvocation nonLiteralExpression = sb.add(sb.i32(7), sb.i32(42)); + + @Test + void rejectEmptyNestedMap() { + ImmutableExpression.NestedMap.Builder builder = Expression.NestedMap.builder(); + assertThrows(IllegalArgumentException.class, builder::build); + } + + @Test + void rejectNestedMapWithKeysOfDifferentTypes() { + ImmutableExpression.NestedMap.Builder builder = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), literalExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(1), literalExpression)); + assertThrows(IllegalArgumentException.class, builder::build); + } + + @Test + void rejectNestedMapWithValuesOfDifferentTypes() { + ImmutableExpression.NestedMap.Builder builder = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), literalExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("b"), sb.i32(1))); + assertThrows(IllegalArgumentException.class, builder::build); + } + + @Test + void acceptNestedMapWithKeysAndValuesOfSameType() { + ImmutableExpression.NestedMap.Builder builder = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), nonLiteralExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("b"), sb.i32(12))); + assertDoesNotThrow(builder::build); + + verifyRoundTrip(projectOf(builder.build())); + } + + @Test + void literalNestedMapTest() { + Expression.NestedMap literalNestedMap = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), literalExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("b"), literalExpression)) + .build(); + + verifyRoundTrip(projectOf(literalNestedMap)); + } + + @Test + void literalNullableNestedMapTest() { + Expression.NestedMap literalNestedMap = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), literalExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("b"), literalExpression)) + .nullable(true) + .build(); + + verifyRoundTrip(projectOf(literalNestedMap)); + } + + @Test + void nonLiteralNestedMapTest() { + Expression.NestedMap nonLiteralNestedMap = + Expression.NestedMap.builder() + .addKeyValues( + Expression.NestedMap.KeyValue.of(nonLiteralExpression, nonLiteralExpression)) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(12), sb.i32(13))) + .build(); + + verifyRoundTrip(projectOf(nonLiteralNestedMap)); + } + + @Test + void nestedMapOfNestedMapsTest() { + Expression.NestedMap inner = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), sb.i32(1))) + .build(); + + Expression.NestedMap outer = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("outer"), inner)) + .build(); + + verifyRoundTrip(projectOf(outer)); + } + + @Test + void repeatedKeysNestedMapTest() { + // A Substrait map expression is a repeated list of key-value pairs, so the same key may appear + // more than once. Both pairs have to survive a round trip. + Expression.NestedMap repeatedKeys = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(1), sb.i32(10))) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(1), sb.i32(20))) + .build(); + + assertEquals(2, repeatedKeys.keyValues().size()); + verifyRoundTrip(projectOf(repeatedKeys)); + } + + @Test + void keyValueOrderIsPreservedTest() { + // Keys deliberately out of natural order, so that a representation which reorders the pairs + // would fail here. + List keyValues = + List.of( + Expression.NestedMap.KeyValue.of(sb.str("zzz"), sb.i32(1)), + Expression.NestedMap.KeyValue.of(sb.str("aaa"), sb.i32(2)), + Expression.NestedMap.KeyValue.of(sb.str("mmm"), sb.i32(3))); + + Expression.NestedMap nestedMap = + Expression.NestedMap.builder().addAllKeyValues(keyValues).build(); + + assertEquals(keyValues, nestedMap.keyValues()); + verifyRoundTrip(projectOf(nestedMap)); + } + + private Project projectOf(Expression expression) { + return Project.builder().addExpressions(expression).input(sb.emptyVirtualTableScan()).build(); + } +} diff --git a/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java b/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java new file mode 100644 index 000000000..f64656909 --- /dev/null +++ b/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java @@ -0,0 +1,68 @@ +package io.substrait.type.proto; + +import io.substrait.TestBase; +import io.substrait.expression.Expression; +import io.substrait.relation.Project; +import org.junit.jupiter.api.Test; + +class NestedStructExpressionTest extends TestBase { + Expression literalExpression = Expression.BoolLiteral.builder().value(true).build(); + Expression.ScalarFunctionInvocation nonLiteralExpression = sb.add(sb.i32(7), sb.i32(42)); + + @Test + void emptyNestedStructTest() { + verifyRoundTrip(projectOf(Expression.NestedStruct.builder().build())); + } + + @Test + void literalNestedStructTest() { + Expression.NestedStruct literalNestedStruct = + Expression.NestedStruct.builder() + .addFields(literalExpression) + .addFields(sb.str("a")) + .build(); + + verifyRoundTrip(projectOf(literalNestedStruct)); + } + + @Test + void literalNullableNestedStructTest() { + Expression.NestedStruct literalNestedStruct = + Expression.NestedStruct.builder().addFields(literalExpression).nullable(true).build(); + + verifyRoundTrip(projectOf(literalNestedStruct)); + } + + @Test + void heterogeneouslyTypedNestedStructTest() { + Expression.NestedStruct nestedStruct = + Expression.NestedStruct.builder() + .addFields(nonLiteralExpression) + .addFields(sb.str("a")) + .addFields(literalExpression) + .build(); + + verifyRoundTrip(projectOf(nestedStruct)); + } + + @Test + void nestedStructOfNestedTypesTest() { + Expression.NestedStruct inner = + Expression.NestedStruct.builder().addFields(sb.i32(1)).nullable(true).build(); + Expression.NestedList list = + Expression.NestedList.builder().addValues(sb.i32(2)).addValues(sb.i32(3)).build(); + Expression.NestedMap map = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), sb.i32(4))) + .build(); + + Expression.NestedStruct outer = + Expression.NestedStruct.builder().addFields(inner).addFields(list).addFields(map).build(); + + verifyRoundTrip(projectOf(outer)); + } + + private Project projectOf(Expression expression) { + return Project.builder().addExpressions(expression).input(sb.emptyVirtualTableScan()).build(); + } +} diff --git a/examples/substrait-spark/src/main/java/io/substrait/examples/util/ExpressionStringify.java b/examples/substrait-spark/src/main/java/io/substrait/examples/util/ExpressionStringify.java index 07e09b821..dcf8fc8c3 100644 --- a/examples/substrait-spark/src/main/java/io/substrait/examples/util/ExpressionStringify.java +++ b/examples/substrait-spark/src/main/java/io/substrait/examples/util/ExpressionStringify.java @@ -274,6 +274,12 @@ public String visit(Expression.NestedList expr, EmptyVisitationContext context) return ""; } + @Override + public String visit(Expression.NestedMap expr, EmptyVisitationContext context) + throws RuntimeException { + return ""; + } + @Override public String visit(FieldReference expr, EmptyVisitationContext context) throws RuntimeException { StringBuilder sb = new StringBuilder("FieldRef#"); From 3db51b4e173d18af697fc5fc5f695faa8f649c21 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Thu, 13 Aug 2026 14:06:41 +0200 Subject: [PATCH 2/2] review: align nested list and map validation, typing and visitor hooks Addresses review feedback on the nested map expression change. `NestedList` and `NestedMap` now share one `commonType` helper, used by both `check()` and `getType()`. It compares element types with nullability disregarded, so a legal plan whose collection mixes nullable and non-nullable operands imports rather than throwing, and it returns the nullable union, so the reported element type no longer depends on the order of the values. The helper lives in a package-private class because `:core` main sources compile with `--release 8`, which has no private interface methods. The `@Value.Check` messages no longer tell the caller to use `ExpressionCreator.emptyList` / `emptyMap`, since the check also runs on the proto import path, where the caller is reading someone else's plan and has no such choice. On import, a map pair missing its key or its value is now rejected by name instead of failing later with `Unknown type: REXTYPE_NOT_SET`, and an `Expression.Nested` with no arm set throws `IllegalArgumentException` rather than `UnsupportedOperationException`. `ExpressionCopyOnWriteVisitor` gains an overridable `visitKeyValue` and delegates to `transformList`, matching every other composite in that class, so a subclass can rewrite pairs without re-implementing the copy logic. `ExpressionCreator.nestedMap` gains `Iterable` and varargs overloads to match `nestedStruct`, `ExpressionProtoConverter.visit(NestedMap)` streams into `addAllKeyValues` like the struct and list arms, and the `projectOf` test helper moves to `TestBase`, where all three nested expression tests use it. --- .../io/substrait/expression/Expression.java | 54 ++++++------ .../expression/ExpressionCreator.java | 20 ++++- .../expression/NestedExpressionUtils.java | 39 +++++++++ .../proto/ExpressionProtoConverter.java | 19 ++-- .../proto/ProtoExpressionConverter.java | 29 +++++-- .../ExpressionCopyOnWriteVisitor.java | 39 ++++++--- core/src/test/java/io/substrait/TestBase.java | 6 ++ .../ExpressionCopyOnWriteVisitorTest.java | 85 ++++++++++++++++++ .../type/proto/NestedListExpressionTest.java | 50 ++++------- .../type/proto/NestedMapExpressionTest.java | 86 ++++++++++++++++++- .../proto/NestedStructExpressionTest.java | 5 -- 11 files changed, 336 insertions(+), 96 deletions(-) create mode 100644 core/src/main/java/io/substrait/expression/NestedExpressionUtils.java create mode 100644 core/src/test/java/io/substrait/relation/ExpressionCopyOnWriteVisitorTest.java diff --git a/core/src/main/java/io/substrait/expression/Expression.java b/core/src/main/java/io/substrait/expression/Expression.java index 79df1a1bc..d990bd9ac 100644 --- a/core/src/main/java/io/substrait/expression/Expression.java +++ b/core/src/main/java/io/substrait/expression/Expression.java @@ -1,5 +1,7 @@ package io.substrait.expression; +import static io.substrait.expression.NestedExpressionUtils.commonType; + import com.google.protobuf.ByteString; import io.substrait.extension.SimpleExtension; import io.substrait.proto.AggregateFunction; @@ -11,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; import org.immutables.value.Value; /** @@ -1790,11 +1793,9 @@ abstract class NestedList implements Nested { public abstract List values(); /** - * Validates that the nested list is not empty and all values have the same type, disregarding - * nullability. Values of mixed nullability are allowed, because SQL list constructors do not - * cast their values to a common type: {@code ARRAY[not_null_column, nullable_column]} produces - * values that differ only in nullability. The list's element type is that of its first value, - * as {@link #getType()} shows. + * Validates that the nested list is not empty and that all values have the same type, + * disregarding nullability. Values of mixed nullability are allowed; the list's element type is + * then the nullable one, as {@link #getType()} shows. * * @throws IllegalArgumentException if the list is empty or its values have differing types */ @@ -1802,20 +1803,17 @@ abstract class NestedList implements Nested { protected void check() { if (values().isEmpty()) { throw new IllegalArgumentException( - "To specify an empty list, use ExpressionCreator.emptyList()"); - } - - List types = - values().stream().map(Expression::getType).collect(java.util.stream.Collectors.toList()); - if (types.stream().map(TypeCreator::asNullable).distinct().limit(2).count() > 1) { - throw new IllegalArgumentException( - String.format("All values in NestedList must have the same type, found: %s", types)); + "A nested list expression must have at least one value; an empty list is expressed as" + + " an empty list literal (see ExpressionCreator.emptyList)"); } + // Throws if the values have differing types. + getType(); } @Override public Type getType() { - return Type.withNullability(nullable()).list(values().get(0).getType()); + return Type.withNullability(nullable()) + .list(commonType("The values of a nested list expression", values())); } @Override @@ -1855,27 +1853,33 @@ abstract class NestedMap implements Nested { public abstract List keyValues(); /** - * Validates that the nested map is not empty and that all keys, and all values, have a single - * common type. + * Validates that the nested map is not empty and that all keys, and all values, have the same + * type, disregarding nullability. Keys or values of mixed nullability are allowed; the map's + * key or value type is then the nullable one, as {@link #getType()} shows. + * + * @throws IllegalArgumentException if the map is empty, its keys have differing types, or its + * values have differing types */ @Value.Check protected void check() { if (keyValues().isEmpty()) { throw new IllegalArgumentException( - "To specify an empty map, use ExpressionCreator.emptyMap"); - } - if (keyValues().stream().map(keyValue -> keyValue.key().getType()).distinct().count() > 1) { - throw new IllegalArgumentException("All keys in a NestedMap must have the same type"); - } - if (keyValues().stream().map(keyValue -> keyValue.value().getType()).distinct().count() > 1) { - throw new IllegalArgumentException("All values in a NestedMap must have the same type"); + "A nested map expression must have at least one key-value pair; an empty map is" + + " expressed as an empty map literal (see ExpressionCreator.emptyMap)"); } + // Throws if the keys, or the values, have differing types. + getType(); } @Override public Type getType() { - KeyValue first = keyValues().get(0); - return Type.withNullability(nullable()).map(first.key().getType(), first.value().getType()); + List keys = keyValues().stream().map(KeyValue::key).collect(Collectors.toList()); + List values = + keyValues().stream().map(KeyValue::value).collect(Collectors.toList()); + return Type.withNullability(nullable()) + .map( + commonType("The keys of a nested map expression", keys), + commonType("The values of a nested map expression", values)); } @Override diff --git a/core/src/main/java/io/substrait/expression/ExpressionCreator.java b/core/src/main/java/io/substrait/expression/ExpressionCreator.java index bd4e819c4..e347f6819 100644 --- a/core/src/main/java/io/substrait/expression/ExpressionCreator.java +++ b/core/src/main/java/io/substrait/expression/ExpressionCreator.java @@ -572,7 +572,7 @@ public static Expression.NestedStruct nestedStruct(boolean nullable, Expression. } /** - * Creates a nested map expression with one or more key-value pairs. + * Creates a nested map expression from an iterable of one or more key-value pairs. * *

Note: This method cannot be used to construct an empty map. To create an empty map, use * {@link ExpressionCreator#emptyMap(boolean, Type, Type)} which returns an {@link @@ -583,10 +583,26 @@ public static Expression.NestedStruct nestedStruct(boolean nullable, Expression. * @return a NestedMap expression */ public static Expression.NestedMap nestedMap( - boolean nullable, List keyValues) { + boolean nullable, Iterable keyValues) { return Expression.NestedMap.builder().nullable(nullable).addAllKeyValues(keyValues).build(); } + /** + * Creates a nested map expression from varargs of one or more key-value pairs. + * + *

Note: This method cannot be used to construct an empty map. To create an empty map, use + * {@link ExpressionCreator#emptyMap(boolean, Type, Type)} which returns an {@link + * Expression.EmptyMapLiteral}. + * + * @param nullable whether the map can be null + * @param keyValues the key-value pairs in the nested map, in the order they should be serialized + * @return a NestedMap expression + */ + public static Expression.NestedMap nestedMap( + boolean nullable, Expression.NestedMap.KeyValue... keyValues) { + return Expression.NestedMap.builder().nullable(nullable).addKeyValues(keyValues).build(); + } + /** * Create a UserDefinedAnyLiteral with google.protobuf.Any representation. * diff --git a/core/src/main/java/io/substrait/expression/NestedExpressionUtils.java b/core/src/main/java/io/substrait/expression/NestedExpressionUtils.java new file mode 100644 index 000000000..386847707 --- /dev/null +++ b/core/src/main/java/io/substrait/expression/NestedExpressionUtils.java @@ -0,0 +1,39 @@ +package io.substrait.expression; + +import io.substrait.type.Type; +import io.substrait.type.TypeCreator; +import java.util.List; +import java.util.stream.Collectors; + +/** Provides common utilities for the nested expression types. */ +final class NestedExpressionUtils { + + private NestedExpressionUtils() {} + + /** + * Returns the type the given expressions have in common, disregarding nullability: the type of + * the first expression, made nullable if any of the expressions is nullable. + * + *

Expressions that differ only in nullability are accepted because SQL collection constructors + * do not cast their operands to a common type: {@code ARRAY[not_null_column, nullable_column]} + * yields values that differ only in nullability. The common type of such a collection is the + * nullable one, since it holds a null. + * + * @param description what the expressions are, used in the exception message + * @param expressions the expressions to reduce to a common type, at least one + * @return the common type + * @throws IllegalArgumentException if the expressions do not all have the same type once + * nullability is disregarded + */ + static Type commonType(String description, List expressions) { + List types = expressions.stream().map(Expression::getType).collect(Collectors.toList()); + if (types.stream().map(TypeCreator::asNullable).distinct().limit(2).count() > 1) { + throw new IllegalArgumentException( + String.format( + "%s must all have the same type, disregarding nullability, but found %s", + description, types)); + } + Type first = types.get(0); + return types.stream().anyMatch(Type::nullable) ? TypeCreator.asNullable(first) : first; + } +} diff --git a/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java b/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java index 856f39efa..8c4fcf66f 100644 --- a/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java @@ -627,14 +627,17 @@ public Expression visit( throws RuntimeException { return nested( bldr -> { - Expression.Nested.Map.Builder mapBldr = Expression.Nested.Map.newBuilder(); - for (io.substrait.expression.Expression.NestedMap.KeyValue keyValue : expr.keyValues()) { - mapBldr.addKeyValues( - Expression.Nested.Map.KeyValue.newBuilder() - .setKey(toProto(keyValue.key())) - .setValue(toProto(keyValue.value()))); - } - bldr.setMap(mapBldr).setNullable(expr.nullable()); + List keyValues = + expr.keyValues().stream() + .map( + keyValue -> + Expression.Nested.Map.KeyValue.newBuilder() + .setKey(toProto(keyValue.key())) + .setValue(toProto(keyValue.value())) + .build()) + .collect(Collectors.toList()); + bldr.setMap(Expression.Nested.Map.newBuilder().addAllKeyValues(keyValues)) + .setNullable(expr.nullable()); }); } diff --git a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java index d0cfbbd81..7197917ab 100644 --- a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java @@ -496,15 +496,34 @@ public Expression.Nested from(io.substrait.proto.Expression.Nested nested) { // repeating a key keeps both of its pairs. List keyValues = nested.getMap().getKeyValuesList().stream() - .map( - keyValue -> - Expression.NestedMap.KeyValue.of( - from(keyValue.getKey()), from(keyValue.getValue()))) + .map(this::from) .collect(Collectors.toList()); return ExpressionCreator.nestedMap(nested.getNullable(), keyValues); default: - throw new IllegalStateException("Unexpected nested type: " + nested.getNestedTypeCase()); + throw new IllegalArgumentException( + "Unsupported nested type: " + nested.getNestedTypeCase()); + } + } + + /** + * Converts a proto key-value pair of a nested map expression into its POJO {@link + * io.substrait.expression.Expression.NestedMap.KeyValue}. + * + * @param keyValue the proto key-value pair to convert + * @return the converted key-value pair + * @throws IllegalArgumentException if the pair is missing its key or its value + */ + private Expression.NestedMap.KeyValue from( + io.substrait.proto.Expression.Nested.Map.KeyValue keyValue) { + if (!keyValue.hasKey()) { + throw new IllegalArgumentException( + "A key-value pair of a nested map expression has no key set"); + } + if (!keyValue.hasValue()) { + throw new IllegalArgumentException( + "A key-value pair of a nested map expression has no value set"); } + return Expression.NestedMap.KeyValue.of(from(keyValue.getKey()), from(keyValue.getValue())); } /** diff --git a/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java b/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java index bc4d0228a..7ece04fa6 100644 --- a/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java +++ b/core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java @@ -10,7 +10,6 @@ import io.substrait.expression.FunctionArg; import io.substrait.expression.ImmutableExpression; import io.substrait.util.EmptyVisitationContext; -import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -406,21 +405,35 @@ public Optional visit(Expression.NestedList expr, EmptyVisitationCon @Override public Optional visit(Expression.NestedMap expr, EmptyVisitationContext context) throws E { - boolean changed = false; - List keyValues = new ArrayList<>(); - for (Expression.NestedMap.KeyValue keyValue : expr.keyValues()) { - Optional key = keyValue.key().accept(this, context); - Optional value = keyValue.value().accept(this, context); - changed |= !allEmpty(key, value); - keyValues.add( - Expression.NestedMap.KeyValue.of( - key.orElse(keyValue.key()), value.orElse(keyValue.value()))); - } + Optional> keyValues = + transformList(expr.keyValues(), context, this::visitKeyValue); + + return keyValues.map( + keyValueList -> Expression.NestedMap.builder().from(expr).keyValues(keyValueList).build()); + } - if (!changed) { + /** + * Visits a key-value pair of a nested map expression. + * + * @param keyValue the key-value pair to visit + * @param context the visitation context + * @return Optional containing modified key-value pair, or empty if no changes + * @throws E if an error occurs during visitation + */ + protected Optional visitKeyValue( + Expression.NestedMap.KeyValue keyValue, EmptyVisitationContext context) throws E { + Optional key = keyValue.key().accept(this, context); + Optional value = keyValue.value().accept(this, context); + + if (allEmpty(key, value)) { return Optional.empty(); } - return Optional.of(Expression.NestedMap.builder().from(expr).keyValues(keyValues).build()); + return Optional.of( + Expression.NestedMap.KeyValue.builder() + .from(keyValue) + .key(key.orElse(keyValue.key())) + .value(value.orElse(keyValue.value())) + .build()); } /** diff --git a/core/src/test/java/io/substrait/TestBase.java b/core/src/test/java/io/substrait/TestBase.java index db27287b6..537f00435 100644 --- a/core/src/test/java/io/substrait/TestBase.java +++ b/core/src/test/java/io/substrait/TestBase.java @@ -12,6 +12,7 @@ import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.extension.ExtensionCollector; import io.substrait.extension.SimpleExtension; +import io.substrait.relation.Project; import io.substrait.relation.ProtoRelConverter; import io.substrait.relation.Rel; import io.substrait.relation.RelProtoConverter; @@ -62,6 +63,11 @@ protected void verifyRoundTrip(Expression expression) { assertEquals(expression, expressionReturned); } + /** Wraps the given expression in a {@link Project} over an empty virtual table scan. */ + protected Project projectOf(Expression expression) { + return Project.builder().addExpressions(expression).input(sb.emptyVirtualTableScan()).build(); + } + public static String asString(String resource) throws IOException { return Resources.toString(Resources.getResource(resource), Charsets.UTF_8); } diff --git a/core/src/test/java/io/substrait/relation/ExpressionCopyOnWriteVisitorTest.java b/core/src/test/java/io/substrait/relation/ExpressionCopyOnWriteVisitorTest.java new file mode 100644 index 000000000..443461c78 --- /dev/null +++ b/core/src/test/java/io/substrait/relation/ExpressionCopyOnWriteVisitorTest.java @@ -0,0 +1,85 @@ +package io.substrait.relation; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.substrait.TestBase; +import io.substrait.expression.Expression; +import io.substrait.util.EmptyVisitationContext; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class ExpressionCopyOnWriteVisitorTest extends TestBase { + + /** Rewrites every i32 literal to its negation, leaving all other literals alone. */ + private static class NegateI32Literals extends ExpressionCopyOnWriteVisitor { + NegateI32Literals() { + super(new RelCopyOnWriteVisitor<>()); + } + + @Override + public Optional visitLiteral(Expression.Literal literal) { + if (!(literal instanceof Expression.I32Literal)) { + return Optional.empty(); + } + Expression.I32Literal i32Literal = (Expression.I32Literal) literal; + return Optional.of( + Expression.I32Literal.builder().from(i32Literal).value(-i32Literal.value()).build()); + } + } + + @Test + void nestedMapKeysAndValuesAreRewritten() { + Expression.NestedMap nestedMap = + Expression.NestedMap.builder() + .nullable(true) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(1), sb.i32(10))) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(2), sb.i32(20))) + .build(); + + assertEquals( + Optional.of( + Expression.NestedMap.builder() + .nullable(true) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(-1), sb.i32(-10))) + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(-2), sb.i32(-20))) + .build()), + nestedMap.accept(new NegateI32Literals(), EmptyVisitationContext.INSTANCE)); + } + + @Test + void unchangedNestedMapIsNotCopied() { + Expression.NestedMap nestedMap = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.str("a"), sb.str("b"))) + .build(); + + assertEquals( + Optional.empty(), + nestedMap.accept(new NegateI32Literals(), EmptyVisitationContext.INSTANCE)); + } + + @Test + void visitKeyValueCanBeOverridden() { + // Rewriting whole pairs only requires overriding visitKeyValue, not visit(NestedMap). + ExpressionCopyOnWriteVisitor swapKeysAndValues = + new ExpressionCopyOnWriteVisitor(new RelCopyOnWriteVisitor<>()) { + @Override + protected Optional visitKeyValue( + Expression.NestedMap.KeyValue keyValue, EmptyVisitationContext context) { + return Optional.of(Expression.NestedMap.KeyValue.of(keyValue.value(), keyValue.key())); + } + }; + + Expression.NestedMap nestedMap = + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(1), sb.i32(10))) + .build(); + + assertEquals( + Optional.of( + Expression.NestedMap.builder() + .addKeyValues(Expression.NestedMap.KeyValue.of(sb.i32(10), sb.i32(1))) + .build()), + nestedMap.accept(swapKeysAndValues, EmptyVisitationContext.INSTANCE)); + } +} diff --git a/core/src/test/java/io/substrait/type/proto/NestedListExpressionTest.java b/core/src/test/java/io/substrait/type/proto/NestedListExpressionTest.java index e90525933..ac2d5e583 100644 --- a/core/src/test/java/io/substrait/type/proto/NestedListExpressionTest.java +++ b/core/src/test/java/io/substrait/type/proto/NestedListExpressionTest.java @@ -28,12 +28,7 @@ void acceptNestedListWithElementsOfSameType() { Expression.NestedList.builder().addValues(nonLiteralExpression).addValues(sb.i32(12)); assertDoesNotThrow(builder::build); - io.substrait.relation.Project project = - io.substrait.relation.Project.builder() - .addExpressions(builder.build()) - .input(sb.emptyVirtualTableScan()) - .build(); - verifyRoundTrip(project); + verifyRoundTrip(projectOf(builder.build())); } @Test @@ -45,15 +40,18 @@ void acceptNestedListWithElementsOfMixedNullability() { .addValues(sb.i32(12)) .addValues(ExpressionCreator.typedNull(N.I32)) .build(); - // The element type is the type of the first value, nullability included. - assertEquals(R.list(R.I32), mixedNullability.getType()); + // The element type is nullable, because the list holds a null. + assertEquals(R.list(N.I32), mixedNullability.getType()); + // The value order does not change the type. + assertEquals( + mixedNullability.getType(), + Expression.NestedList.builder() + .addValues(ExpressionCreator.typedNull(N.I32)) + .addValues(sb.i32(12)) + .build() + .getType()); - io.substrait.relation.Project project = - io.substrait.relation.Project.builder() - .addExpressions(mixedNullability) - .input(sb.emptyVirtualTableScan()) - .build(); - verifyRoundTrip(project); + verifyRoundTrip(projectOf(mixedNullability)); } @Test @@ -70,13 +68,7 @@ void literalNestedListTest() { .addValues(literalExpression) .build(); - io.substrait.relation.Project project = - io.substrait.relation.Project.builder() - .addExpressions(literalNestedList) - .input(sb.emptyVirtualTableScan()) - .build(); - - verifyRoundTrip(project); + verifyRoundTrip(projectOf(literalNestedList)); } @Test @@ -88,13 +80,7 @@ void literalNullableNestedListTest() { .nullable(true) .build(); - io.substrait.relation.Project project = - io.substrait.relation.Project.builder() - .addExpressions(literalNestedList) - .input(sb.emptyVirtualTableScan()) - .build(); - - verifyRoundTrip(project); + verifyRoundTrip(projectOf(literalNestedList)); } @Test @@ -105,12 +91,6 @@ void nonLiteralNestedListTest() { .addValues(nonLiteralExpression) .build(); - io.substrait.relation.Project project = - io.substrait.relation.Project.builder() - .addExpressions(nonLiteralNestedList) - .input(sb.emptyVirtualTableScan()) - .build(); - - verifyRoundTrip(project); + verifyRoundTrip(projectOf(nonLiteralNestedList)); } } diff --git a/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java b/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java index 3b2a574be..5bca5644d 100644 --- a/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java +++ b/core/src/test/java/io/substrait/type/proto/NestedMapExpressionTest.java @@ -6,8 +6,8 @@ import io.substrait.TestBase; import io.substrait.expression.Expression; +import io.substrait.expression.ExpressionCreator; import io.substrait.expression.ImmutableExpression; -import io.substrait.relation.Project; import java.util.List; import org.junit.jupiter.api.Test; @@ -39,6 +39,46 @@ void rejectNestedMapWithValuesOfDifferentTypes() { assertThrows(IllegalArgumentException.class, builder::build); } + @Test + void rejectNestedMapWithValuesOfDifferentDecimalTypes() { + // Values of the same kind but with different parameters are not the same type. + ImmutableExpression.NestedMap.Builder builder = + Expression.NestedMap.builder() + .addKeyValues( + Expression.NestedMap.KeyValue.of( + sb.str("a"), ExpressionCreator.typedNull(N.decimal(10, 2)))) + .addKeyValues( + Expression.NestedMap.KeyValue.of( + sb.str("b"), ExpressionCreator.typedNull(N.decimal(12, 2)))); + assertThrows(IllegalArgumentException.class, builder::build); + } + + @Test + void acceptNestedMapWithKeysAndValuesOfMixedNullability() { + // Keys or values that differ only in nullability are valid; SQL builds such maps from + // MAP['a', not_null_column, 'b', nullable_column]. + Expression.NestedMap.KeyValue notNull = + Expression.NestedMap.KeyValue.of(sb.str("a"), sb.i32(1)); + Expression.NestedMap.KeyValue nullable = + Expression.NestedMap.KeyValue.of( + ExpressionCreator.typedNull(N.STRING), ExpressionCreator.typedNull(N.I32)); + + Expression.NestedMap mixedNullability = + Expression.NestedMap.builder().addKeyValues(notNull).addKeyValues(nullable).build(); + // The key and value types are nullable, because the map holds a null key and a null value. + assertEquals(R.map(N.STRING, N.I32), mixedNullability.getType()); + // The pair order does not change the type. + assertEquals( + mixedNullability.getType(), + Expression.NestedMap.builder() + .addKeyValues(nullable) + .addKeyValues(notNull) + .build() + .getType()); + + verifyRoundTrip(projectOf(mixedNullability)); + } + @Test void acceptNestedMapWithKeysAndValuesOfSameType() { ImmutableExpression.NestedMap.Builder builder = @@ -131,7 +171,47 @@ void keyValueOrderIsPreservedTest() { verifyRoundTrip(projectOf(nestedMap)); } - private Project projectOf(Expression expression) { - return Project.builder().addExpressions(expression).input(sb.emptyVirtualTableScan()).build(); + @Test + void rejectMapPairWithoutKeyOnImport() { + io.substrait.proto.Expression.Nested.Map.KeyValue keyValue = + io.substrait.proto.Expression.Nested.Map.KeyValue.newBuilder() + .setValue(expressionProtoConverter.toProto(sb.i32(1))) + .build(); + + assertThrows( + IllegalArgumentException.class, () -> protoExpressionConverter.from(protoMapOf(keyValue))); + } + + @Test + void rejectMapPairWithoutValueOnImport() { + io.substrait.proto.Expression.Nested.Map.KeyValue keyValue = + io.substrait.proto.Expression.Nested.Map.KeyValue.newBuilder() + .setKey(expressionProtoConverter.toProto(sb.str("a"))) + .build(); + + assertThrows( + IllegalArgumentException.class, () -> protoExpressionConverter.from(protoMapOf(keyValue))); + } + + @Test + void rejectNestedExpressionWithoutKindOnImport() { + io.substrait.proto.Expression nested = + io.substrait.proto.Expression.newBuilder() + .setNested(io.substrait.proto.Expression.Nested.newBuilder()) + .build(); + + assertThrows(IllegalArgumentException.class, () -> protoExpressionConverter.from(nested)); + } + + /** Builds a proto map expression directly, so that pairs the POJO model rejects can be tested. */ + private io.substrait.proto.Expression protoMapOf( + io.substrait.proto.Expression.Nested.Map.KeyValue... keyValues) { + return io.substrait.proto.Expression.newBuilder() + .setNested( + io.substrait.proto.Expression.Nested.newBuilder() + .setMap( + io.substrait.proto.Expression.Nested.Map.newBuilder() + .addAllKeyValues(List.of(keyValues)))) + .build(); } } diff --git a/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java b/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java index f64656909..76dd43528 100644 --- a/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java +++ b/core/src/test/java/io/substrait/type/proto/NestedStructExpressionTest.java @@ -2,7 +2,6 @@ import io.substrait.TestBase; import io.substrait.expression.Expression; -import io.substrait.relation.Project; import org.junit.jupiter.api.Test; class NestedStructExpressionTest extends TestBase { @@ -61,8 +60,4 @@ void nestedStructOfNestedTypesTest() { verifyRoundTrip(projectOf(outer)); } - - private Project projectOf(Expression expression) { - return Project.builder().addExpressions(expression).input(sb.emptyVirtualTableScan()).build(); - } }