diff --git a/core/src/main/java/io/substrait/relation/Set.java b/core/src/main/java/io/substrait/relation/Set.java index 6160c2460..4efef01e2 100644 --- a/core/src/main/java/io/substrait/relation/Set.java +++ b/core/src/main/java/io/substrait/relation/Set.java @@ -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); diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java index 8691466c6..3cdbfd3d1 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java @@ -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( diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java index cd10a7ef3..a367e0217 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java @@ -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(); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java b/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java index b7cc0d141..deac90e87 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java +++ b/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java @@ -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"); }; @@ -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 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))); } } diff --git a/substrait b/substrait index bc4d6fb9b..297715c7b 160000 --- a/substrait +++ b/substrait @@ -1 +1 @@ -Subproject commit bc4d6fb9bc0435c3db24172566c343e119fc50a9 +Subproject commit 297715c7b7b6cb367973cc64c1a2a296df12a812