Added more cases to make binarySearch for term/terms 100% coverage

Signed-off-by: expani <[email protected]>
opensearch-project · Jan 27, 2025 · 2a825a4 · 2a825a4
1 parent cd83416
commit 2a825a4
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 20 deletions.
diff --git a/...ain/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java b/...ain/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java
@@ -121,7 +121,11 @@ public void addChildNode(InMemoryTreeNode childNode, Long dimensionValue) {
         if (childNode.getNodeType() == StarTreeNodeType.STAR.getValue()) {
             this.childStarNode.set(childNode);
         } else {
-            this.children.put(dimensionValue, childNode);
+            if (childNode.getNodeType() == StarTreeNodeType.NULL.getValue()) {
+                this.children.put(Long.MAX_VALUE, childNode);
+            } else {
+                this.children.put(dimensionValue, childNode);
+            }
             assert assertStarTreeChildOrder(childNode);
         }
     }

diff --git a/...compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeSearchTests.java b/...compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeSearchTests.java
@@ -29,17 +29,57 @@
 public class FixedLengthStarTreeNodeSearchTests extends OpenSearchTestCase {
 
     public void testExactMatch() {
-        createStarTreeForDimension(new long[] { 1, 2 }, true, true, List.of(fixedLengthStarTreeNode -> {
-            try {
-                boolean result = true;
-                result &= 1 == fixedLengthStarTreeNode.getChildForDimensionValue(1L).getDimensionValue();
-                result &= 2 == fixedLengthStarTreeNode.getChildForDimensionValue(2L).getDimensionValue();
-                return result;
-            } catch (IOException e) {
-                throw new RuntimeException(e);
+        for (boolean createStarNode : new boolean[] { true, false }) {
+            for (boolean createNullNode : new boolean[] { true, false }) {
+                createStarTreeForDimension(new long[] { -1, 1, 2, 5 }, createStarNode, createNullNode, List.of(fixedLengthStarTreeNode -> {
+                    try {
+                        boolean result = true;
+                        FixedLengthStarTreeNode lastMatchedNode;
+                        lastMatchedNode = (FixedLengthStarTreeNode) fixedLengthStarTreeNode.getChildForDimensionValue(-1L);
+                        result &= -1 == lastMatchedNode.getDimensionValue();
+                        result &= null == lastMatchedNode.getChildForDimensionValue(5L);
+                        result &= null == lastMatchedNode.getChildForDimensionValue(5L, lastMatchedNode);
+                        lastMatchedNode = (FixedLengthStarTreeNode) fixedLengthStarTreeNode.getChildForDimensionValue(1L, lastMatchedNode);
+                        result &= 1 == lastMatchedNode.getDimensionValue();
+                        lastMatchedNode = (FixedLengthStarTreeNode) fixedLengthStarTreeNode.getChildForDimensionValue(5L, lastMatchedNode);
+                        result &= 5 == lastMatchedNode.getDimensionValue();
+                        lastMatchedNode = (FixedLengthStarTreeNode) fixedLengthStarTreeNode.getChildForDimensionValue(2L, lastMatchedNode);
+                        result &= null == lastMatchedNode;
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(null);
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(null, null);
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(4L);
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(6, Long.MAX_VALUE));
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(Long.MIN_VALUE, -2));
+                        return result;
+                    } catch (IOException e) {
+                        throw new RuntimeException(e);
+                    }
+                }));
+                createStarTreeForDimension(new long[] { 1 }, createStarNode, createNullNode, List.of(fixedLengthStarTreeNode -> {
+                    try {
+                        boolean result = true;
+                        result &= 1 == fixedLengthStarTreeNode.getChildForDimensionValue(1L).getDimensionValue();
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(2L);
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(2, Long.MAX_VALUE));
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(Long.MIN_VALUE, 0));
+                        return result;
+                    } catch (IOException e) {
+                        throw new RuntimeException(e);
+                    }
+                }));
+                createStarTreeForDimension(new long[] {}, createStarNode, createNullNode, List.of(fixedLengthStarTreeNode -> {
+                    try {
+                        boolean result = true;
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(1L);
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(0, Long.MAX_VALUE));
+                        result &= null == fixedLengthStarTreeNode.getChildForDimensionValue(randomLongBetween(Long.MIN_VALUE, 0));
+                        return result;
+                    } catch (IOException e) {
+                        throw new RuntimeException(e);
+                    }
+                }));
             }
-        }));
-
+        }
     }
 
     private void createStarTreeForDimension(
@@ -51,10 +91,12 @@ private void createStarTreeForDimension(
 
         try (Directory directory = newFSDirectory(createTempDir())) {
 
-            long starTreeDataLength = 0;
+            long starTreeDataLength;
 
             try (IndexOutput dataOut = directory.createOutput("star-tree-data", IOContext.DEFAULT)) {
                 StarTreeWriter starTreeWriter = new StarTreeWriter();
+                int starNodeLengthContribution = 0;
+                int nullNodeLengthContribution = 0;
 
                 InMemoryTreeNode rootNode = new InMemoryTreeNode(
                     0,
@@ -77,6 +119,7 @@ private void createStarTreeForDimension(
                     starChild.setChildDimensionId(-1);
                     starChild.setAggregatedDocId(randomInt());
                     rootNode.addChildNode(starChild, (long) ALL);
+                    starNodeLengthContribution++;
                 }
 
                 for (long dimensionValue : dimensionValues) {
@@ -99,18 +142,26 @@ private void createStarTreeForDimension(
                     rootNode.addChildNode(nullNode, (long) ALL);
                 }
 
-                starTreeDataLength = starTreeWriter.writeStarTree(dataOut, rootNode, 2 + rootNode.getChildren().size(), "star-tree");
+                starTreeDataLength = starTreeWriter.writeStarTree(
+                    dataOut,
+                    rootNode,
+                    starNodeLengthContribution + rootNode.getChildren().size() + 1,
+                    "star-tree"
+                );
 
                 // asserting on the actual length of the star tree data file
-                assertEquals(starTreeDataLength, 33L * rootNode.getChildren().size() + 2 * 33);
+                assertEquals(starTreeDataLength, (33L * rootNode.getChildren().size()) + (starNodeLengthContribution * 33L) + 33L);
             }
 
-            try (IndexInput dataIn = directory.openInput("star-tree-data", IOContext.READONCE)) {
-                StarTreeMetadata starTreeMetadata = mock(StarTreeMetadata.class);
-                when(starTreeMetadata.getDataLength()).thenReturn(starTreeDataLength);
-                when(starTreeMetadata.getDataStartFilePointer()).thenReturn(0L);
-                FixedLengthStarTreeNode effectiveRoot = (FixedLengthStarTreeNode) StarTreeFactory.createStarTree(dataIn, starTreeMetadata);
-                for (Predicate<FixedLengthStarTreeNode> predicate : predicates) {
+            for (Predicate<FixedLengthStarTreeNode> predicate : predicates) {
+                try (IndexInput dataIn = directory.openInput("star-tree-data", IOContext.READONCE)) {
+                    StarTreeMetadata starTreeMetadata = mock(StarTreeMetadata.class);
+                    when(starTreeMetadata.getDataLength()).thenReturn(starTreeDataLength);
+                    when(starTreeMetadata.getDataStartFilePointer()).thenReturn(0L);
+                    FixedLengthStarTreeNode effectiveRoot = (FixedLengthStarTreeNode) StarTreeFactory.createStarTree(
+                        dataIn,
+                        starTreeMetadata
+                    );
                     assertTrue(predicate.test(effectiveRoot));
                 }
             }