diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/TraversingIndex.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/TraversingIndex.java index 00a7f3f..cad4f4d 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/TraversingIndex.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/TraversingIndex.java @@ -28,6 +28,8 @@ import org.apache.jackrabbit.oak.spi.query.Filter.PathRestriction; import org.apache.jackrabbit.oak.spi.query.QueryIndex; import org.apache.jackrabbit.oak.spi.state.NodeState; +import static org.apache.jackrabbit.oak.spi.query.QueryConstants.REP_FACET; + /** * An index that traverses over a given subtree. */ @@ -79,6 +81,11 @@ public class TraversingIndex implements QueryIndex { // not an appropriate index for native search return Double.POSITIVE_INFINITY; } + Filter.PropertyRestriction facetRestriction = filter.getPropertyRestriction(REP_FACET); + if (facetRestriction != null) { + // not an appropriate index for facets + return Double.POSITIVE_INFINITY; + } if (filter.isAlwaysFalse()) { return 0; } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexTest.java index d791793..49a116f 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexTest.java @@ -19,8 +19,11 @@ package org.apache.jackrabbit.oak.query.index; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.spi.query.QueryConstants.REP_FACET; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Arrays; @@ -28,8 +31,10 @@ import java.util.Collections; import java.util.List; import org.apache.jackrabbit.oak.spi.query.Cursor; +import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.junit.Assert; import org.junit.Test; /** @@ -79,4 +84,12 @@ public class TraversingIndexTest { assertFalse(c.hasNext()); } + @Test + public void testFacets() { + TraversingIndex traversingIndex = new TraversingIndex(); + Filter mockFilter = mock(Filter.class); + when(mockFilter.getPropertyRestriction(REP_FACET)).thenReturn(mock(Filter.PropertyRestriction.class)); + Assert.assertEquals(traversingIndex.getCost(mockFilter, null), Double.POSITIVE_INFINITY, 0.001); + } + } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java index 1a0acb4..1c9b85c 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java @@ -34,6 +34,7 @@ import java.util.stream.Collectors; import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; import org.apache.jackrabbit.core.query.AbstractQueryTest; +import org.apache.jackrabbit.oak.plugins.index.IndexConstants; import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants; import org.apache.jackrabbit.oak.query.facet.FacetResult; import org.junit.After; @@ -850,6 +851,30 @@ public class FacetTest extends AbstractQueryTest { assertEquals("Unexpected facet labels", newHashSet("t1", "t2", "t3"), facetLabels); } + /** + * Tests the scenario where we have a facet query and the traversal cost is less than the index cost that + * serves the facet query. TraversingIndex should not be used for facets. + * @throws Exception in case of errors. + */ + public void testTraversalNotAllowed() throws Exception { + // Increase cost of lucene index + Node luceneGlobal = superuser.getNode("/oak:index/luceneGlobal"); + luceneGlobal.setProperty(IndexConstants.ENTRY_COUNT_PROPERTY_NAME, Long.MAX_VALUE); + // remove test mode as entry count property is ignored in test mode + luceneGlobal.getProperty("testMode").remove(); + Node n1 = testRootNode.addNode("node1"); + n1.setProperty("text", "t1"); + n1.setProperty("name","Node1"); + markIndexForReindex(); + superuser.save(); + QueryManager qm = superuser.getWorkspace().getQueryManager(); + // use issamenode() so that traversal cost comes low + String xpath = "select [rep:facet(text)] from [nt:base] where issamenode('" + n1.getPath() + "') and [text]='t1'"; + Query q = qm.createQuery(xpath, Query.JCR_SQL2); + QueryResult result = q.execute(); + assertEquals(result.getRows().getSize(), 1); + } + public void testMergedFacetsOverUnionSummingCount() throws Exception { // the distribution of nodes with t1 and t2 are intentionally across first and second set (below) // put such that second condition turns facet count around