diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/IndexManager.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/IndexManager.java index 4448b634..684404c0 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/IndexManager.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/IndexManager.java @@ -28,7 +28,9 @@ import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; +import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMapName; +import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName; import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMetaMapName; /** @@ -93,32 +95,57 @@ public void close() { Iterable indexMetas = indexMetaMap.values(); for (IndexMeta indexMeta : indexMetas) { if (indexMeta != null && indexMeta.getIndexDescriptor() != null) { - String indexMapName = indexMeta.getIndexMap(); - NitriteMap indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class); - indexMap.close(); + for (NitriteMap indexMap : existingLayoutMaps(indexMeta)) { + indexMap.close(); + } } } - // close index meta indexMetaMap.close(); } } public void clearAll() { - // close all index maps + // clear and close all index maps if (!indexMetaMap.isClosed() && !indexMetaMap.isDropped()) { Iterable indexMetas = indexMetaMap.values(); for (IndexMeta indexMeta : indexMetas) { if (indexMeta != null && indexMeta.getIndexDescriptor() != null) { - String indexMapName = indexMeta.getIndexMap(); - NitriteMap indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class); - indexMap.clear(); - indexMap.close(); + for (NitriteMap indexMap : existingLayoutMaps(indexMeta)) { + indexMap.clear(); + indexMap.close(); + } } } } } + /** + * The maps an index actually occupies in the store. {@link IndexMeta#getIndexMap()} records + * the classic map name, but a single-field index may instead live in the composite layout + * (non-unique) or the single-id layout (unique), each under a derived name of its own, and + * an index in mid-migration can briefly have two. Closing, clearing or dropping only the + * recorded map leaves the real one behind: after {@code clear()} its stale entries resolve + * to deleted documents, and a unique index rejects the very keys the collection no longer + * holds. + */ + private List> existingLayoutMaps(IndexMeta indexMeta) { + List names = new ArrayList<>(); + names.add(indexMeta.getIndexMap()); + IndexDescriptor descriptor = indexMeta.getIndexDescriptor(); + if (!descriptor.isCompoundIndex()) { + names.add(deriveCompositeIndexMapName(descriptor)); + names.add(deriveUniqueIndexMapName(descriptor)); + } + List> maps = new ArrayList<>(); + for (String name : names) { + if (nitriteStore.hasMap(name)) { + maps.add(nitriteStore.openMap(name, Object.class, Object.class)); + } + } + return maps; + } + /** * Is dirty index boolean. * @@ -174,11 +201,10 @@ IndexDescriptor createIndexDescriptor(Fields fields, String indexType) { void dropIndexDescriptor(Fields fields) { IndexMeta meta = indexMetaMap.get(fields); if (meta != null && meta.getIndexDescriptor() != null) { - String indexMapName = meta.getIndexMap(); - NitriteMap indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class); - indexMap.drop(); + for (NitriteMap indexMap : existingLayoutMaps(meta)) { + indexMap.drop(); + } } - indexMetaMap.remove(fields); updateIndexDescriptorCache(); } diff --git a/nitrite/src/main/java/org/dizitart/no2/common/util/IndexUtils.java b/nitrite/src/main/java/org/dizitart/no2/common/util/IndexUtils.java index 0fc99622..9b081d28 100644 --- a/nitrite/src/main/java/org/dizitart/no2/common/util/IndexUtils.java +++ b/nitrite/src/main/java/org/dizitart/no2/common/util/IndexUtils.java @@ -48,6 +48,17 @@ public static String deriveCompositeIndexMapName(IndexDescriptor descriptor) { return deriveIndexMapName(descriptor) + INTERNAL_NAME_SEPARATOR + "composite"; } + /** + * Derives the name of the map holding a unique index in its single-id layout, one + * {@code value -> id} entry per key. + * + * @param descriptor the index descriptor + * @return the map name + */ + public static String deriveUniqueIndexMapName(IndexDescriptor descriptor) { + return deriveIndexMapName(descriptor) + INTERNAL_NAME_SEPARATOR + "unique"; + } + public static String deriveIndexMetaMapName(String collectionName) { return INDEX_META_PREFIX + INTERNAL_NAME_SEPARATOR + collectionName; } diff --git a/nitrite/src/main/java/org/dizitart/no2/index/IndexMap.java b/nitrite/src/main/java/org/dizitart/no2/index/IndexMap.java index 1bf2214d..e62cdc64 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/IndexMap.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/IndexMap.java @@ -40,6 +40,8 @@ public class IndexMap { // (value, id) pairs (see IndexEntryKey). This IndexMap still presents the classic // value -> List view to the scanner and the filters. private NitriteMap compositeMap; + // single-id layout (unique index): values are NitriteIds, exposed as one-element lists + private boolean singleValued; @Getter @Setter @@ -79,6 +81,24 @@ public static IndexMap composite(NitriteMap compositeMap) { return new IndexMap(compositeMap, true); } + /** + * Instantiates an {@link IndexMap} over a unique index stored in the single-id layout + * ({@code value -> id}). The scanner and the filters expect a list of ids under every key, + * so each stored id is handed out as a one-element list. + * + * @param uniqueMap the backing map + * @return the index map + */ + public static IndexMap unique(NitriteMap uniqueMap) { + IndexMap indexMap = new IndexMap(uniqueMap); + indexMap.singleValued = true; + return indexMap; + } + + private static Object exposeValue(Object value, boolean singleValued) { + return singleValued && value instanceof NitriteId ? Collections.singletonList(value) : value; + } + /** * Normalizes a key returned by the backing map to the {@link DBNull} singleton * when it represents the null key. Persistent stores deserialize the stored null @@ -228,7 +248,7 @@ public Object get(DBValue dbValue) { return compositeGet(dbValue == null ? DBNull.getInstance() : dbValue); } if (nitriteMap != null) { - return nitriteMap.get(dbValue); + return exposeValue(nitriteMap.get(dbValue), singleValued); } else if (navigableMap != null) { return navigableMap.get(dbValue); } @@ -284,10 +304,11 @@ public boolean hasNext() { public Pair next() { Pair next = entryIterator.next(); DBValue dbKey = next.getFirst(); + Object value = exposeValue(next.getSecond(), singleValued); if (dbKey instanceof DBNull) { - return new Pair<>(null, next.getSecond()); + return new Pair<>(null, value); } else { - return new Pair<>(dbKey, next.getSecond()); + return new Pair<>(dbKey, value); } } }; diff --git a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java index 45dedf97..d10f48b1 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java @@ -132,8 +132,8 @@ default List addNitriteIds(List nitriteIds, FieldValues fi // ConcurrentModificationException. CopyOnWriteArrayList swaps its backing array // atomically on each mutation, so the background serializer always sees a stable // snapshot. Non-unique indexes avoid list values entirely via the composite layout - // (issue #1260); only unique indexes and the text index reach this path, where the - // per-key list is small enough that copy-on-write cost is negligible. + // (issue #1260) and unique indexes store their single id directly; only the text + // index still reaches this path. nitriteIds = new CopyOnWriteArrayList<>(); } diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index 8b86af74..d324c8eb 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -26,6 +26,7 @@ import org.dizitart.no2.common.Fields; import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.filters.ComparableFilter; +import org.dizitart.no2.exceptions.UniqueConstraintException; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.NitriteStore; @@ -37,6 +38,7 @@ import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMapName; +import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName; import static org.dizitart.no2.common.util.ObjectUtils.convertToObjectArray; /** @@ -48,6 +50,7 @@ public class SingleFieldIndex implements NitriteIndex { private final IndexDescriptor indexDescriptor; private final NitriteStore nitriteStore; private volatile boolean migrationChecked; + private volatile boolean uniqueMigrationChecked; /** * Instantiates a new {@link SingleFieldIndex}. @@ -61,9 +64,9 @@ public SingleFieldIndex(IndexDescriptor indexDescriptor, NitriteStore nitrite } /** - * The composite-key layout (issue #1260) is used for every non-unique index. Unique indexes - * keep the classic {@code value -> [id]} array layout because the uniqueness check relies on - * its single-array shape. + * The composite-key layout (issue #1260) is used for every non-unique index. A unique index + * has at most one id per key, so it stores that id directly ({@code value -> id}); the + * classic {@code value -> [id]} list layout it used before is migrated on first use. */ private boolean useCompositeLayout() { return !isUnique(); @@ -78,10 +81,15 @@ public void write(FieldValues fieldValues) { Object element = fieldValues.get(firstField); if (!useCompositeLayout()) { - // unique indexes (and stores without comparable key ordering) keep the classic - // value -> [id] layout. - NitriteMap> indexMap = findIndexMap(); - forEachElement(element, dbValue -> addIndexElement(indexMap, fieldValues, dbValue)); + // one id per key: a violation is another document already holding the key + NitriteMap indexMap = findUniqueMap(); + forEachElement(element, dbValue -> { + NitriteId existing = indexMap.get(dbValue); + if (existing != null && !existing.equals(fieldValues.getNitriteId())) { + throw new UniqueConstraintException("Unique key constraint violation for " + fields); + } + indexMap.put(dbValue, fieldValues.getNitriteId()); + }); } else { // non-unique indexes use the composite-key layout: one O(log n) point write per // (value, id) pair, instead of an O(n) read-modify-write of a shared list (issue #1260) @@ -100,8 +108,13 @@ public void remove(FieldValues fieldValues) { Object element = fieldValues.get(firstField); if (!useCompositeLayout()) { - NitriteMap> indexMap = findIndexMap(); - forEachElement(element, dbValue -> removeIndexElement(indexMap, fieldValues, dbValue)); + NitriteMap indexMap = findUniqueMap(); + forEachElement(element, dbValue -> { + NitriteId existing = indexMap.get(dbValue); + if (existing != null && existing.equals(fieldValues.getNitriteId())) { + indexMap.remove(dbValue); + } + }); } else { NitriteMap indexMap = findCompositeMap(); forEachElement(element, dbValue -> @@ -112,9 +125,10 @@ public void remove(FieldValues fieldValues) { @Override public void drop() { if (!useCompositeLayout()) { - NitriteMap> indexMap = findIndexMap(); - indexMap.clear(); - indexMap.drop(); + // drop whichever layouts exist without migrating first; nothing being dropped + // needs converting + dropMapIfPresent(deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class); + dropMapIfPresent(deriveIndexMapName(indexDescriptor), DBValue.class, ArrayList.class); } else { NitriteMap indexMap = findCompositeMap(); indexMap.clear(); @@ -129,7 +143,7 @@ public LinkedHashSet findNitriteIds(FindPlan findPlan) { IndexMap iMap = useCompositeLayout() ? IndexMap.composite(findCompositeMap()) - : new IndexMap(findIndexMap()); + : IndexMap.unique(findUniqueMap()); return scanIndex(findPlan, iMap); } @@ -147,11 +161,9 @@ public List> readSortKeys(long collectionSize) { keys.add(new Pair<>(key.getValue(), key.getNitriteId())); } } else { - for (Pair> entry : (Iterable>>) (Iterable) findIndexMap().entries()) { - for (NitriteId nitriteId : (List) entry.getSecond()) { - if (!seen.add(nitriteId)) return null; - keys.add(new Pair<>(entry.getFirst(), nitriteId)); - } + for (Pair entry : findUniqueMap().entries()) { + if (!seen.add(entry.getSecond())) return null; + keys.add(new Pair<>(entry.getFirst(), entry.getSecond())); } } @@ -180,25 +192,47 @@ private void forEachElement(Object element, java.util.function.Consumer } } - @SuppressWarnings("unchecked") - private void addIndexElement(NitriteMap> indexMap, - FieldValues fieldValues, DBValue element) { - List nitriteIds = (List) indexMap.get(element); - nitriteIds = addNitriteIds(nitriteIds, fieldValues); - indexMap.put(element, nitriteIds); + private NitriteMap findUniqueMap() { + migrateLegacyUniqueIndex(); + return nitriteStore.openMap(deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class); } + /** + * Rewrites a unique index left in the classic {@code value -> [id]} list layout into the + * single-id layout the first time the index is accessed, then drops the legacy map. The + * list layout paid a copy-on-write list per key for a list that never held more than one + * id. Idempotent and run once per index instance. + */ @SuppressWarnings("unchecked") - private void removeIndexElement(NitriteMap> indexMap, - FieldValues fieldValues, DBValue element) { - List nitriteIds = (List) indexMap.get(element); - if (nitriteIds != null && !nitriteIds.isEmpty()) { - nitriteIds.remove(fieldValues.getNitriteId()); - if (nitriteIds.size() == 0) { - indexMap.remove(element); - } else { - indexMap.put(element, nitriteIds); + private void migrateLegacyUniqueIndex() { + if (uniqueMigrationChecked) return; + synchronized (this) { + if (uniqueMigrationChecked) return; + String legacyName = deriveIndexMapName(indexDescriptor); + if (nitriteStore.hasMap(legacyName)) { + NitriteMap> legacy = findIndexMap(); + if (!legacy.isEmpty()) { + NitriteMap unique = nitriteStore.openMap( + deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class); + for (Pair> entry : (Iterable>>) (Iterable) legacy.entries()) { + List nitriteIds = (List) entry.getSecond(); + if (nitriteIds != null && !nitriteIds.isEmpty()) { + unique.put(entry.getFirst(), nitriteIds.get(0)); + } + } + } + legacy.clear(); + legacy.drop(); } + uniqueMigrationChecked = true; + } + } + + private void dropMapIfPresent(String mapName, Class keyType, Class valueType) { + if (nitriteStore.hasMap(mapName)) { + NitriteMap map = nitriteStore.openMap(mapName, keyType, valueType); + map.clear(); + map.drop(); } } diff --git a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java index 05c54b44..9942dba5 100644 --- a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java @@ -22,10 +22,12 @@ import org.dizitart.no2.common.DBValue; import org.dizitart.no2.common.FieldValues; import org.dizitart.no2.common.Fields; +import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.filters.ComparableFilter; import org.dizitart.no2.filters.IndexScanFilter; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.memory.InMemoryStore; +import org.dizitart.no2.exceptions.UniqueConstraintException; import org.junit.Test; import java.util.ArrayList; @@ -33,10 +35,12 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import static org.dizitart.no2.common.tuples.Pair.pair; import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMapName; +import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName; import static org.dizitart.no2.filters.FluentFilter.where; import static org.junit.Assert.*; @@ -107,5 +111,99 @@ public void testLegacyArrayIndexMigratedToComposite() { assertFalse(store.hasMap(legacyName)); assertTrue(store.hasMap(deriveCompositeIndexMapName(desc))); } -} + @Test + public void testUniqueIndexKeepsOneIdPerKey() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("a"), "c"); + SingleFieldIndex index = new SingleFieldIndex(desc, store); + + index.write(values(1L, "a", "k")); + try { + index.write(values(2L, "a", "k")); + fail("a second document under the same key must be rejected"); + } catch (UniqueConstraintException expected) { + // ok + } + // the same document again is not a violation + index.write(values(1L, "a", "k")); + + NitriteMap unique = store.openMap(deriveUniqueIndexMapName(desc), DBValue.class, NitriteId.class); + assertEquals(NitriteId.createId(1L), unique.get(new DBValue("k"))); + assertFalse("no list-layout map is created for a new unique index", store.hasMap(deriveIndexMapName(desc))); + assertEquals(Collections.singletonList(NitriteId.createId(1L)), new ArrayList<>(index.findNitriteIds(eqPlan(desc, "a", "k")))); + + // another document's id must not remove the entry + index.remove(values(2L, "a", "k")); + assertEquals(NitriteId.createId(1L), unique.get(new DBValue("k"))); + index.remove(values(1L, "a", "k")); + assertNull(unique.get(new DBValue("k"))); + index.write(values(2L, "a", "k")); + assertEquals(NitriteId.createId(2L), unique.get(new DBValue("k"))); + } + + @Test + public void testLegacyUniqueListLayoutMigratesOnFirstUse() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("a"), "c"); + NitriteMap> legacy = store.openMap(deriveIndexMapName(desc), DBValue.class, ArrayList.class); + legacy.put(new DBValue("k"), new CopyOnWriteArrayList<>(Collections.singletonList(NitriteId.createId(7L)))); + + SingleFieldIndex index = new SingleFieldIndex(desc, store); + assertEquals(Collections.singletonList(NitriteId.createId(7L)), new ArrayList<>(index.findNitriteIds(eqPlan(desc, "a", "k")))); + + assertFalse("legacy map is dropped after migration", store.hasMap(deriveIndexMapName(desc))); + NitriteMap unique = store.openMap(deriveUniqueIndexMapName(desc), DBValue.class, NitriteId.class); + assertEquals(NitriteId.createId(7L), unique.get(new DBValue("k"))); + try { + index.write(values(8L, "a", "k")); + fail("uniqueness must hold over migrated entries"); + } catch (UniqueConstraintException expected) { + // ok + } + } + + @Test + public void testUniqueDropRemovesBothLayouts() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("a"), "c"); + store.openMap(deriveIndexMapName(desc), DBValue.class, ArrayList.class) + .put(new DBValue("old"), new CopyOnWriteArrayList<>(Collections.singletonList(NitriteId.createId(1L)))); + SingleFieldIndex index = new SingleFieldIndex(desc, store); + index.write(values(2L, "a", "new")); + + index.drop(); + + assertFalse(store.hasMap(deriveIndexMapName(desc))); + assertFalse(store.hasMap(deriveUniqueIndexMapName(desc))); + } + + @Test + public void testReadSortKeysOfUniqueIndex() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("a"), "c"); + SingleFieldIndex index = new SingleFieldIndex(desc, store); + index.write(values(3L, "a", "c")); + index.write(values(1L, "a", "a")); + index.write(values(2L, "a", "b")); + + List> keys = index.readSortKeys(3); + assertEquals(Arrays.asList(NitriteId.createId(1L), NitriteId.createId(2L), NitriteId.createId(3L)), + keys.stream().map(Pair::getSecond).collect(java.util.stream.Collectors.toList())); + assertNull("an index that does not cover every document cannot stand in for it", index.readSortKeys(4)); + } + + private static FieldValues values(long id, String field, Object value) { + FieldValues fieldValues = new FieldValues(); + fieldValues.setNitriteId(NitriteId.createId(id)); + fieldValues.getValues().add(pair(field, value)); + return fieldValues; + } + + private static FindPlan eqPlan(IndexDescriptor desc, String field, Object value) { + FindPlan plan = new FindPlan(); + plan.setIndexDescriptor(desc); + plan.setIndexScanFilter(new IndexScanFilter(Collections.singletonList((ComparableFilter) where(field).eq(value)))); + return plan; + } +} diff --git a/nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionDeleteTest.java b/nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionDeleteTest.java index 541621ce..917c12b9 100644 --- a/nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionDeleteTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionDeleteTest.java @@ -17,6 +17,7 @@ package org.dizitart.no2.integration.collection; +import org.dizitart.no2.collection.Document; import org.dizitart.no2.collection.DocumentCursor; import org.dizitart.no2.common.WriteResult; import org.dizitart.no2.filters.Filter; @@ -125,4 +126,24 @@ public void testRemoveDocument() { assertEquals(collection.find(where("firstName").eq("fn2")).size(), 0); assertEquals(collection.find(where("firstName").eq("fn3")).size(), 1); } + + @Test + public void testClearEmptiesEveryIndexLayout() { + collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "lastName"); + collection.createIndex(IndexOptions.indexOptions(IndexType.UNIQUE), "firstName"); + collection.insert(Document.createDocument("firstName", "fn").put("lastName", "ln"), + Document.createDocument("firstName", "fn2").put("lastName", "ln")); + assertEquals(2, collection.find(where("lastName").eq("ln")).toList().size()); + + collection.clear(); + assertEquals(0, collection.find(where("lastName").eq("ln")).size()); + assertEquals(0, collection.find(where("firstName").eq("fn")).size()); + + // fresh documents with the same keys: no stale rows in the composite layout, no stale id in the unique one + collection.insert(Document.createDocument("firstName", "fn").put("lastName", "ln"), + Document.createDocument("firstName", "fn2").put("lastName", "ln")); + assertEquals(2, collection.find(where("lastName").eq("ln")).size()); + assertEquals(2, collection.find(where("lastName").eq("ln")).toList().size()); + assertEquals(1, collection.find(where("firstName").eq("fn")).toList().size()); + } }