From b4eb2f47bef8485d942edbbbefcbceea0b128921 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Fri, 4 Sep 2026 12:33:59 +0900 Subject: [PATCH] fix: skip index maintenance when an update leaves the indexed value unchanged DocumentIndexWriter.updateIndexEntry treated an index as affected whenever the update document carried the indexed field, and then removed and rewrote the entry. An update that writes the whole document back, the common upsert shape, carries every indexed field with its old value, so every index was rewritten on every update for nothing. On the pre-4.4.0 list layout that was a copy of the whole per-key id list twice per index per write. The writer now compares the old and new values of each affected index, deeply so arrays and embedded values count as equal when their contents are, and skips the index when they match. A dirty index is not skipped: its rebuild still has to happen on the first write, so that path is unchanged. Tests cover the unchanged value, an array with equal contents, a changed value, and the dirty-index rebuild with an unchanged value. Co-Authored-By: Claude Fable 5.1 --- .../operation/DocumentIndexWriter.java | 31 +++++++++ .../operation/DocumentIndexWriterTest.java | 66 ++++++++++++++++++- 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java index b705f4a3c..7bc56482e 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java @@ -20,11 +20,14 @@ import org.dizitart.no2.collection.Document; import org.dizitart.no2.common.FieldValues; import org.dizitart.no2.common.Fields; +import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.common.util.DocumentUtils; import org.dizitart.no2.index.IndexDescriptor; import org.dizitart.no2.index.NitriteIndexer; import java.util.Collection; +import java.util.Objects; +import java.util.List; /** * @since 4.0 @@ -73,6 +76,15 @@ void updateIndexEntry(Document oldDocument, Document newDocument, Document updat // if the index is affected by the update if (DocumentUtils.isAffectedByUpdate(fields, updatedFields)) { + // "affected" only means the update carries the field. An update that + // writes the whole document back, the common upsert shape, carries every + // indexed field with its old value, and rewriting those entries is pure + // cost. A dirty index still has to be rebuilt, so that case is not skipped. + if (!indexOperations.shouldRebuildIndex(fields) + && sameIndexedValues(oldDocument, newDocument, fields)) { + continue; + } + String indexType = indexDescriptor.getIndexType(); NitriteIndexer nitriteIndexer = nitriteConfig.findIndexer(indexType); @@ -83,6 +95,25 @@ void updateIndexEntry(Document oldDocument, Document newDocument, Document updat } } + /** + * Whether the two documents hold the same values for every field of the index, compared + * deeply so that arrays and embedded values count as equal when their contents are. + */ + private static boolean sameIndexedValues(Document oldDocument, Document newDocument, Fields fields) { + List> before = DocumentUtils.getValues(oldDocument, fields).getValues(); + List> after = DocumentUtils.getValues(newDocument, fields).getValues(); + if (before.size() != after.size()) { + return false; + } + for (int i = 0; i < before.size(); i++) { + if (!Objects.equals(before.get(i).getFirst(), after.get(i).getFirst()) + || !Objects.deepEquals(before.get(i).getSecond(), after.get(i).getSecond())) { + return false; + } + } + return true; + } + private void writeIndexEntryInternal(IndexDescriptor indexDescriptor, Document document, NitriteIndexer nitriteIndexer) { if (indexDescriptor != null) { diff --git a/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java b/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java index 42edb9745..bcf4c292e 100644 --- a/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java @@ -21,6 +21,7 @@ import org.dizitart.no2.common.Fields; import org.dizitart.no2.index.IndexDescriptor; import org.dizitart.no2.index.IndexType; +import org.dizitart.no2.index.NitriteIndexer; import org.dizitart.no2.index.UniqueIndexer; import org.dizitart.no2.store.memory.InMemoryStore; import org.junit.Test; @@ -28,6 +29,7 @@ import java.util.ArrayList; import static org.dizitart.no2.collection.Document.createDocument; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; public class DocumentIndexWriterTest { @@ -72,5 +74,67 @@ public void testUpdateIndexEntry() { (new DocumentIndexWriter(nitriteConfig, indexOperations)).updateIndexEntry(createDocument("a", 1), createDocument("a", 2), createDocument("a", 3)); verify(indexOperations).listIndexes(); } -} + @Test + public void testUpdateSkipsIndexWhenIndexedValueIsUnchanged() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", 1).put("b", "old"), + createDocument("a", 1).put("b", "new"), + createDocument("a", 1).put("b", "new")); + + verify(indexer, never()).removeIndexEntry(any(), any(), any()); + verify(indexer, never()).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateSkipsIndexWhenArrayValueHasSameContents() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", new int[]{1, 2}), + createDocument("a", new int[]{1, 2}), + createDocument("a", new int[]{1, 2})); + + verify(indexer, never()).removeIndexEntry(any(), any(), any()); + verify(indexer, never()).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateRewritesIndexWhenIndexedValueChanges() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", 1), createDocument("a", 2), createDocument("a", 2)); + + verify(indexer).removeIndexEntry(any(), any(), any()); + verify(indexer).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateStillRebuildsDirtyIndexWhenValueIsUnchanged() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + IndexOperations indexOperations = mock(IndexOperations.class); + IndexDescriptor descriptor = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("a"), "c"); + when(indexOperations.listIndexes()).thenReturn(new ArrayList<>(java.util.List.of(descriptor))); + when(indexOperations.shouldRebuildIndex(any())).thenReturn(true); + NitriteConfig nitriteConfig = mock(NitriteConfig.class); + doReturn(indexer).when(nitriteConfig).findIndexer(IndexType.NON_UNIQUE); + + new DocumentIndexWriter(nitriteConfig, indexOperations) + .updateIndexEntry(createDocument("a", 1), createDocument("a", 1), createDocument("a", 1)); + + verify(indexOperations, atLeastOnce()).buildIndex(descriptor, true); + } + + private static DocumentIndexWriter writerWithIndexOn(String field, NitriteIndexer indexer, boolean dirty) { + IndexOperations indexOperations = mock(IndexOperations.class); + IndexDescriptor descriptor = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames(field), "c"); + when(indexOperations.listIndexes()).thenReturn(new ArrayList<>(java.util.List.of(descriptor))); + when(indexOperations.shouldRebuildIndex(any())).thenReturn(dirty); + NitriteConfig nitriteConfig = mock(NitriteConfig.class); + doReturn(indexer).when(nitriteConfig).findIndexer(IndexType.NON_UNIQUE); + return new DocumentIndexWriter(nitriteConfig, indexOperations); + } +}