This is an automated email from the ASF dual-hosted git repository.
thomasm pushed a commit to branch OAK-12057
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/OAK-12057 by this push:
new 3f7e4cdc80 OAK-12057 - Select same plan with or without LIMIT option
(#2722)
3f7e4cdc80 is described below
commit 3f7e4cdc801336d188f9b8d3ad917b1301747a8a
Author: Benjamin Habegger <[email protected]>
AuthorDate: Wed Feb 4 13:37:04 2026 +0100
OAK-12057 - Select same plan with or without LIMIT option (#2722)
* Simplify cost evaluation test
* OAK-12057 - Select same plan with or without LIMIT option
---
.../org/apache/jackrabbit/oak/query/QueryImpl.java | 8 --
.../oak/query/SQL2OptimiseQueryTest.java | 10 +-
.../jackrabbit/oak/query/SQL2ParserTest.java | 81 ++++++++++++++-
.../jackrabbit/oak/IndexCostEvaluationTest.java | 110 +++++++++------------
.../jackrabbit/oak/spi/query/QueryIndex.java | 4 -
5 files changed, 127 insertions(+), 86 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 3f08b4fae4..cf9430d655 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 @@ 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()
@@ -1117,12 +1115,6 @@ 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 b42594ec93..ec2d4978d1 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 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;
@@ -88,7 +86,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(), qeSettings);
+ getMappings(), getNodeTypes());
Query original;
original = parser.parse(query, false);
assertNotNull(original);
@@ -219,7 +217,7 @@ public class SQL2OptimiseQueryTest extends
AbstractQueryTest {
@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 class SQL2OptimiseQueryTest extends
AbstractQueryTest {
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 @@ public class SQL2OptimiseQueryTest extends
AbstractQueryTest {
@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 20f96dca09..2709a2cd0b 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.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;
@@ -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 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(DummyQueryIndex.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(DummyQueryIndex.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'");
@@ -188,4 +249,14 @@ 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]"));
}
+
+ interface DummyQueryIndex extends QueryIndex,
QueryIndex.AdvancedQueryIndex {
+ /*
+ * This is needed as AdvancedQueryIndex is what we what to mock but
the QueryIndexProvider
+ * returns QueryIndex instances and, in reality implementation of
AdvancedQueryIndex also implement
+ * QueryIndex, but AdvancedQueryIndex does not explicitly extend
QueryIndex.
+ *
+ * Making this link explicit would break the spi versioning.
+ */
+ }
}
diff --git
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/IndexCostEvaluationTest.java
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/IndexCostEvaluationTest.java
index f09e2eb21b..7f096cfc2a 100644
---
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/IndexCostEvaluationTest.java
+++
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/IndexCostEvaluationTest.java
@@ -17,8 +17,8 @@
package org.apache.jackrabbit.oak;
import ch.qos.logback.classic.Level;
-import org.apache.jackrabbit.api.JackrabbitRepository;
-import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.api.ContentSession;
import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
import org.apache.jackrabbit.oak.jcr.Jcr;
import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexPlan;
@@ -28,6 +28,7 @@ import org.apache.jackrabbit.oak.spi.query.Cursor;
import org.apache.jackrabbit.oak.spi.query.Filter;
import org.apache.jackrabbit.oak.spi.query.QueryIndex;
import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
@@ -36,66 +37,61 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
-import javax.jcr.Node;
-import javax.jcr.Repository;
-import javax.jcr.SimpleCredentials;
import java.io.File;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import static javax.jcr.query.Query.JCR_SQL2;
import static org.junit.Assert.assertTrue;
public class IndexCostEvaluationTest {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder(new
File("target"));
- private static final String TEST_USER_NAME = "testUserName";
-
- private Repository repository = null;
- private JackrabbitSession session = null;
- private Node root = null;
- private LogCustomizer custom;
+ private ContentSession session = null;
+ private LogCustomizer logCollector;
@Before
public void before() throws Exception {
- custom = LogCustomizer
+ logCollector = LogCustomizer
.forLogger(
"org.apache.jackrabbit.oak.query.QueryImpl")
.enable(Level.DEBUG).create();
- custom.starting();
-
-
- TestIndexProvider testProvider = new TestIndexProvider();
- TestIndexProvider2 testProvider2 = new TestIndexProvider2();
- TestIndexProvider3 testProvider3 = new TestIndexProvider3();
-
- Jcr jcr = new Jcr()
- .with((QueryIndexProvider) testProvider)
- .with((QueryIndexProvider) testProvider2)
- .with((QueryIndexProvider) testProvider3);
-
- repository = jcr.createRepository();
- session = (JackrabbitSession) repository.login(new
SimpleCredentials("admin", "admin".toCharArray()));
- root = session.getRootNode();
+ logCollector.starting();
+ double luceneMinCost = 2.2;
+ double elasticMinCost = 2.1;
+ TestIndexProvider testProvider = new TestIndexProvider("test-index",
luceneMinCost);
+ TestIndexProvider testProvider2 = new TestIndexProvider("test-index2",
luceneMinCost);
+ TestIndexProvider testProvider3 = new TestIndexProvider("test-index3",
elasticMinCost);
+
+ Jcr jcr = new Jcr(new Oak(), false)
+ .with(new OpenSecurityProvider())
+ .with(new InitialContent())
+ .with(testProvider)
+ .with(testProvider2)
+ .with(testProvider3);
+
+ ContentRepository repository = jcr.createContentRepository();
+ session = repository.login(null, null);
}
-
@After
- public void after() {
- custom.finished();
- session.logout();
- if (repository instanceof JackrabbitRepository) {
- ((JackrabbitRepository) repository).shutdown();
- }
+ public void after() throws IOException {
+ session.close();
+ logCollector.finished();
}
// In cases where two indexes have same min cost i.e. both indexes are on
par, we don't skip cost evaluation
// even of cost from previous index is less than min cost of new index.
@Test
public void costEvaluationTest() throws Exception {
+ String query = "SELECT * FROM [rep:Authorizable] WHERE
[rep:principalName] = 'anonymous'";
+ session.getLatestRoot().getQueryEngine().executeQuery(query, JCR_SQL2,
1, 0, null, null);
+
boolean evaluationContinueLogPresent = false;
boolean evaluationSkipLogPresent = false;
- for (String log : custom.getLogs()) {
+ for (String log : logCollector.getLogs()) {
if (log.equals("minCost: 2.1 of index :test-index2 > best Cost:
2.0 from index: test-index, but both indexes have same minimum cost - cost
evaluation will continue")) {
evaluationContinueLogPresent = true;
}
@@ -107,8 +103,12 @@ public class IndexCostEvaluationTest {
assertTrue(evaluationSkipLogPresent);
}
- private class TestIndexProvider implements QueryIndexProvider {
- TestIndex index = new TestIndex();
+ private static class TestIndexProvider implements QueryIndexProvider {
+ private final TestIndex index;
+
+ public TestIndexProvider(String indexName, double minimumCost) {
+ this.index = new TestIndex(indexName, minimumCost);
+ }
@Override
public @NotNull List<? extends QueryIndex> getQueryIndexes(NodeState
nodeState) {
@@ -116,35 +116,19 @@ public class IndexCostEvaluationTest {
}
}
- private class TestIndexProvider2 extends TestIndexProvider {
- public TestIndexProvider2() {
- this.index = new TestIndex() {
- public String getIndexName() {
- return "test-index2";
- }
- };
- }
- }
+ private static class TestIndex implements QueryIndex,
QueryIndex.AdvancedQueryIndex {
- private class TestIndexProvider3 extends TestIndexProvider {
- public TestIndexProvider3() {
- this.index = new TestIndex() {
- public String getIndexName() {
- return "test-index3";
- }
-
- public double getMinimumCost() {
- return PropertyIndexPlan.COST_OVERHEAD + 0.11;
- }
- };
- }
- }
+ private final String name;
+ private final double minimumCost;
- private class TestIndex implements QueryIndex,
QueryIndex.AdvancedQueryIndex {
+ public TestIndex(String indexName, double minimumCost) {
+ this.name = indexName;
+ this.minimumCost = minimumCost;
+ }
@Override
public double getMinimumCost() {
- return PropertyIndexPlan.COST_OVERHEAD + 0.1;
+ return minimumCost;
}
@Override
@@ -164,7 +148,7 @@ public class IndexCostEvaluationTest {
@Override
public String getIndexName() {
- return "test-index";
+ return name;
}
@Override
@@ -172,7 +156,7 @@ public class IndexCostEvaluationTest {
IndexPlan.Builder b = new IndexPlan.Builder();
Filter f = new FilterImpl(null, "SELECT * FROM [nt:file]", new
QueryEngineSettings());
IndexPlan plan1 =
b.setEstimatedEntryCount(10).setPlanName("testIndexPlan1").setFilter(f).build();
- List<IndexPlan> indexList = new ArrayList<IndexPlan>();
+ List<IndexPlan> indexList = new ArrayList<>();
indexList.add(plan1);
return indexList;
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 0dcba9f79d..6bab6bb03e 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.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;
/**
@@ -373,9 +372,6 @@ 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;
