Skip to content

Commit

Permalink
fix: Set proto-to-rel type handling
Browse files Browse the repository at this point in the history
  • Loading branch information
Blizzara committed Mar 5, 2025
1 parent fb36197 commit e7be830
Showing 1 changed file with 48 additions and 1 deletion.
49 changes: 48 additions & 1 deletion core/src/main/java/io/substrait/relation/Set.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import io.substrait.proto.SetRel;
import io.substrait.type.Type;
import io.substrait.type.TypeCreator;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
import org.immutables.value.Value;

@Value.Immutable
Expand Down Expand Up @@ -42,7 +46,50 @@ public static Set.SetOp fromProto(SetRel.SetOp proto) {

@Override
protected Type.Struct deriveRecordType() {
return getInputs().get(0).getRecordType();
// The different inputs may have schemas that differ in nullability, but not in type.
// In that case we should return a schema that is nullable where any of the inputs is nullable.
Stream<Type.Struct> inputRecordTypes = getInputs().stream().map(Rel::getRecordType);
return inputRecordTypes
.reduce(
(a, b) -> {
if (a.equals(b)) {
return a; // Short-circuit trivial case
}

if (a.fields().size() != b.fields().size()) {
throw new IllegalArgumentException(
"Set's input records have different number of fields");
}

List<Type> fields = new ArrayList<>();
for (int i = 0; i < a.fields().size(); i++) {
fields.add(coalesceNullabilityOrThrow(a.fields().get(i), b.fields().get(i), i));
}

return Type.Struct.builder()
.fields(fields)
.nullable(a.nullable() || b.nullable())
.build();
})
.orElseThrow(() -> new IllegalStateException("A Set relation needs at least one input"));
}

private Type coalesceNullabilityOrThrow(Type a, Type b, int idx) {
if (a.equals(b)) {
return a;
} else if (a.getClass() == b.getClass() && a.nullable() != b.nullable()) {
// Class is equal but nullability is not (i.e. at least one is nullable), this we
// can fix by making the result nullable.
Type nullableFieldA = TypeCreator.asNullable(a);
// There may be differences also in the nullability or content of the inner fields.
// In those cases we throw. TODO: should the coalescing be recursive?
if (nullableFieldA.equals(TypeCreator.asNullable(b))) {
return nullableFieldA;
}
}
throw new IllegalArgumentException(
String.format(
"Set's input records have different types for field %s: %s vs %s", idx, a, b));
}

@Override
Expand Down

0 comments on commit e7be830

Please sign in to comment.