This is an automated email from the ASF dual-hosted git repository.
thomasm pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 763031951c OAK-10490 Suggest queries return duplicate entries if
prefetch is enabled (#1148)
763031951c is described below
commit 763031951c3962aae875ec315d961ab14798dc56
Author: Thomas Mueller <[email protected]>
AuthorDate: Fri Oct 13 13:44:01 2023 +0200
OAK-10490 Suggest queries return duplicate entries if prefetch is enabled
(#1148)
* OAK-10490 Suggest queries return duplicate entries if prefetch is enabled
* OAK-10490 Suggest queries return duplicate entries if prefetch is enabled
* OAK-10490 Suggest queries return duplicate entries if prefetch is enabled
* OAK-10490 Suggest queries return duplicate entries if prefetch is enabled
---
.../jackrabbit/oak/query/QueryEngineSettings.java | 2 +-
.../index/search/spi/query/FulltextIndex.java | 152 +++++++++++--------
.../search/spi/query/FulltextIndexPlanner.java | 2 +-
.../index/search/spi/query/FulltextIndexTest.java | 161 ++++++++++++++++++++-
4 files changed, 248 insertions(+), 69 deletions(-)
diff --git
a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
index 0800f5520b..0bf4a9a48d 100644
---
a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
+++
b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
@@ -61,7 +61,7 @@ public class QueryEngineSettings implements
QueryEngineSettingsMBean, QueryLimit
public static final String OAK_QUERY_PREFETCH_COUNT = "oak.prefetchCount";
- public static final String FT_NAME_PREFETCH_FOR_QUERIES = "FT_OAK-9893";
+ public static final String FT_NAME_PREFETCH_FOR_QUERIES = "FT_OAK-10490";
public static final int DEFAULT_PREFETCH_COUNT =
Integer.getInteger(OAK_QUERY_PREFETCH_COUNT, -1);
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..6d1b2663bb 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,11 +400,16 @@ public abstract class FulltextIndex implements
AdvancedQueryIndex, QueryIndex, N
private final Cursor pathCursor;
private final String pathPrefix;
- FulltextResultRow currentRow;
private final SizeEstimator sizeEstimator;
- private long estimatedSize;
private final int numberOfFacets;
+ // the cached value
+ private long estimatedSize;
+
+ // the current row in the pathCursor
+ // (so we don't have to extend the PathCursor)
+ FulltextResultRow currentRowInPathIterator;
+
public FulltextPathCursor(final Iterator<FulltextResultRow> it, final
IteratorRewoundStateProvider iterStateProvider,
final IndexPlan plan, QueryLimits settings,
SizeEstimator sizeEstimator) {
pathPrefix = plan.getPathPrefix();
@@ -425,7 +430,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 +441,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
@@ -464,68 +469,13 @@ public abstract class FulltextIndex implements
AdvancedQueryIndex, QueryIndex, N
@Override
public IndexRow next() {
- final IndexRow pathRow = pathCursor.next();
- return new IndexRow() {
-
- @Override
- public boolean isVirtualRow() {
- return currentRow.isVirutal;
- }
-
- @Override
- public String getPath() {
- String sub = pathRow.getPath();
- if (isVirtualRow()) {
- return sub;
- } else if (!"".equals(pathPrefix) &&
PathUtils.denotesRoot(sub)) {
- return pathPrefix;
- } else if (PathUtils.isAbsolute(sub)) {
- return pathPrefix + sub;
- } else {
- return PathUtils.concat(pathPrefix, sub);
- }
- }
+ IndexRow pathRow = pathCursor.next();
- @Override
- public PropertyValue getValue(String columnName) {
- // overlay the score
- if (QueryConstants.JCR_SCORE.equals(columnName)) {
- return PropertyValues.newDouble(currentRow.score);
- }
- if (QueryConstants.REP_SPELLCHECK.equals(columnName) ||
QueryConstants.REP_SUGGEST.equals(columnName)) {
- return PropertyValues.newString(currentRow.suggestion);
- }
- if
(QueryConstants.OAK_SCORE_EXPLANATION.equals(columnName)) {
- return
PropertyValues.newString(currentRow.explanation);
- }
- if (columnName.startsWith(QueryConstants.REP_EXCERPT)) {
- String excerpt = currentRow.excerpts.get(columnName);
- if (excerpt != null) {
- return PropertyValues.newString(excerpt);
- }
- }
- if (columnName.startsWith(QueryConstants.REP_FACET)) {
- try {
- List<Facet> facets =
currentRow.getFacets(numberOfFacets, columnName);
- if (facets != null) {
- JsopWriter writer = new JsopBuilder();
- writer.object();
- for (Facet f : facets) {
-
writer.key(f.getLabel()).value(f.getCount());
- }
- writer.endObject();
- return
PropertyValues.newString(writer.toString());
- }
- } catch (IOException | RuntimeException e) {
- LOG.warn(e.getMessage());
- LOG.debug(e.getMessage(), e);
- throw new RuntimeException(e);
- }
- }
- return pathRow.getValue(columnName);
- }
+ // currentRowInPathIterator is changed as a side effect
+ // of calling pathCursor.next()
+ FulltextResultRow row = currentRowInPathIterator;
- };
+ return new IndexResultRow(pathRow, row, pathPrefix,
numberOfFacets);
}
@Override
@@ -537,6 +487,80 @@ public abstract class FulltextIndex implements
AdvancedQueryIndex, QueryIndex, N
}
}
+ private static class IndexResultRow implements IndexRow {
+
+ private final IndexRow pathRow;
+ private final FulltextResultRow resultRow;
+ private final String pathPrefix;
+ private final int numberOfFacets;
+
+ IndexResultRow(IndexRow pathRow, FulltextResultRow row, String
pathPrefix, int numberOfFacets) {
+ this.pathRow = pathRow;
+ this.resultRow = row;
+ this.pathPrefix = pathPrefix;
+ this.numberOfFacets = numberOfFacets;
+ }
+
+ @Override
+ public boolean isVirtualRow() {
+ return resultRow.isVirutal;
+ }
+
+ @Override
+ public String getPath() {
+ String sub = pathRow.getPath();
+ if (isVirtualRow()) {
+ return sub;
+ } else if (!"".equals(pathPrefix) && PathUtils.denotesRoot(sub)) {
+ return pathPrefix;
+ } else if (PathUtils.isAbsolute(sub)) {
+ return pathPrefix + sub;
+ } else {
+ return PathUtils.concat(pathPrefix, sub);
+ }
+ }
+
+ @Override
+ public PropertyValue getValue(String columnName) {
+ // overlay the score
+ if (QueryConstants.JCR_SCORE.equals(columnName)) {
+ return PropertyValues.newDouble(resultRow.score);
+ }
+ if (QueryConstants.REP_SPELLCHECK.equals(columnName) ||
QueryConstants.REP_SUGGEST.equals(columnName)) {
+ return PropertyValues.newString(resultRow.suggestion);
+ }
+ if (QueryConstants.OAK_SCORE_EXPLANATION.equals(columnName)) {
+ return PropertyValues.newString(resultRow.explanation);
+ }
+ if (columnName.startsWith(QueryConstants.REP_EXCERPT)) {
+ String excerpt = resultRow.excerpts.get(columnName);
+ if (excerpt != null) {
+ return PropertyValues.newString(excerpt);
+ }
+ }
+ if (columnName.startsWith(QueryConstants.REP_FACET)) {
+ try {
+ List<Facet> facets = resultRow.getFacets(numberOfFacets,
columnName);
+ if (facets != null) {
+ JsopWriter writer = new JsopBuilder();
+ writer.object();
+ for (Facet f : facets) {
+ writer.key(f.getLabel()).value(f.getCount());
+ }
+ writer.endObject();
+ return PropertyValues.newString(writer.toString());
+ }
+ } catch (IOException | RuntimeException e) {
+ LOG.warn(e.getMessage());
+ LOG.debug(e.getMessage(), e);
+ throw new RuntimeException(e);
+ }
+ }
+ return pathRow.getValue(columnName);
+ }
+
+ };
+
public interface IteratorRewoundStateProvider {
int rewoundCount();
}
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;
+ }
+
+ }
+
+}
