diff --git a/server/src/main/java/org/opensearch/index/codec/composite/composite99/Composite99DocValuesWriter.java b/server/src/main/java/org/opensearch/index/codec/composite/composite99/Composite99DocValuesWriter.java index f113e7a37fbd4..0d4e35f7c3ab8 100644 --- a/server/src/main/java/org/opensearch/index/codec/composite/composite99/Composite99DocValuesWriter.java +++ b/server/src/main/java/org/opensearch/index/codec/composite/composite99/Composite99DocValuesWriter.java @@ -68,7 +68,7 @@ public class Composite99DocValuesWriter extends DocValuesConsumer { public IndexOutput metaOut; private final Set segmentFieldSet; private final boolean segmentHasCompositeFields; - private AtomicInteger fieldNumberAcrossStarTrees; + private final AtomicInteger fieldNumberAcrossCompositeFields; private final Map fieldProducerMap = new HashMap<>(); @@ -78,7 +78,7 @@ public Composite99DocValuesWriter(DocValuesConsumer delegate, SegmentWriteState this.delegate = delegate; this.state = segmentWriteState; this.mapperService = mapperService; - this.fieldNumberAcrossStarTrees = new AtomicInteger(); + this.fieldNumberAcrossCompositeFields = new AtomicInteger(); this.compositeMappedFieldTypes = mapperService.getCompositeFieldTypes(); compositeFieldSet = new HashSet<>(); segmentFieldSet = new HashSet<>(); @@ -223,7 +223,7 @@ private void createCompositeIndicesIfPossible(DocValuesProducer valuesProducer, if (compositeFieldSet.isEmpty()) { for (CompositeMappedFieldType mappedType : compositeMappedFieldTypes) { if (mappedType instanceof StarTreeMapper.StarTreeFieldType) { - try (StarTreesBuilder starTreesBuilder = new StarTreesBuilder(state, mapperService, fieldNumberAcrossStarTrees)) { + try (StarTreesBuilder starTreesBuilder = new StarTreesBuilder(state, mapperService, fieldNumberAcrossCompositeFields)) { starTreesBuilder.build(metaOut, dataOut, fieldProducerMap, composite99DocValuesConsumer); } } @@ -312,7 +312,7 @@ private void mergeStarTreeFields(MergeState mergeState) throws IOException { } } } - try (StarTreesBuilder starTreesBuilder = new StarTreesBuilder(state, mapperService, fieldNumberAcrossStarTrees)) { + try (StarTreesBuilder starTreesBuilder = new StarTreesBuilder(state, mapperService, fieldNumberAcrossCompositeFields)) { starTreesBuilder.buildDuringMerge(metaOut, dataOut, starTreeSubsPerField, composite99DocValuesConsumer); } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java index 0acb0e17becc6..13d5059a241b1 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java @@ -367,7 +367,7 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer, A throw new IllegalStateException("Unknown metric doc value type"); } } catch (IllegalArgumentException e) { - logger.info("could not parse the value, exiting creation of star tree"); + logger.error("could not parse the value, exiting creation of star tree"); } } } @@ -671,7 +671,7 @@ private SequentialDocValuesIterator getIteratorForNumericField( String name ) throws IOException { if (fieldInfo == null) { - fieldInfo = getFieldInfo(name, DocValuesType.NUMERIC, 1); + fieldInfo = getFieldInfo(name, DocValuesType.NUMERIC); } SequentialDocValuesIterator sequentialDocValuesIterator; assert fieldProducerMap.containsKey(fieldInfo.name); @@ -822,7 +822,7 @@ private StarTreeDocument createAggregatedDocs(InMemoryTreeNode node) throws IOEx StarTreeDocument aggregatedStarTreeDocument = null; // For leaf node - if (node.getChildren().isEmpty() && node.getChildStarNode() == null) { + if (!node.hasChild()) { if (node.getStartDocId() == node.getEndDocId() - 1) { // If it has only one document, use it as the aggregated document diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java index 2ac1001a2865a..c3bf4475f75c2 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/node/InMemoryTreeNode.java @@ -119,11 +119,10 @@ public void setNodeType(byte nodeType) { public void addChildNode(InMemoryTreeNode childNode, Long dimensionValue) { if (childNode.getNodeType() == StarTreeNodeType.STAR.getValue()) { - assert this.childStarNode.get() == null; this.childStarNode.set(childNode); } else { - assert assertStarTreeChildOrder(childNode); this.children.put(dimensionValue, childNode); + assert assertStarTreeChildOrder(childNode); } } @@ -158,6 +157,12 @@ private boolean assertStarTreeChildOrder(InMemoryTreeNode childNode) { lastNode = entry.getValue(); } assert lastNode.dimensionValue <= childNode.dimensionValue; + } else if (childNode.nodeType == StarTreeNodeType.NULL.getValue() && !this.children.isEmpty()) { + InMemoryTreeNode lastNode = null; + for (Map.Entry entry : this.children.entrySet()) { + lastNode = entry.getValue(); + } + assert lastNode.nodeType == StarTreeNodeType.NULL.getValue(); } return true; } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java index a55454ab78337..061841d3e140a 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java @@ -95,12 +95,7 @@ public Long value(int currentDocId) throws IOException { if (docId == DocIdSetIterator.NO_MORE_DOCS || docId != currentDocId) { return null; } - if (docValue == null) { - docValue = sortedNumericDocValues.nextValue(); - } - Long nextValue = docValue; - docValue = null; - return nextValue; + return sortedNumericDocValues.nextValue(); } else { throw new IllegalStateException("Unsupported Iterator requested for SequentialDocValuesIterator"); diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java index 95e7e28e929ae..9657f867422a0 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNodeTests.java @@ -12,6 +12,7 @@ import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils; import org.opensearch.index.compositeindex.datacube.startree.fileformats.StarTreeWriter; import org.opensearch.index.compositeindex.datacube.startree.fileformats.meta.StarTreeMetadata; import org.opensearch.index.compositeindex.datacube.startree.node.InMemoryTreeNode; @@ -156,6 +157,26 @@ public void testGetChildrenIterator() throws IOException { assertEquals(starTreeNode.getNumChildren(), count); } + public void testChildrenOrder() throws IOException { + Iterator iterator = starTreeNode.getChildrenIterator(); + int count = 0; + while (iterator.hasNext()) { + FixedLengthStarTreeNode child = iterator.next(); + count++; + if (count == 1) { + StarTreeTestUtils.assertStarTreeNode(child, starChild); + } else if (count == 2) { + StarTreeTestUtils.assertStarTreeNode(child, childWithMinus1); + } else if (count == starTreeNode.getNumChildren()) { + StarTreeTestUtils.assertStarTreeNode(child, nullChild); + } else { + StarTreeTestUtils.assertStarTreeNode(child, node.getChildren().get(child.getDimensionValue())); + } + assertNotNull(child); + } + assertEquals(starTreeNode.getNumChildren(), count); + } + public void testGetChildForStarNode() throws IOException { // Assuming the first child is a star node in our test data FixedLengthStarTreeNode starNode = (FixedLengthStarTreeNode) starTreeNode.getChildStarNode();