changed order for null node

Signed-off-by: Sarthak Aggarwal <[email protected]>
opensearch-project · Sep 2, 2024 · d2bba60 · d2bba60
1 parent b921ada
commit d2bba60
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 26 deletions.
diff --git a/...arch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java b/...arch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java
@@ -250,13 +250,12 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue) throws IO
             low++;
         }
 
+        int high = getInt(LAST_CHILD_ID_OFFSET);
         // if the current node is null node, increment the low to reduce the search space
-        if (matchStarTreeNodeTypeOrNull(new FixedLengthStarTreeNode(in, low), StarTreeNodeType.NULL) != null) {
-            low++;
+        if (matchStarTreeNodeTypeOrNull(new FixedLengthStarTreeNode(in, high), StarTreeNodeType.NULL) != null) {
+            high--;
         }
 
-        int high = getInt(LAST_CHILD_ID_OFFSET);
-
         while (low <= high) {
             int mid = low + (high - low) / 2;
             FixedLengthStarTreeNode midNode = new FixedLengthStarTreeNode(in, mid);

diff --git a/...ain/java/org/opensearch/index/compositeindex/datacube/startree/node/StarTreeNodeType.java b/...ain/java/org/opensearch/index/compositeindex/datacube/startree/node/StarTreeNodeType.java
@@ -23,11 +23,11 @@
  * <p>
  * The node type can be one of the following:
  * <ul>
- *     <li>Star Node: Represented by the value -2. A star node is a special node that represents
+ *     <li>Star Node: Represented by the value -1. A star node is a special node that represents
  *     all possible values for a dimension.</li>
- *     <li>Null Node: Represented by the value -1. A null node indicates the absence of any value
+ *     <li>Null Node: Represented by the value 0. A null node indicates the absence of any value
  *     for a dimension.</li>
- *     <li>Default Node: Represented by the value 0. A default node represents a node with an
+ *     <li>Default Node: Represented by the value -1. A default node represents a node with an
  *     actual dimension value.</li>
  * </ul>
  *
@@ -42,17 +42,17 @@ public enum StarTreeNodeType {
      * Represents a star node type.
      *
      */
-    STAR("star", (byte) -2),
+    STAR("star", (byte) -1),
 
     /**
-     * Represents a null node type.
+     * Represents a default node type.
      */
-    NULL("null", (byte) -1),
+    DEFAULT("default", (byte) 0),
 
     /**
-     * Represents a default node type.
+     * Represents a null node type.
      */
-    DEFAULT("default", (byte) 0);
+    NULL("null", (byte) 1);
 
     private final String name;
     private final byte value;

diff --git a/...rc/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java b/...rc/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java
@@ -37,6 +37,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 public class StarTreeTestUtils {
@@ -197,12 +198,22 @@ public static void validateFileFormats(
                         // check if star tree node exists
                         resultChildNode = rootNode.getChildStarNode();
                         assertNotNull(child);
+                        assertNotNull(starTreeNode.getChildStarNode());
                         assertStarTreeNode(child, resultChildNode);
                         childStarNodeAsserted = true;
                     } else {
                         resultChildNode = sortedChildren.get(childCount);
                         assertNotNull(child);
                         assertNotNull(resultChildNode);
+                        if (child.getStarTreeNodeType() != StarTreeNodeType.NULL.getValue()) {
+                            assertNotNull(starTreeNode.getChildForDimensionValue(child.getDimensionValue()));
+                        } else {
+                            StarTreeNode finalStarTreeNode = starTreeNode;
+                            assertThrows(
+                                AssertionError.class,
+                                () -> finalStarTreeNode.getChildForDimensionValue(child.getDimensionValue())
+                            );
+                        }
                         assertStarTreeNode(child, resultChildNode);
                         assertNotEquals(child.getStarTreeNodeType(), StarTreeNodeType.STAR.getValue());
                         childCount++;

diff --git a/...index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java b/...index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java
@@ -44,20 +44,15 @@ public void setup() throws IOException {
         dataOut = directory.createOutput("star-tree-data", IOContext.DEFAULT);
         StarTreeWriter starTreeWriter = new StarTreeWriter();
 
-        node = new InMemoryTreeNode(0, randomInt(), randomInt(), randomFrom((byte) 0, (byte) -1, (byte) 2), -1);
+        node = new InMemoryTreeNode(0, randomInt(), randomInt(), randomFrom((byte) 0, (byte) -1, (byte) 1), -1);
         node.setChildDimensionId(1);
         node.setAggregatedDocId(randomInt());
 
-        starChild = new InMemoryTreeNode(node.getDimensionId() + 1, randomInt(), randomInt(), (byte) -2, -1);
+        starChild = new InMemoryTreeNode(node.getDimensionId() + 1, randomInt(), randomInt(), (byte) -1, -1);
         starChild.setChildDimensionId(-1);
         starChild.setAggregatedDocId(randomInt());
         node.addChildNode(starChild, (long) ALL);
 
-        nullChild = new InMemoryTreeNode(node.getDimensionId() + 1, randomInt(), randomInt(), (byte) -1, -1);
-        nullChild.setChildDimensionId(-1);
-        nullChild.setAggregatedDocId(randomInt());
-        node.addChildNode(nullChild, null);
-
         childWithMinus1 = new InMemoryTreeNode(node.getDimensionId() + 1, randomInt(), randomInt(), (byte) 0, -1);
         childWithMinus1.setChildDimensionId(-1);
         childWithMinus1.setAggregatedDocId(randomInt());
@@ -76,6 +71,11 @@ public void setup() throws IOException {
             node.addChildNode(child, child.getDimensionValue());
         }
 
+        nullChild = new InMemoryTreeNode(node.getDimensionId() + 1, randomInt(), randomInt(), (byte) 1, -1);
+        nullChild.setChildDimensionId(-1);
+        nullChild.setAggregatedDocId(randomInt());
+        node.addChildNode(nullChild, null);
+
         long starTreeDataLength = starTreeWriter.writeStarTree(dataOut, node, 2 + node.getChildren().size(), "star-tree");
 
         // asserting on the actual length of the star tree data file

diff --git a/...ava/org/opensearch/index/compositeindex/datacube/startree/node/StarTreeNodeTypeTests.java b/...ava/org/opensearch/index/compositeindex/datacube/startree/node/StarTreeNodeTypeTests.java
@@ -14,12 +14,12 @@ public class StarTreeNodeTypeTests extends OpenSearchTestCase {
 
     public void testStarNodeType() {
         assertEquals("star", StarTreeNodeType.STAR.getName());
-        assertEquals((byte) -2, StarTreeNodeType.STAR.getValue());
+        assertEquals((byte) -1, StarTreeNodeType.STAR.getValue());
     }
 
     public void testNullNodeType() {
         assertEquals("null", StarTreeNodeType.NULL.getName());
-        assertEquals((byte) -1, StarTreeNodeType.NULL.getValue());
+        assertEquals((byte) 1, StarTreeNodeType.NULL.getValue());
     }
 
     public void testDefaultNodeType() {
@@ -28,20 +28,20 @@ public void testDefaultNodeType() {
     }
 
     public void testFromValue() {
-        assertEquals(StarTreeNodeType.STAR, StarTreeNodeType.fromValue((byte) -2));
-        assertEquals(StarTreeNodeType.NULL, StarTreeNodeType.fromValue((byte) -1));
+        assertEquals(StarTreeNodeType.STAR, StarTreeNodeType.fromValue((byte) -1));
+        assertEquals(StarTreeNodeType.NULL, StarTreeNodeType.fromValue((byte) 1));
         assertEquals(StarTreeNodeType.DEFAULT, StarTreeNodeType.fromValue((byte) 0));
     }
 
     public void testFromValueInvalid() {
-        IllegalStateException exception = expectThrows(IllegalStateException.class, () -> StarTreeNodeType.fromValue((byte) 1));
-        assertEquals("Unrecognized value byte to determine star-tree node type: [1]", exception.getMessage());
+        IllegalStateException exception = expectThrows(IllegalStateException.class, () -> StarTreeNodeType.fromValue((byte) 2));
+        assertEquals("Unrecognized value byte to determine star-tree node type: [2]", exception.getMessage());
     }
 
     public void testEnumValues() {
         StarTreeNodeType[] values = StarTreeNodeType.values();
         assertEquals(3, values.length);
-        assertArrayEquals(new StarTreeNodeType[] { StarTreeNodeType.STAR, StarTreeNodeType.NULL, StarTreeNodeType.DEFAULT }, values);
+        assertArrayEquals(new StarTreeNodeType[] { StarTreeNodeType.STAR, StarTreeNodeType.DEFAULT, StarTreeNodeType.NULL }, values);
     }
 
     public void testEnumValueOf() {