From ec6bdff8f14b71c84faa795b967c20e303f808f5 Mon Sep 17 00:00:00 2001 From: Benjamin Habegger Date: Fri, 9 Jan 2026 08:24:24 +0100 Subject: [PATCH] OAK-12057 - Select same plan with or without LIMIT option --- .../jackrabbit/oak/query/QueryImpl.java | 8 --- .../oak/query/SQL2OptimiseQueryTest.java | 10 ++- .../jackrabbit/oak/query/SQL2ParserTest.java | 72 +++++++++++++++++-- .../jackrabbit/oak/spi/query/QueryIndex.java | 6 +- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java index 3f08b4fae4e..cf9430d655b 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java @@ -1081,8 +1081,6 @@ private SelectorExecutionPlan getBestSelectorExecutionPlan( double almostBestCost = Double.POSITIVE_INFINITY; IndexPlan almostBestPlan = null; - long maxEntryCount = saturatedAdd(offset.orElse(0L), limit.orElse(Long.MAX_VALUE)); - // Sort the indexes according to their minimum cost to be able to skip the remaining indexes if the cost of the // current index is below the minimum cost of the next index. List queryIndexes = indexProvider.getQueryIndexes(rootState).stream() @@ -1117,12 +1115,6 @@ private SelectorExecutionPlan getBestSelectorExecutionPlan( if (p.getSupportsPathRestriction()) { entryCount = scaleEntryCount(rootState, filter, entryCount); } - if (sortOrder == null || p.getSortOrder() != null) { - // if the query is unordered, or - // if the query contains "order by" and the index can sort on that, - // then we don't need to read all entries from the index - entryCount = Math.min(maxEntryCount, entryCount); - } double c = p.getCostPerExecution() + entryCount * p.getCostPerEntry(); if (LOG.isDebugEnabled()) { diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java index b42594ec934..ec2d4978d1a 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java @@ -36,8 +36,6 @@ import java.text.ParseException; import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.jcr.RepositoryException; @@ -88,7 +86,7 @@ public void limitUnionSize() throws ParseException { + " AND ((CONTAINS(*, '10') AND ([jcr:uuid] LIKE '11' OR [jcr:uuid] LIKE '12'))\n" + " OR (CONTAINS(*, '13') AND ([jcr:uuid] LIKE '14' OR [jcr:uuid] LIKE '15')))"; SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes(), qeSettings); + getMappings(), getNodeTypes()); Query original; original = parser.parse(query, false); assertNotNull(original); @@ -219,7 +217,7 @@ private static Tree addChildWithProperty(@NotNull Tree father, @NotNull String n @Test public void optimise() throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes(), qeSettings); + getMappings(), getNodeTypes()); String statement; Query original, optimised; @@ -294,7 +292,7 @@ public void optimiseAndOrAnd() throws ParseException { private void optimiseAndOrAnd(String statement, String expected) throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes(), qeSettings); + getMappings(), getNodeTypes()); Query original; original = parser.parse(statement, false); assertNotNull(original); @@ -310,7 +308,7 @@ private void optimiseAndOrAnd(String statement, String expected) throws ParseExc @Test public void optimizeKeepsQueryOptions() throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes(), qeSettings); + getMappings(), getNodeTypes()); Query original; String statement = "select * from [nt:unstructured] as [c] " + "where [a]=1 or [b]=2 option(index tag x)"; diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java index 20f96dca09d..69fb886b0e9 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java @@ -20,13 +20,22 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; import java.text.ParseException; +import java.util.List; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.query.ast.NodeTypeInfoProvider; import org.apache.jackrabbit.oak.query.stats.QueryStatsData; import org.apache.jackrabbit.oak.query.xpath.XPathToSQL2Converter; +import org.apache.jackrabbit.oak.spi.query.QueryIndex; +import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.jetbrains.annotations.NotNull; import org.junit.Ignore; import org.junit.Test; @@ -40,14 +49,12 @@ public class SQL2ParserTest { private static final SQL2Parser p = createTestSQL2Parser(); public static SQL2Parser createTestSQL2Parser() { - return createTestSQL2Parser(NamePathMapper.DEFAULT, nodeTypes, new QueryEngineSettings()); + return createTestSQL2Parser(NamePathMapper.DEFAULT, nodeTypes); } - public static SQL2Parser createTestSQL2Parser(NamePathMapper mappings, NodeTypeInfoProvider nodeTypes2, - QueryEngineSettings qeSettings) { + public static SQL2Parser createTestSQL2Parser(NamePathMapper mappings, NodeTypeInfoProvider nodeTypes2) { QueryStatsData data = new QueryStatsData("", ""); - return new SQL2Parser(mappings, nodeTypes2, new QueryEngineSettings(), - data.new QueryExecutionStats()); + return new SQL2Parser(mappings, nodeTypes2, new QueryEngineSettings(), data.new QueryExecutionStats()); } @@ -87,6 +94,60 @@ public void testUnwrappedOr() throws ParseException { assertTrue(q.contains(token)); } + /* + * When a query with LIMIT option, it should still select the index with the least entries + * as those might require being traversed during post-filtering. + * + * See OAK-12057 + */ + @Test + public void testPlanningWithLimit() throws ParseException { + // Given + var query = "SELECT * \n" + + "FROM [nt:base] AS s \n" + + "WHERE ISDESCENDANTNODE(s, '/content') AND s.[j:c]='/conf/wknd'\n" + + "OPTION (LIMIT 2)"; + + // - the first available option + var indexA = mock(QueryIndex.AdvancedQueryIndex.class); + var planA = mock(QueryIndex.IndexPlan.class); + given(indexA.getPlans(any(), any(), any())).willReturn(List.of(planA)); + given(planA.getPlanName()).willReturn("planA"); + given(planA.getEstimatedEntryCount()).willReturn(10000L); // more entries + given(planA.getCostPerEntry()).willReturn(1.0); + given(planA.getCostPerExecution()).willReturn(100.0); + + // - the better option + var indexB = mock(QueryIndex.AdvancedQueryIndex.class); + var planB = mock(QueryIndex.IndexPlan.class); + given(indexB.getPlans(any(), any(), any())).willReturn(List.of(planB)); + given(planB.getPlanName()).willReturn("planB"); + given(planB.getEstimatedEntryCount()).willReturn(100L); // less entries + given(planB.getCostPerEntry()).willReturn(1.0); + given(planB.getCostPerExecution()).willReturn(100.0); + given(indexB.getPlanDescription(eq(planB), any())).willReturn("planB"); + + var indexProvider = new QueryIndexProvider() { + @Override + public @NotNull List getQueryIndexes(NodeState nodeState) { + return List.of(indexA, indexB); + } + }; + + var context = mock(ExecutionContext.class); + given(context.getIndexProvider()).willReturn(indexProvider); + + // When + var parsedQuery = p.parse(query,false); + parsedQuery.init(); + parsedQuery.setExecutionContext(context); + parsedQuery.setTraversalEnabled(false); + parsedQuery.prepare(); + + // Then + assertEquals("[nt:base] as [s] /* planB */", parsedQuery.getPlan()); + } + @Test public void testCoalesce() throws ParseException { p.parse("SELECT * FROM [nt:base] WHERE COALESCE([j:c/m/d:t], [j:c/j:t])='a'"); @@ -188,4 +249,5 @@ public void orderingWithUnionOfNodetype() throws Exception { xpath = "//(element(*, type1) | element(*, type2))[@a='b' or @c='d'] order by @foo"; assertTrue("Converted xpath " + xpath + "doesn't end with 'order by [foo]'", c.convert(xpath).endsWith("order by [foo]")); } + } diff --git a/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java b/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java index 0dcba9f79d2..8be5f6419fa 100644 --- a/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java +++ b/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java @@ -34,7 +34,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static java.util.stream.Collectors.toMap; import static org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction; /** @@ -174,7 +173,7 @@ interface AdvanceFulltextQueryIndex extends FulltextQueryIndex, AdvancedQueryInd * (returning the rows in a specific order), and that can provide detailed * information about the cost. */ - interface AdvancedQueryIndex { + interface AdvancedQueryIndex extends QueryIndex { /** * Return the possible index plans for the given filter and sort order. @@ -373,9 +372,6 @@ default boolean logWarningForPathFilterMismatch() { * A builder for index plans. */ class Builder { - - private static Logger LOG = LoggerFactory.getLogger(QueryIndex.IndexPlan.Builder.class); - protected double costPerExecution = 1.0; protected double costPerEntry = 1.0; protected long estimatedEntryCount = 1000000;