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 2014/06/23 13:15:42 UTC
svn commit: r1604750 - in /jackrabbit/oak/branches/1.0:
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/
oak-core/src/test/resources/org/apache/jackrabbit/oak/query/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/
Author: thomasm
Date: Mon Jun 23 11:15:42 2014
New Revision: 1604750
URL: http://svn.apache.org/r1604750
Log:
OAK-1894 PropertyIndex only considers the cost of a single indexed property
Modified:
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java
jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt
jackrabbit/oak/branches/1.0/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryPlanTest.java
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java?rev=1604750&r1=1604749&r2=1604750&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java Mon Jun 23 11:15:42 2014
@@ -30,6 +30,8 @@ import org.apache.jackrabbit.oak.spi.que
import org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction;
import org.apache.jackrabbit.oak.spi.query.QueryIndex;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import com.google.common.base.Charsets;
import com.google.common.collect.Iterables;
@@ -90,6 +92,8 @@ class PropertyIndex implements QueryInde
*/
private static final String EMPTY_TOKEN = ":";
+ private static final Logger LOG = LoggerFactory.getLogger(PropertyIndex.class);
+
static Set<String> encode(PropertyValue value) {
if (value == null) {
return null;
@@ -113,6 +117,37 @@ class PropertyIndex implements QueryInde
return values;
}
+ private Cheapest findCheapestProperty(Filter filter, PropertyIndexLookup lookup) {
+ Cheapest cost = new Cheapest();
+ for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
+ String propertyName = PathUtils.getName(pr.propertyName);
+ double propertyCost = Double.POSITIVE_INFINITY;
+ // TODO support indexes on a path
+ // currently, only indexes on the root node are supported
+ if (lookup.isIndexed(propertyName, "/", filter)) {
+ if (pr.firstIncluding && pr.lastIncluding
+ && pr.first != null && pr.first.equals(pr.last)) {
+ // "[property] = $value"
+ propertyCost = lookup.getCost(filter, propertyName, pr.first);
+ } else if (pr.list != null) {
+ propertyCost = 0;
+ for (PropertyValue p : pr.list) {
+ propertyCost += lookup.getCost(filter, propertyName, p);
+ }
+ } else {
+ // processed as "[property] is not null"
+ propertyCost = lookup.getCost(filter, propertyName, null);
+ }
+ }
+ LOG.debug("property cost for {} is {}", propertyName, propertyCost);
+ if (propertyCost < cost.cost) {
+ cost.cost = propertyCost;
+ cost.propertyRestriction = pr;
+ }
+ }
+ return cost;
+ }
+
//--------------------------------------------------------< QueryIndex >--
@Override
@@ -142,29 +177,9 @@ class PropertyIndex implements QueryInde
}
PropertyIndexLookup lookup = getLookup(root);
- for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
- String propertyName = PathUtils.getName(pr.propertyName);
- // TODO support indexes on a path
- // currently, only indexes on the root node are supported
- if (lookup.isIndexed(propertyName, "/", filter)) {
- if (pr.firstIncluding && pr.lastIncluding
- && pr.first != null && pr.first.equals(pr.last)) {
- // "[property] = $value"
- return lookup.getCost(filter, propertyName, pr.first);
- } else if (pr.list != null) {
- double cost = 0;
- for (PropertyValue p : pr.list) {
- cost += lookup.getCost(filter, propertyName, p);
- }
- return cost;
- } else {
- // processed as "[property] is not null"
- return lookup.getCost(filter, propertyName, null);
- }
- }
- }
- // not an appropriate index
- return Double.POSITIVE_INFINITY;
+ Cheapest cheapest = findCheapestProperty(filter, lookup);
+ LOG.debug("Cheapest property cost is {} for property {}", cheapest.cost, cheapest.propertyRestriction != null ? cheapest.propertyRestriction.propertyName : null);
+ return cheapest.cost;
}
@Override
@@ -173,7 +188,11 @@ class PropertyIndex implements QueryInde
PropertyIndexLookup lookup = getLookup(root);
int depth = 1;
- for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
+
+ Cheapest cheapest = findCheapestProperty(filter, lookup);
+ PropertyRestriction pr = cheapest.propertyRestriction;
+
+ if (pr != null) {
String propertyName = PathUtils.getName(pr.propertyName);
depth = PathUtils.getDepth(pr.propertyName);
// TODO support indexes on a path
@@ -184,7 +203,6 @@ class PropertyIndex implements QueryInde
&& pr.first != null && pr.first.equals(pr.last)) {
// "[property] = $value"
paths = lookup.query(filter, propertyName, pr.first);
- break;
} else if (pr.list != null) {
for (PropertyValue pv : pr.list) {
Iterable<String> p = lookup.query(filter, propertyName, pv);
@@ -194,11 +212,9 @@ class PropertyIndex implements QueryInde
paths = Iterables.concat(paths, p);
}
}
- break;
} else {
// processed as "[property] is not null"
paths = lookup.query(filter, propertyName, null);
- break;
}
}
}
@@ -217,7 +233,10 @@ class PropertyIndex implements QueryInde
StringBuilder buff = new StringBuilder("property");
StringBuilder notIndexed = new StringBuilder();
PropertyIndexLookup lookup = getLookup(root);
- for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
+ Cheapest cheapest = findCheapestProperty(filter, lookup);
+ PropertyRestriction pr = cheapest.propertyRestriction;
+
+ if (pr != null) {
String propertyName = PathUtils.getName(pr.propertyName);
// TODO support indexes on a path
// currently, only indexes on the root node are supported
@@ -251,4 +270,9 @@ class PropertyIndex implements QueryInde
return buff.toString();
}
+ private static class Cheapest {
+ private double cost = Double.POSITIVE_INFINITY;
+ private PropertyRestriction propertyRestriction;
+ }
+
}
\ No newline at end of file
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt?rev=1604750&r1=1604749&r2=1604750&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt Mon Jun 23 11:15:42 2014
@@ -206,7 +206,7 @@ explain select *
explain select [rep:excerpt]
from [nt:base]
where [jcr:uuid] is not null
-[nt:base] as [nt:base] /* property jcr:uuid (rep:excerpt)
+[nt:base] as [nt:base] /* property jcr:uuid
where [nt:base].[jcr:uuid] is not null */
commit / + "test": { "jcr:uuid": "xyz", "a": { "jcr:uuid": "123" } }
Modified: jackrabbit/oak/branches/1.0/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryPlanTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryPlanTest.java?rev=1604750&r1=1604749&r2=1604750&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryPlanTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryPlanTest.java Mon Jun 23 11:15:42 2014
@@ -23,6 +23,8 @@ import static org.junit.Assert.assertFal
import static org.junit.Assert.assertTrue;
import javax.jcr.Node;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.query.Query;
import javax.jcr.query.QueryManager;
@@ -42,7 +44,49 @@ public class QueryPlanTest extends Abstr
public QueryPlanTest(NodeStoreFixture fixture) {
super(fixture);
}
-
+
+ @Test
+ // OAK-1898
+ public void correctPropertyIndexUsage() throws Exception {
+ Session session = getAdminSession();
+ QueryManager qm = session.getWorkspace().getQueryManager();
+ Node testRootNode = session.getRootNode().addNode("testroot");
+ createPropertyIndex(session, "fiftyPercent");
+ createPropertyIndex(session, "tenPercent");
+ createPropertyIndex(session, "hundredPercent");
+ for (int i = 0; i < 300; i++) {
+ Node n = testRootNode.addNode("n" + i, "oak:Unstructured");
+ if (i % 10 == 0) {
+ n.setProperty("tenPercent", i);
+ }
+ if (i % 2 == 0) {
+ n.setProperty("fiftyPercent", i);
+ }
+ n.setProperty("hundredPercent", i);
+ }
+ session.save();
+
+ String xpath = "/jcr:root//*[@tenPercent and @fiftyPercent and @hundredPercent]";
+
+ Query q;
+ QueryResult result;
+ RowIterator it;
+
+ q = qm.createQuery("explain " + xpath, "xpath");
+ result = q.execute();
+ it = result.getRows();
+ assertTrue(it.hasNext());
+ String plan = it.nextRow().getValue("plan").getString();
+ // System.out.println("plan: " + plan);
+ // should not use the index on "jcr:uuid"
+ assertEquals("[nt:base] as [a] /* property tenPercent " +
+ "where ((([a].[tenPercent] is not null) " +
+ "and ([a].[fiftyPercent] is not null)) " +
+ "and ([a].[hundredPercent] is not null)) " +
+ "and (isdescendantnode([a], [/])) */",
+ plan);
+ }
+
@Test
// OAK-1898
public void traversalVersusPropertyIndex() throws Exception {
@@ -68,13 +112,12 @@ public class QueryPlanTest extends Abstr
it = result.getRows();
assertTrue(it.hasNext());
String plan = it.nextRow().getValue("plan").getString();
- System.out.println("plan: " + plan);
+ // System.out.println("plan: " + plan);
// should not use the index on "jcr:uuid"
assertEquals("[nt:base] as [a] /* property jcr:uuid " +
"where ([a].[jcr:uuid] is not null) and " +
"(isdescendantnode([a], [/testroot/n/n/n/n/n/n/n])) */",
plan);
-
}
@Test
@@ -250,4 +293,13 @@ public class QueryPlanTest extends Abstr
assertEquals("/testroot/b/c/d/e2", path);
assertFalse(it.hasNext());
}
+
+ private static void createPropertyIndex(Session s, String propertyName) throws RepositoryException {
+ Node n = s.getRootNode().getNode("oak:index").
+ addNode(propertyName, "oak:QueryIndexDefinition");
+ n.setProperty("type", "property");
+ n.setProperty("propertyNames", new String[]{propertyName}, PropertyType.NAME);
+ s.save();
+ }
+
}