You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by th...@apache.org on 2017/02/10 14:02:06 UTC
svn commit: r1782471 - in /jackrabbit/oak/branches/1.6: ./
oak-core/src/main/java/org/apache/jackrabbit/oak/query/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/
Author: thomasm
Date: Fri Feb 10 14:02:06 2017
New Revision: 1782471
URL: http://svn.apache.org/viewvc?rev=1782471&view=rev
Log:
OAK-5621 Warn traversal queries: false positives for joins and SQL-2 queries using or
Modified:
jackrabbit/oak/branches/1.6/ (props changed)
jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java
jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java
jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java
jackrabbit/oak/branches/1.6/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java
Propchange: jackrabbit/oak/branches/1.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Feb 10 14:02:06 2017
@@ -1,3 +1,3 @@
/jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1781068,1781075,1781386,1781846,1781907,1782000,1782196
+/jackrabbit/oak/trunk:1781068,1781075,1781386,1781846,1781907,1782000,1782196,1782447
/jackrabbit/trunk:1345480
Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java?rev=1782471&r1=1782470&r2=1782471&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java Fri Feb 10 14:02:06 2017
@@ -190,4 +190,21 @@ public interface Query {
*/
void setQueryOptions(QueryOptions options);
+ /**
+ * Whether the query is potentially slow.
+ * Only supported for prepared queries.
+ *
+ * @return true if traversal is the only option
+ */
+ boolean isPotentiallySlow();
+
+ /**
+ * Verify the query is not potentially slow. Only supported for prepared
+ * queries.
+ *
+ * @throws IllegalArgumentException if potentially slow, and configured to
+ * fail in this case
+ */
+ void verifyNotPotentiallySlow();
+
}
Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java?rev=1782471&r1=1782470&r2=1782471&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java Fri Feb 10 14:02:06 2017
@@ -292,6 +292,7 @@ public abstract class QueryEngineImpl im
// we only have the original query so we prepare and return it.
result = queries.iterator().next();
result.prepare();
+ result.verifyNotPotentiallySlow();
LOG.debug("No alternatives found. Query: {}", result);
} else {
double bestCost = Double.POSITIVE_INFINITY;
@@ -300,8 +301,12 @@ public abstract class QueryEngineImpl im
// it's the default behaviour. That way, we always log the cost and
// can more easily analyze problems. The querySelectionMode flag can
// be used to override the cheapest.
+ boolean isPotentiallySlow = true;
for (Query q : checkNotNull(queries)) {
q.prepare();
+ if (!q.isPotentiallySlow()) {
+ isPotentiallySlow = false;
+ }
double cost = q.getEstimatedCost();
LOG.debug("cost: {} for query {}", cost, q);
if (q.containsUnfilteredFullTextCondition()) {
@@ -329,6 +334,9 @@ public abstract class QueryEngineImpl im
case CHEAPEST:
default:
}
+ if (isPotentiallySlow) {
+ result.verifyNotPotentiallySlow();
+ }
}
return result;
Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java?rev=1782471&r1=1782470&r2=1782471&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java Fri Feb 10 14:02:06 2017
@@ -203,6 +203,8 @@ public class QueryImpl implements Query
private boolean isInternal;
+ private boolean potentiallySlowTraversalQuery;
+
QueryImpl(String statement, SourceImpl source, ConstraintImpl constraint,
ColumnImpl[] columns, NamePathMapper mapper, QueryEngineSettings settings) {
this.statement = statement;
@@ -644,6 +646,11 @@ public class QueryImpl implements Query
// use a greedy algorithm
SourceImpl result = null;
Set<SourceImpl> available = new HashSet<SourceImpl>();
+ // the query is only slow if all possible join orders are slow
+ // (in theory, due to using the greedy algorithm, a query might be considered
+ // slow even thought there is a plan that doesn't need to use traversal, but
+ // only for 3-way and higher joins, and only if traversal is considered very fast)
+ boolean isPotentiallySlowJoin = true;
while (sources.size() > 0) {
int bestIndex = 0;
double bestCost = Double.POSITIVE_INFINITY;
@@ -663,12 +670,16 @@ public class QueryImpl implements Query
bestIndex = i;
best = test;
}
+ if (!potentiallySlowTraversalQuery) {
+ isPotentiallySlowJoin = false;
+ }
test.unprepare();
}
available.add(sources.remove(bestIndex));
result = best;
best.prepare(bestPlan);
}
+ potentiallySlowTraversalQuery = isPotentiallySlowJoin;
estimatedCost = result.prepare().getEstimatedCost();
source = result;
isSortedByIndex = canSortByIndex();
@@ -1035,7 +1046,7 @@ public class QueryImpl implements Query
bestPlan = indexPlan;
}
}
- boolean potentiallySlowTraversalQuery = bestIndex == null;
+ potentiallySlowTraversalQuery = bestIndex == null;
if (traversalEnabled) {
TraversingIndex traversal = new TraversingIndex();
double cost = traversal.getCost(filter, rootState);
@@ -1051,6 +1062,17 @@ public class QueryImpl implements Query
}
}
}
+ return new SelectorExecutionPlan(filter.getSelector(), bestIndex,
+ bestPlan, bestCost);
+ }
+
+ @Override
+ public boolean isPotentiallySlow() {
+ return potentiallySlowTraversalQuery;
+ }
+
+ @Override
+ public void verifyNotPotentiallySlow() {
if (potentiallySlowTraversalQuery) {
QueryOptions.Traversal traversal = queryOptions.traversal;
if (traversal == Traversal.DEFAULT) {
@@ -1078,7 +1100,6 @@ public class QueryImpl implements Query
throw new IllegalArgumentException(message);
}
}
- return new SelectorExecutionPlan(filter.getSelector(), bestIndex, bestPlan, bestCost);
}
private List<OrderEntry> getSortOrder(FilterImpl filter) {
Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java?rev=1782471&r1=1782470&r2=1782471&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java Fri Feb 10 14:02:06 2017
@@ -396,4 +396,16 @@ public class UnionQueryImpl implements Q
right.containsUnfilteredFullTextCondition();
}
+ @Override
+ public boolean isPotentiallySlow() {
+ return left.isPotentiallySlow() ||
+ right.isPotentiallySlow();
+ }
+
+ @Override
+ public void verifyNotPotentiallySlow() {
+ left.verifyNotPotentiallySlow();
+ right.verifyNotPotentiallySlow();
+ }
+
}
Modified: jackrabbit/oak/branches/1.6/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java?rev=1782471&r1=1782470&r2=1782471&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java (original)
+++ jackrabbit/oak/branches/1.6/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java Fri Feb 10 14:02:06 2017
@@ -72,36 +72,69 @@ public class QueryTest extends AbstractR
public void traversalOption() throws Exception {
Session session = getAdminSession();
QueryManager qm = session.getWorkspace().getQueryManager();
- try {
- qm.createQuery("//*[@test] option(traversal fail)",
- "xpath").execute();
- fail();
- } catch (InvalidQueryException e) {
- // expected
- }
- try {
- qm.createQuery("select * from [nt:base] option(traversal fail)",
- Query.JCR_SQL2).execute();
- fail();
- } catch (InvalidQueryException e) {
- // expected
- }
- qm.createQuery("//*[@test] option(traversal ok)",
- "xpath").execute();
- qm.createQuery("//*[@test] option(traversal warn)",
- "xpath").execute();
- qm.createQuery("select * from [nt:base] option(traversal ok)",
- Query.JCR_SQL2).execute();
- qm.createQuery("select * from [nt:base] option(traversal warn)",
- Query.JCR_SQL2).execute();
+
+ // for union queries:
+ // both subqueries use an index
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] where ischildnode('/') or [jcr:uuid] = 1 option(traversal fail)"));
+ // no subquery uses an index
+ assertFalse(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] where [x] = 1 or [y] = 2 option(traversal fail)"));
+ // first one does not, second one does
+ assertFalse(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] where [jcr:uuid] = 1 or [x] = 2 option(traversal fail)"));
+ // first one does, second one does not
+ assertFalse(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] where [x] = 2 or [jcr:uuid] = 1 option(traversal fail)"));
+
+ // queries that possibly use traversal (depending on the join order)
+ assertTrue(isValidQuery(qm, "xpath",
+ "/jcr:root/content//*/jcr:content[@jcr:uuid='1'] option(traversal fail)"));
+ assertTrue(isValidQuery(qm, "xpath",
+ "/jcr:root/content/*/jcr:content[@jcr:uuid='1'] option(traversal fail)"));
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] as [a] inner join [nt:base] as [b] on ischildnode(b, a) " +
+ "where [a].[jcr:uuid] = 1 option(traversal fail)"));
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] as [a] inner join [nt:base] as [b] on ischildnode(a, b) " +
+ "where [a].[jcr:uuid] = 1 option(traversal fail)"));
+
+ // union with joins
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] as [a] inner join [nt:base] as [b] on ischildnode(a, b) " +
+ "where ischildnode([a], '/') or [a].[jcr:uuid] = 1 option(traversal fail)"));
+
+ assertFalse(isValidQuery(qm, "xpath",
+ "//*[@test] option(traversal fail)"));
+ assertFalse(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] option(traversal fail)"));
+ assertTrue(isValidQuery(qm, "xpath",
+ "//*[@test] option(traversal ok)"));
+ assertTrue(isValidQuery(qm, "xpath",
+ "//*[@test] option(traversal warn)"));
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] option(traversal ok)"));
+ assertTrue(isValidQuery(qm, Query.JCR_SQL2,
+ "select * from [nt:base] option(traversal warn)"));
+
// the following is not really traversal, it is just listing child nodes:
- qm.createQuery("/jcr:root/*[@test] option(traversal fail)",
- "xpath").execute();
+ assertTrue(isValidQuery(qm, "xpath",
+ "/jcr:root/*[@test] option(traversal fail)"));
// the following is not really traversal; it is just one node:
- qm.createQuery("/jcr:root/oak:index[@test] option(traversal fail)",
- "xpath").execute();
+ assertTrue(isValidQuery(qm, "xpath",
+ "/jcr:root/oak:index[@test] option(traversal fail)"));
- }
+ }
+
+ private static boolean isValidQuery(QueryManager qm, String language, String query) throws RepositoryException {
+ try {
+ qm.createQuery(query, language).execute();
+ return true;
+ } catch (InvalidQueryException e) {
+ assertTrue(e.toString(), e.toString().indexOf("query without index") >= 0);
+ return false;
+ }
+ }
@Test
public void firstSelector() throws Exception {