This is an automated email from the ASF dual-hosted git repository. thomasm pushed a commit to branch OAK-10490 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 9bad1ccb72dcb436246596e5462a35c1e4e6cb68 Author: Thomas Mueller <[email protected]> AuthorDate: Thu Oct 12 16:27:23 2023 +0200 OAK-10490 Suggest queries return duplicate entries if prefetch is enabled --- .../index/search/spi/query/FulltextIndex.java | 9 +- .../search/spi/query/FulltextIndexPlanner.java | 2 +- .../index/search/spi/query/FulltextIndexTest.java | 161 ++++++++++++++++++++- 3 files changed, 165 insertions(+), 7 deletions(-) diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java index c0807ad676..caab37045d 100644 --- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java +++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java @@ -400,7 +400,7 @@ public abstract class FulltextIndex implements AdvancedQueryIndex, QueryIndex, N private final Cursor pathCursor; private final String pathPrefix; - FulltextResultRow currentRow; + FulltextResultRow currentRowInPathIterator; private final SizeEstimator sizeEstimator; private long estimatedSize; private final int numberOfFacets; @@ -425,7 +425,7 @@ public abstract class FulltextIndex implements AdvancedQueryIndex, QueryIndex, N readCount = 0; rewoundCount = iterStateProvider.rewoundCount(); } - currentRow = it.next(); + currentRowInPathIterator = it.next(); readCount++; if (readCount % TRAVERSING_WARNING == 0) { Cursors.checkReadLimit(readCount, settings); @@ -436,7 +436,7 @@ public abstract class FulltextIndex implements AdvancedQueryIndex, QueryIndex, N log.warn("Index-Traversed {} nodes with filter {}", readCount, plan.getFilter()); } } - return currentRow.path; + return currentRowInPathIterator.path; } @Override @@ -465,6 +465,9 @@ public abstract class FulltextIndex implements AdvancedQueryIndex, QueryIndex, N @Override public IndexRow next() { final IndexRow pathRow = pathCursor.next(); + // we need to copy the reference to the current row here, + // otherwise all the returned IndexRows might reference the same row + final FulltextResultRow currentRow = currentRowInPathIterator; return new IndexRow() { @Override diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java index 8ca45cb0b7..66d4130c5f 100644 --- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java +++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java @@ -1163,7 +1163,7 @@ public class FulltextIndexPlanner { nodeNameRestriction = true; } - private void disableUniquePaths(){ + public void disableUniquePaths(){ uniquePathsRequired = false; } } diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTest.java index 9bb4f8b159..316081b597 100644 --- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTest.java +++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTest.java @@ -18,11 +18,32 @@ */ package org.apache.jackrabbit.oak.plugins.index.search.spi.query; -import org.junit.Test; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; +import org.apache.jackrabbit.oak.plugins.index.search.SizeEstimator; +import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex.FulltextPathCursor; +import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex.FulltextResultRow; +import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex.IteratorRewoundStateProvider; +import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner.PlanResult; +import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.spi.query.Filter; +import org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction; +import org.apache.jackrabbit.oak.spi.query.IndexRow; +import org.apache.jackrabbit.oak.spi.query.QueryConstants; +import org.apache.jackrabbit.oak.spi.query.QueryIndex.IndexPlan; +import org.apache.jackrabbit.oak.spi.query.QueryIndex.OrderEntry; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.jetbrains.annotations.Nullable; +import org.junit.Test; + public class FulltextIndexTest { @Test @@ -36,7 +57,141 @@ public class FulltextIndexTest { field = FulltextIndex.parseFacetField("rep:facet(jcr:primaryType)"); assertNotNull(field); assertEquals("jcr:primaryType", field); + } + + /** + * Test that we can read the rows first, and then read the data from the rows. + */ + @Test + public void fulltextPathCursorTest() { + ArrayList<FulltextResultRow> rowList = new ArrayList<>(); + String s1 = "s1", s2 = "s2"; + rowList.add(new FulltextResultRow(s1, 1)); + rowList.add(new FulltextResultRow(s2, 2)); + FulltextPathCursor fulltextPathCursor = new TestFulltextPathCursor(rowList.iterator()); + IndexRow row1 = fulltextPathCursor.next(); + IndexRow row2 = fulltextPathCursor.next(); + String suggest1 = row1.getValue(QueryConstants.REP_SUGGEST).getValue(Type.STRING); + String suggest2 = row2.getValue(QueryConstants.REP_SUGGEST).getValue(Type.STRING); + assertEquals(s1, suggest1); + assertEquals(s2, suggest2); } -} + static class TestFulltextPathCursor extends FulltextPathCursor { + public TestFulltextPathCursor(Iterator<FulltextResultRow> it) { + super(it, new IteratorRewoundStateProvider() { + + @Override + public int rewoundCount() { + return 0; + } + + }, new TestIndexPlan(), new QueryEngineSettings(), new SizeEstimator() { + + @Override + public long getSize() { + return 0; + } + + }); + } + + } + + static class TestIndexPlan implements IndexPlan { + + @Override + public double getCostPerExecution() { + return 0; + } + + @Override + public double getCostPerEntry() { + return 0; + } + + @Override + public long getEstimatedEntryCount() { + return 0; + } + + @Override + public Filter getFilter() { + return null; + } + + @Override + public void setFilter(Filter filter) { + } + + @Override + public boolean isDelayed() { + return false; + } + + @Override + public boolean isFulltextIndex() { + return false; + } + + @Override + public boolean includesNodeData() { + return false; + } + + @Override + public List<OrderEntry> getSortOrder() { + return null; + } + + @Override + public NodeState getDefinition() { + return null; + } + + @Override + public String getPathPrefix() { + return null; + } + + @Override + public boolean getSupportsPathRestriction() { + return false; + } + + @Override + public @Nullable PropertyRestriction getPropertyRestriction() { + return null; + } + + @Override + public IndexPlan copy() { + return null; + } + + @Override + public @Nullable Object getAttribute(String name) { + if (name.equals(FulltextIndex.ATTR_PLAN_RESULT)) { + IndexDefinition indexDef = new IndexDefinition(EmptyNodeState.EMPTY_NODE, EmptyNodeState.EMPTY_NODE, + ""); + PlanResult pr = new PlanResult("test", indexDef, null); + pr.disableUniquePaths(); + return pr; + } + return null; + } + + @Override + public @Nullable String getPlanName() { + return null; + } + + @Override + public boolean isDeprecated() { + return false; + } + + } + +}
