From ad529e268ccdbaf88077bb099d577e7ffd9f4018 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 7 Sep 2026 13:00:25 -0700 Subject: [PATCH 1/3] Don't defer any index creations --- .../src/org/labkey/experiment/api/ExpMaterialTableImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 6ff9ce9f4f0..e5bc1cfddeb 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -1373,11 +1373,11 @@ private _MaterializedQueryHelper getOrCreateMQH() // be added as a followup step .addIndex("CREATE UNIQUE INDEX uq_${NAME}_rowid ON temp.${NAME} (rowid)") .addIndex("CREATE INDEX idx_${NAME}_container ON temp.${NAME} (container)") - .addDeferredIndex("CREATE INDEX idx_${NAME}_root ON temp.${NAME} (rootmaterialrowid)") + .addIndex("CREATE INDEX idx_${NAME}_root ON temp.${NAME} (rootmaterialrowid)") // Deferred despite being UNIQUE. Source data guarantees uniqueness, and this is very expensive to build - .addDeferredIndex("CREATE UNIQUE INDEX uq_${NAME}_lsid ON temp.${NAME} (lsid)"); + .addIndex("CREATE UNIQUE INDEX uq_${NAME}_lsid ON temp.${NAME} (lsid)"); - getDomainIndexDdl().forEach(builder::addDeferredIndex); + getDomainIndexDdl().forEach(builder::addIndex); if (isIncrementalUpdateDisabled()) builder.addInvalidCheck(() -> String.valueOf(getInvalidateCounters(_ss.getLSID()).update.get())); From 673c0915eac13289287f6c42030521df5007edfd Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 7 Sep 2026 13:28:59 -0700 Subject: [PATCH 2/3] Fix comment --- .../src/org/labkey/experiment/api/ExpMaterialTableImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index e5bc1cfddeb..e6cb7707a7c 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -1374,7 +1374,6 @@ private _MaterializedQueryHelper getOrCreateMQH() .addIndex("CREATE UNIQUE INDEX uq_${NAME}_rowid ON temp.${NAME} (rowid)") .addIndex("CREATE INDEX idx_${NAME}_container ON temp.${NAME} (container)") .addIndex("CREATE INDEX idx_${NAME}_root ON temp.${NAME} (rootmaterialrowid)") - // Deferred despite being UNIQUE. Source data guarantees uniqueness, and this is very expensive to build .addIndex("CREATE UNIQUE INDEX uq_${NAME}_lsid ON temp.${NAME} (lsid)"); getDomainIndexDdl().forEach(builder::addIndex); From cc239d3da43b94f815e2d166c7d2af3e67f02e9a Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 7 Sep 2026 14:17:53 -0700 Subject: [PATCH 3/3] Avoid caching misses when a subsequent row could cause it to become a hit --- .../api/dataiterator/SimpleTranslator.java | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index b17fc536299..32a0943dc06 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java @@ -203,6 +203,7 @@ public static class RemapConverter private boolean _includePkLookup; // if true, will perform an initial PK lookup before attempting the AK lookup private final boolean _allowBulkLoads; + private final boolean _cacheMisses; private final Set> _bulkLoads = new HashSet<>(); private List>> _maps = null; @@ -210,11 +211,18 @@ public static class RemapConverter private Pair> _pkColumnLookupMap = null; public RemapConverter(@NotNull TableInfo targetTable, boolean includeTitleColumn, boolean allowBulkLoads, boolean includePkLookup) + { + this(targetTable, includeTitleColumn, allowBulkLoads, includePkLookup, true); + } + + /** @param cacheMisses false when the same operation writes the lookup target, so an unresolved key must be re-queried instead of memoized */ + public RemapConverter(@NotNull TableInfo targetTable, boolean includeTitleColumn, boolean allowBulkLoads, boolean includePkLookup, boolean cacheMisses) { _targetTable = targetTable; _includeTitleColumn = includeTitleColumn; _allowBulkLoads = allowBulkLoads; _includePkLookup = includePkLookup; + _cacheMisses = cacheMisses; } public void setIncludePkLookup(boolean includePkLookup) @@ -371,11 +379,14 @@ private Object fetch(Triple> triple, vs = bulkLoaded; } - // ArrayListValuedHashMap returns an empty collection if 'k' is not in the map. - // If there are no values in the database, stash a MISS marker to avoid re-fetching. assert vs != null; if (vs.isEmpty()) + { + if (!_cacheMisses) + return null; + map.put(k, MISS); + } } Object v = getSingleValue(k, vs); @@ -414,6 +425,7 @@ private Object fetch(Pair> pair, Object k) { if (k == null || (k instanceof String strKey && !GUID.isGUID(strKey))) { + // Not a data miss: a key that isn't a GUID can never become one, so memoize regardless of _cacheMisses map.put(k, MISS); return null; } @@ -434,7 +446,8 @@ private Object fetch(Pair> pair, Object k) return map.get(k); else { - map.put(k, MISS); + if (_cacheMisses) + map.put(k, MISS); return null; } } @@ -949,7 +962,8 @@ public RemappingConvertColumn(final @NotNull SimpleConvertColumn convertCol, fin _toCol = toCol; _missing = missing; _includeTitleColumn = includeTitleColumn; - _remapper = new RemapConverter(_toCol.getFkTableInfo(), _includeTitleColumn, false, true); + // The import can create the rows this resolves against, so a miss must not outlive the row that saw it + _remapper = new RemapConverter(_toCol.getFkTableInfo(), _includeTitleColumn, false, true, false); _lookupResolutionType = lookupResolutionType; } @@ -2330,6 +2344,33 @@ public void remapPkLookupMapIsRetained() assertSame("pk lookup map was rebuilt rather than retained", pkMap, converter.pkLookupMap()); } + @Test + public void remapMissIsNotMemoizedWhenCacheMissesIsOff() + { + RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, true, false); + MultiValuedMap cache = converter.getMaps().getFirst().getRight(); + + String absent = "no-enum-value-supplies-this"; + assertNull(converter.mappedValue(absent)); + assertFalse("miss was memoized, so a row the import adds later can never resolve", cache.containsKey(absent)); + + // Stands in for the row appearing after the first lookup + Integer added = 42; + cache.put(absent, added); + assertEquals(added, converter.mappedValue(absent)); + } + + @Test + public void remapMissIsMemoizedByDefault() + { + RemapConverter converter = new RemapConverter(remapLookupTable(), true, false, true); + MultiValuedMap cache = converter.getMaps().getFirst().getRight(); + + String absent = "no-enum-value-supplies-this"; + assertNull(converter.mappedValue(absent)); + assertTrue("callers that don't write the lookup target rely on the MISS marker to avoid re-querying", cache.containsKey(absent)); + } + @Test public void getFileRootSubstitutedFilePathTest() {