From e62a8d5ad10d4364da1d70716b240aef7591694f Mon Sep 17 00:00:00 2001
From: Stamatis Zampetakis <zabetak@gmail.com>
Date: Mon, 18 Nov 2024 16:41:48 +0100
Subject: [PATCH] [CALCITE-6698] Wrong (swapped) results in
 PartiallyOrderedSet#getNonChildren and getNonParents

1. Align the behavior of getNonChildren/getNonParents with:
i) the class documentation;
ii) the behavior of getParents/getChildren methods.
2. Add explicit Javadoc to clarify the results of its method.
3. Update and add test cases with expected results.
---
 .../calcite/util/PartiallyOrderedSet.java     | 22 +++++++++--
 .../calcite/util/PartiallyOrderedSetTest.java | 37 +++++++++++++++++--
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/util/PartiallyOrderedSet.java b/core/src/main/java/org/apache/calcite/util/PartiallyOrderedSet.java
index 94df762bc82..e7ff1dca09c 100644
--- a/core/src/main/java/org/apache/calcite/util/PartiallyOrderedSet.java
+++ b/core/src/main/java/org/apache/calcite/util/PartiallyOrderedSet.java
@@ -90,6 +90,10 @@ public class PartiallyOrderedSet<E> extends AbstractSet<E> {
    * in the set.
    */
   private final Node<E> topNode;
+  /**
+   * Synthetic node to hold all nodes that have no children. It does not appear
+   * in the set.
+   */
   private final Node<E> bottomNode;
 
   /**
@@ -552,7 +556,7 @@ public void out(StringBuilder buf) {
     // breadth-first search, to iterate over every element once, printing
     // those nearest the top element first
     final Set<E> seen = new HashSet<>();
-    final Deque<E> unseen = new ArrayDeque<>(getNonChildren());
+    final Deque<E> unseen = new ArrayDeque<>(getNonParents());
     while (!unseen.isEmpty()) {
       E e = unseen.pop();
       buf.append("  ");
@@ -681,12 +685,24 @@ private void closure(Function<E, Iterable<E>> generator, E e, ImmutableList.Buil
     }
   }
 
+  /**
+   * Returns all elements in the set that have no children. These elements are the leaf/minimal
+   * elements of the poset.
+   *
+   * @return a list of elements that have no children.
+   */
   public List<E> getNonChildren() {
-    return strip(topNode.childList);
+    return strip(bottomNode.parentList);
   }
 
+  /**
+   * Returns all elements in the set that have no parents. These elements are the roots/maximal
+   * elements of the poset.
+   *
+   * @return a list of elements that have no parents.
+   */
   public List<E> getNonParents() {
-    return strip(bottomNode.parentList);
+    return strip(topNode.childList);
   }
 
   @Override public void clear() {
diff --git a/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java b/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java
index 05a48fde57d..334bb86d4f3 100644
--- a/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java
+++ b/core/src/test/java/org/apache/calcite/util/PartiallyOrderedSetTest.java
@@ -20,6 +20,7 @@
 
 import java.util.AbstractList;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -110,8 +111,8 @@ private static boolean isBitSuperset(Integer e1, Integer e2) {
     poset.add(abcd);
     printValidate(poset);
     assertThat(poset, hasSize(2));
-    assertThat(poset.getNonChildren(), hasToString("['abcd']"));
-    assertThat(poset.getNonParents(), hasToString("['']"));
+    assertThat(poset.getNonChildren(), hasToString("['']"));
+    assertThat(poset.getNonParents(), hasToString("['abcd']"));
 
     final String ab = "'ab'";
     poset.add(ab);
@@ -158,8 +159,8 @@ private static boolean isBitSuperset(Integer e1, Integer e2) {
 
     poset.add(b);
     printValidate(poset);
-    assertThat(poset.getNonChildren(), hasToString("['abcd']"));
-    assertThat(poset.getNonParents(), hasToString("['']"));
+    assertThat(poset.getNonChildren(), hasToString("['']"));
+    assertThat(poset.getNonParents(), hasToString("['abcd']"));
     assertThat(poset.getChildren(b), hasToString("['']"));
     assertEqualsList("['ab', 'bcd']", poset.getParents(b));
     assertThat(poset.getChildren(b), hasToString("['']"));
@@ -180,6 +181,32 @@ private static boolean isBitSuperset(Integer e1, Integer e2) {
     assertEqualsList("['ab', 'abcd']", poset.getAncestors("'a'"));
   }
 
+  @Test void testGetNonParentsOnLteIntPosetReturnsMaxValue() {
+    PartiallyOrderedSet<Integer> poset =
+        new PartiallyOrderedSet<>((i, j) -> i <= j, Arrays.asList(20, 30, 40));
+    assertThat(poset.getNonParents(), hasToString("[40]"));
+  }
+
+  @Test void testGetNonChildrenOnLteIntPosetReturnsMinValue() {
+    PartiallyOrderedSet<Integer> poset =
+        new PartiallyOrderedSet<>((i, j) -> i <= j, Arrays.asList(20, 30, 40));
+    assertThat(poset.getNonChildren(), hasToString("[20]"));
+  }
+
+  @Test void testGetNonParentsOnGteIntPosetReturnsMinValue() {
+    PartiallyOrderedSet<Integer> poset =
+        new PartiallyOrderedSet<>((i, j) -> i >= j, Arrays.asList(20, 30, 40));
+    assertThat(poset.getNonParents(), hasToString("[20]"));
+  }
+
+  @Test void testGetNonChildrenOnGteIntPosetReturnsMaxValue() {
+    PartiallyOrderedSet<Integer> poset =
+        new PartiallyOrderedSet<>((i, j) -> i >= j, Arrays.asList(20, 30, 40));
+    printValidate(poset);
+    System.out.println(poset.getChildren(20));
+    assertThat(poset.getNonChildren(), hasToString("[40]"));
+  }
+
   @Test void testPosetTricky() {
     final PartiallyOrderedSet<String> poset =
         new PartiallyOrderedSet<>(STRING_SUBSET_ORDERING);
@@ -195,6 +222,8 @@ private static boolean isBitSuperset(Integer e1, Integer e2) {
     printValidate(poset);
     poset.add("'ab'");
     printValidate(poset);
+    assertThat(poset.getNonChildren(), hasToString("['a', 'b']"));
+    assertThat(poset.getNonParents(), hasToString("['ac', 'ab']"));
   }
 
   @Test void testPosetBits() {