fix: skip index maintenance when an update leaves the indexed value unchanged (#1297)

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 <noreply@anthropic.com>

Author: brettwooldridge

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<Pair<String, Object>> before = DocumentUtils.getValues(oldDocument, fields).getValues();
+        List<Pair<String, Object>> 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) {

nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java

@@ -21,13 +21,15 @@
 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;
 
 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);
+    }
+}