From 9a9a3fbea29b88be46d0cd0a748e47e4390036f4 Mon Sep 17 00:00:00 2001 From: Balmukund Trivedi Date: Tue, 11 Aug 2026 14:46:16 -0700 Subject: [PATCH] Test index intersection membership against a Set (#4932) SubqueryIterator streams the results of the first index and keeps the elements which the other indexes also returned. Membership was tested with List.contains inside the filter, so each streamed element scanned the other list from the start, which costs O(n*m). The list is deliberately unbounded. StandardJanusGraphTx passes Query.NO_LIMIT to processIntersectingRetrievals so that the intersection is complete and no result is missed, which makes subLimit Integer.MAX_VALUE and materialises the whole matching set of every other index. Build the set once outside the filter, which makes the intersection O(n + m). Signed-off-by: Balmukund Trivedi Co-Authored-By: Claude Opus 5 (1M context) --- .../graphdb/util/SubqueryIterator.java | 8 +- .../graphdb/util/SubqueryIteratorTest.java | 116 ++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 janusgraph-core/src/test/java/org/janusgraph/graphdb/util/SubqueryIteratorTest.java diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/util/SubqueryIterator.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/util/SubqueryIterator.java index ea27a4f274..a1d11625f4 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/graphdb/util/SubqueryIterator.java +++ b/janusgraph-core/src/main/java/org/janusgraph/graphdb/util/SubqueryIterator.java @@ -29,8 +29,10 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.function.Function; import java.util.stream.Stream; @@ -73,8 +75,12 @@ public SubqueryIterator(JointIndexQuery.Subquery subQuery, IndexSerializer index throw new JanusGraphException("Could not call index", e); } } + //Membership is tested once for every element the first index returns, and otherResults is deliberately + //unbounded: StandardJanusGraphTx passes NO_LIMIT to processIntersectingRetrievals so that the intersection is + //complete. Scanning the list for each element would make the intersection cost O(n*m) + final Set otherResultSet = otherResults == null ? null : new HashSet<>(otherResults); elementIterator = stream - .filter(e -> otherResults == null || otherResults.contains(e)) + .filter(e -> otherResultSet == null || otherResultSet.contains(e)) .map(e -> { JanusGraphElement r = function.apply(e); if (r == null) { diff --git a/janusgraph-core/src/test/java/org/janusgraph/graphdb/util/SubqueryIteratorTest.java b/janusgraph-core/src/test/java/org/janusgraph/graphdb/util/SubqueryIteratorTest.java new file mode 100644 index 0000000000..44c911b0da --- /dev/null +++ b/janusgraph-core/src/test/java/org/janusgraph/graphdb/util/SubqueryIteratorTest.java @@ -0,0 +1,116 @@ +// Copyright 2026 JanusGraph Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.janusgraph.graphdb.util; + +import org.janusgraph.core.JanusGraphElement; +import org.janusgraph.diskstorage.BackendTransaction; +import org.janusgraph.graphdb.database.IndexSerializer; +import org.janusgraph.graphdb.query.graph.JointIndexQuery; +import org.janusgraph.graphdb.query.profile.QueryProfiler; +import org.janusgraph.graphdb.transaction.StandardJanusGraphTx; +import org.janusgraph.graphdb.transaction.subquerycache.SubqueryCache; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +//When a query is covered by more than one index, StandardJanusGraphTx intersects the results: the first index is +//streamed, and every element is checked for membership in the results the other indexes agreed on. That other set is +//deliberately unbounded, because NO_LIMIT is passed to processIntersectingRetrievals to keep the intersection +//complete, so the membership check has to be cheap. +public class SubqueryIteratorTest { + + private static final long ID_ONLY_IN_FIRST_INDEX = 1L; + private static final long ID_IN_EVERY_INDEX = 2L; + private static final long ID_ONLY_IN_OTHER_INDEXES = 99L; + + private List streamedIds(List firstIndexResults, List otherResults, int limit) { + final JointIndexQuery.Subquery subQuery = mock(JointIndexQuery.Subquery.class); + when(subQuery.getProfiler()).thenReturn(QueryProfiler.NO_OP); + + final IndexSerializer indexSerializer = mock(IndexSerializer.class); + when(indexSerializer.query(any(), any(), any())).thenReturn(firstIndexResults.stream()); + + //A mock returns an empty List rather than null, which would look like a cache hit holding no results + final SubqueryCache indexCache = mock(SubqueryCache.class); + when(indexCache.getIfPresent(any())).thenReturn(null); + + final List returnedIds = new ArrayList<>(); + try (SubqueryIterator iterator = new SubqueryIterator(subQuery, indexSerializer, + mock(BackendTransaction.class), mock(StandardJanusGraphTx.class), indexCache, limit, + id -> { + //The conversion function turns an id into an element. Its identity does not matter here, only which + //ids reach it + returnedIds.add(id); + return mock(JanusGraphElement.class); + }, otherResults)) { + iterator.forEachRemaining(element -> { }); + } + return returnedIds; + } + + @Test + public void shouldKeepOnlyTheIdsPresentInTheOtherIndexResults() { + final List streamed = streamedIds( + Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_IN_EVERY_INDEX), + Arrays.asList(ID_IN_EVERY_INDEX, ID_ONLY_IN_OTHER_INDEXES), + Integer.MAX_VALUE); + assertEquals(Collections.singletonList(ID_IN_EVERY_INDEX), streamed); + } + + @Test + public void shouldKeepEveryIdWhenThereIsNoOtherIndex() { + //A single index query passes null, which must not be treated as an empty intersection + final List firstIndexResults = Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_IN_EVERY_INDEX); + assertEquals(firstIndexResults, streamedIds(firstIndexResults, null, Integer.MAX_VALUE)); + } + + @Test + public void shouldKeepNoIdWhenTheOtherIndexesAgreedOnNothing() { + assertEquals(Collections.emptyList(), streamedIds( + Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_IN_EVERY_INDEX), Collections.emptyList(), Integer.MAX_VALUE)); + } + + @Test + public void shouldStopAtTheLimit() { + final List otherResults = Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_IN_EVERY_INDEX); + assertEquals(Collections.singletonList(ID_ONLY_IN_FIRST_INDEX), streamedIds( + Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_IN_EVERY_INDEX), otherResults, 1)); + } + + @Test + public void shouldNotBeConfusedByADuplicateInTheOtherIndexResults() { + //processIntersectingRetrievals returns a List, so a repeated id is possible. Membership is all that matters + assertEquals(Collections.singletonList(ID_IN_EVERY_INDEX), streamedIds( + Collections.singletonList(ID_IN_EVERY_INDEX), + Arrays.asList(ID_IN_EVERY_INDEX, ID_IN_EVERY_INDEX), Integer.MAX_VALUE)); + } + + @Test + public void shouldNotDependOnStreamOrderMatchingTheOtherIndexOrder() { + //The streamed order is preserved, and is independent of the order the other indexes reported + assertEquals(Arrays.asList(ID_IN_EVERY_INDEX, ID_ONLY_IN_FIRST_INDEX), streamedIds( + Arrays.asList(ID_IN_EVERY_INDEX, ID_ONLY_IN_FIRST_INDEX), + Arrays.asList(ID_ONLY_IN_FIRST_INDEX, ID_ONLY_IN_OTHER_INDEXES, ID_IN_EVERY_INDEX), + Integer.MAX_VALUE)); + } +}