Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update to substrait v0.57.0 #300

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
kadinrabo marked this conversation as resolved.
Show resolved Hide resolved
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";
kadinrabo marked this conversation as resolved.
Show resolved Hide resolved
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)));
}
}
Loading