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
commit 5d770808d6d8a782ba3f5bf953d4c9f8d3fa8a9e Author: Thomas Mueller <[email protected]> AuthorDate: Tue Feb 3 09:10:43 2026 +0100 Revert "OAK-12057 - Select same plan with or without LIMIT option (#2684)" This reverts commit 436f733f8a9208d21df29bbae8258b5774cc3aee. --- .../org/apache/jackrabbit/oak/query/QueryImpl.java | 8 +++ .../oak/query/SQL2OptimiseQueryTest.java | 10 +-- .../jackrabbit/oak/query/SQL2ParserTest.java | 72 ++-------------------- .../jackrabbit/oak/spi/query/QueryIndex.java | 6 +- .../jackrabbit/oak/spi/query/package-info.java | 2 +- 5 files changed, 25 insertions(+), 73 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 cf9430d655..3f08b4fae4 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,6 +1081,8 @@ public class QueryImpl implements Query { 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<? extends QueryIndex> queryIndexes = indexProvider.getQueryIndexes(rootState).stream() @@ -1115,6 +1117,12 @@ public class QueryImpl implements Query { 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 ec2d4978d1..b42594ec93 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,6 +36,8 @@ import static org.junit.Assert.assertTrue; import java.text.ParseException; import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.jcr.RepositoryException; @@ -86,7 +88,7 @@ public class SQL2OptimiseQueryTest extends AbstractQueryTest { + " 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()); + getMappings(), getNodeTypes(), qeSettings); Query original; original = parser.parse(query, false); assertNotNull(original); @@ -217,7 +219,7 @@ public class SQL2OptimiseQueryTest extends AbstractQueryTest { @Test public void optimise() throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes()); + getMappings(), getNodeTypes(), qeSettings); String statement; Query original, optimised; @@ -292,7 +294,7 @@ public class SQL2OptimiseQueryTest extends AbstractQueryTest { private void optimiseAndOrAnd(String statement, String expected) throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes()); + getMappings(), getNodeTypes(), qeSettings); Query original; original = parser.parse(statement, false); assertNotNull(original); @@ -308,7 +310,7 @@ public class SQL2OptimiseQueryTest extends AbstractQueryTest { @Test public void optimizeKeepsQueryOptions() throws ParseException { SQL2Parser parser = SQL2ParserTest.createTestSQL2Parser( - getMappings(), getNodeTypes()); + getMappings(), getNodeTypes(), qeSettings); 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 69fb886b0e..20f96dca09 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,22 +20,13 @@ import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT; 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; @@ -49,12 +40,14 @@ public class SQL2ParserTest { private static final SQL2Parser p = createTestSQL2Parser(); public static SQL2Parser createTestSQL2Parser() { - return createTestSQL2Parser(NamePathMapper.DEFAULT, nodeTypes); + return createTestSQL2Parser(NamePathMapper.DEFAULT, nodeTypes, new QueryEngineSettings()); } - public static SQL2Parser createTestSQL2Parser(NamePathMapper mappings, NodeTypeInfoProvider nodeTypes2) { + public static SQL2Parser createTestSQL2Parser(NamePathMapper mappings, NodeTypeInfoProvider nodeTypes2, + QueryEngineSettings qeSettings) { QueryStatsData data = new QueryStatsData("", ""); - return new SQL2Parser(mappings, nodeTypes2, new QueryEngineSettings(), data.new QueryExecutionStats()); + return new SQL2Parser(mappings, nodeTypes2, new QueryEngineSettings(), + data.new QueryExecutionStats()); } @@ -94,60 +87,6 @@ public class SQL2ParserTest { 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<? extends QueryIndex> 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'"); @@ -249,5 +188,4 @@ public class SQL2ParserTest { 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 8be5f6419f..0dcba9f79d 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,6 +34,7 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; 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; /** @@ -173,7 +174,7 @@ public interface QueryIndex { * (returning the rows in a specific order), and that can provide detailed * information about the cost. */ - interface AdvancedQueryIndex extends QueryIndex { + interface AdvancedQueryIndex { /** * Return the possible index plans for the given filter and sort order. @@ -372,6 +373,9 @@ public interface QueryIndex { * 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; diff --git a/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java b/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java index 7830bdb111..200a10dc57 100644 --- a/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java +++ b/oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java @@ -18,7 +18,7 @@ /** * This package contains oak query index related classes. */ -@Version("3.2.1") +@Version("3.2.0") package org.apache.jackrabbit.oak.spi.query; import org.osgi.annotation.versioning.Version;
