Skip to content

Commit

Permalink
chore: update to substrait v0.57.0 (#300)
Browse files Browse the repository at this point in the history
feat(POJO): handle SET_OP_MINUS_PRIMARY_ALL
feat(POJO): handle SET_OP_INTERSECTION_MULTISET_ALL

feat(Isthmus): add mappings for new SetOps

BREAKING CHANGE: EXCEPT ALL and INTERSECT ALL now output different SetOps
  • Loading branch information
kadinrabo authored Oct 4, 2024
1 parent 91ce863 commit eb484a3
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 12 deletions.
2 changes: 2 additions & 0 deletions core/src/main/java/io/substrait/relation/Set.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ public abstract class Set extends AbstractRel implements HasExtension {
public static enum SetOp {
UNKNOWN(SetRel.SetOp.SET_OP_UNSPECIFIED),
MINUS_PRIMARY(SetRel.SetOp.SET_OP_MINUS_PRIMARY),
MINUS_PRIMARY_ALL(SetRel.SetOp.SET_OP_MINUS_PRIMARY_ALL),
MINUS_MULTISET(SetRel.SetOp.SET_OP_MINUS_MULTISET),
INTERSECTION_PRIMARY(SetRel.SetOp.SET_OP_INTERSECTION_PRIMARY),
INTERSECTION_MULTISET(SetRel.SetOp.SET_OP_INTERSECTION_MULTISET),
INTERSECTION_MULTISET_ALL(SetRel.SetOp.SET_OP_INTERSECTION_MULTISET_ALL),
UNION_DISTINCT(SetRel.SetOp.SET_OP_UNION_DISTINCT),
UNION_ALL(SetRel.SetOp.SET_OP_UNION_ALL);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,17 @@ public RelNode visit(Set set) throws RuntimeException {
input -> {
relBuilder.push(input.accept(this));
});
// TODO: MINUS_MULTISET and INTERSECTION_PRIMARY mappings are set to be removed as they do not
// correspond to the Calcite relations they are associated with. They are retained for now
// to enable users to migrate off of them.
// See: https://github.com/substrait-io/substrait-java/issues/303
var builder =
switch (set.getSetOp()) {
case MINUS_PRIMARY -> relBuilder.minus(false, numInputs);
case MINUS_MULTISET -> relBuilder.minus(true, numInputs);
case INTERSECTION_PRIMARY -> relBuilder.intersect(false, numInputs);
case INTERSECTION_MULTISET -> relBuilder.intersect(true, numInputs);
case MINUS_PRIMARY_ALL, MINUS_MULTISET -> relBuilder.minus(true, numInputs);
case INTERSECTION_PRIMARY, INTERSECTION_MULTISET -> relBuilder.intersect(
false, numInputs);
case INTERSECTION_MULTISET_ALL -> relBuilder.intersect(true, numInputs);
case UNION_DISTINCT -> relBuilder.union(false, numInputs);
case UNION_ALL -> relBuilder.union(true, numInputs);
case UNKNOWN -> throw new UnsupportedOperationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,15 @@ public Rel visit(org.apache.calcite.rel.core.Union union) {
@Override
public Rel visit(org.apache.calcite.rel.core.Intersect intersect) {
var inputs = apply(intersect.getInputs());
var setOp = intersect.all ? Set.SetOp.INTERSECTION_MULTISET : Set.SetOp.INTERSECTION_PRIMARY;
var setOp =
intersect.all ? Set.SetOp.INTERSECTION_MULTISET_ALL : Set.SetOp.INTERSECTION_MULTISET;
return Set.builder().inputs(inputs).setOp(setOp).build();
}

@Override
public Rel visit(org.apache.calcite.rel.core.Minus minus) {
var inputs = apply(minus.getInputs());
var setOp = minus.all ? Set.SetOp.MINUS_MULTISET : Set.SetOp.MINUS_PRIMARY;
var setOp = minus.all ? Set.SetOp.MINUS_PRIMARY_ALL : Set.SetOp.MINUS_PRIMARY;
return Set.builder().inputs(inputs).setOp(setOp).build();
}

Expand Down
18 changes: 12 additions & 6 deletions isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ public static String getSetQuery(Set.SetOp op, boolean multi) {
String opString =
switch (op) {
case MINUS_PRIMARY -> "EXCEPT";
case MINUS_MULTISET -> "EXCEPT ALL";
case INTERSECTION_PRIMARY -> "INTERSECT";
case INTERSECTION_MULTISET -> "INTERSECT ALL";
case MINUS_PRIMARY_ALL -> "EXCEPT ALL";
case INTERSECTION_MULTISET -> "INTERSECT";
case INTERSECTION_MULTISET_ALL -> "INTERSECT ALL";
case UNION_DISTINCT -> "UNION";
case UNION_ALL -> "UNION ALL";
case UNKNOWN -> throw new UnsupportedOperationException(
default -> throw new UnsupportedOperationException(
"Unknown set operation is not supported");
};

Expand All @@ -49,10 +49,16 @@ public static String getSetQuery(Set.SetOp op, boolean multi) {
}
}

// Generate all combinations excluding the UNKNOWN operator
// Generate all SetOp types excluding:
// * MINUS_MULTISET, INTERSECTION_PRIMARY: do not map to Calcite relations
// * UNKNOWN: invalid
public static Stream<Arguments> setTestConfig() {
return Arrays.stream(Set.SetOp.values())
.filter(op -> op != Set.SetOp.UNKNOWN)
.filter(
op ->
op != Set.SetOp.UNKNOWN
&& op != Set.SetOp.MINUS_MULTISET
&& op != Set.SetOp.INTERSECTION_PRIMARY)
.flatMap(op -> Stream.of(arguments(op, false), arguments(op, true)));
}
}

0 comments on commit eb484a3

Please sign in to comment.