docs: minor doc updates

substrait-io · Oct 4, 2024 · 413fede · 413fede
1 parent d70f7d9
commit 413fede
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 4 deletions.
diff --git a/core/src/main/java/io/substrait/relation/Set.java b/core/src/main/java/io/substrait/relation/Set.java
@@ -8,7 +8,6 @@
 public abstract class Set extends AbstractRel implements HasExtension {
   public abstract Set.SetOp getSetOp();
 
-  // MINUS_MULTISET and INTERSECTION_PRIMARY mappings are set to be removed due to no direct SQL mapping (substrait-io/substrait/pull/708)
   public static enum SetOp {
     UNKNOWN(SetRel.SetOp.SET_OP_UNSPECIFIED),
     MINUS_PRIMARY(SetRel.SetOp.SET_OP_MINUS_PRIMARY),

diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java
@@ -211,7 +211,10 @@ public RelNode visit(Set set) throws RuntimeException {
             input -> {
               relBuilder.push(input.accept(this));
             });
-    // MINUS_MULTISET and INTERSECTION_PRIMARY mappings are set to be removed due to no direct SQL mapping (substrait-io/substrait/pull/708)
+    // 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);

diff --git a/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java b/isthmus/src/test/java/io/substrait/isthmus/utils/SetUtils.java
@@ -49,8 +49,9 @@ public static String getSetQuery(Set.SetOp op, boolean multi) {
     }
   }
 
-  // Generate all combinations excluding the UNKNOWN operator
-  // Note: MINUS_MULTISET and INTERSECTION_PRIMARY mappings to Calcite are set to be removed due to no direct SQL mapping (substrait-io/substrait/pull/708)
+  // 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(