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 ju...@apache.org on 2014/07/17 16:18:11 UTC
svn commit: r1611359 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/index/property/
main/java/org/apache/jackrabbit/oak/query/ast/
test/java/org/apache/jackrabbit/oak/plugins/index/property/
test/java/org/apache/ja...
Author: jukka
Date: Thu Jul 17 14:18:11 2014
New Revision: 1611359
URL: http://svn.apache.org/r1611359
Log:
OAK-1965: Support for constraints like: foo = 'X' OR bar = 'Y'
Push down per-selector multi-property OR constraints and use then in PropertyIndexPlan
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/BindVariableValueImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/LiteralImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/StaticOperandImpl.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java
jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java Thu Jul 17 14:18:11 2014
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugin
import static com.google.common.base.Predicates.in;
import static com.google.common.collect.Iterables.any;
import static com.google.common.collect.Iterables.isEmpty;
+import static com.google.common.collect.Sets.newHashSet;
import static com.google.common.collect.Sets.newLinkedHashSet;
import static java.util.Collections.emptySet;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.DECLARING_NODE_TYPES;
@@ -34,6 +35,14 @@ import org.apache.jackrabbit.oak.plugins
import org.apache.jackrabbit.oak.plugins.index.property.strategy.IndexStoreStrategy;
import org.apache.jackrabbit.oak.plugins.index.property.strategy.UniqueEntryStoreStrategy;
import org.apache.jackrabbit.oak.query.QueryEngineSettings;
+import org.apache.jackrabbit.oak.query.ast.ComparisonImpl;
+import org.apache.jackrabbit.oak.query.ast.ConstraintImpl;
+import org.apache.jackrabbit.oak.query.ast.DynamicOperandImpl;
+import org.apache.jackrabbit.oak.query.ast.InImpl;
+import org.apache.jackrabbit.oak.query.ast.Operator;
+import org.apache.jackrabbit.oak.query.ast.OrImpl;
+import org.apache.jackrabbit.oak.query.ast.PropertyValueImpl;
+import org.apache.jackrabbit.oak.query.ast.StaticOperandImpl;
import org.apache.jackrabbit.oak.spi.query.Cursor;
import org.apache.jackrabbit.oak.spi.query.Cursors;
import org.apache.jackrabbit.oak.spi.query.Filter;
@@ -67,6 +76,8 @@ public class PropertyIndexPlan {
private final String name;
+ private final Set<String> properties;
+
private final IndexStoreStrategy strategy;
private final Filter filter;
@@ -84,6 +95,7 @@ public class PropertyIndexPlan {
PropertyIndexPlan(String name, NodeState definition, Filter filter) {
this.name = name;
this.definition = definition;
+ this.properties = newHashSet(definition.getNames(PROPERTY_NAMES));
if (definition.getBoolean(UNIQUE_PROPERTY_NAME)) {
this.strategy = UNIQUE;
@@ -103,13 +115,14 @@ public class PropertyIndexPlan {
int bestDepth = 1;
if (matchesNodeTypes) {
- for (String property : definition.getNames(PROPERTY_NAMES)) {
- int depth = 1;
-
+ for (String property : properties) {
PropertyRestriction restriction =
filter.getPropertyRestriction(property);
+ int depth = 1;
+
if (restriction == null) {
// no direct restriction, try one with a relative path
+ // TODO: avoid repeated scans through the restrictions
String suffix = "/" + property;
for (PropertyRestriction relative
: filter.getPropertyRestrictions()) {
@@ -119,16 +132,33 @@ public class PropertyIndexPlan {
}
}
}
- if (restriction == null) {
- continue; // no matching restrictions
+
+ if (restriction != null) {
+ Set<String> values = getValues(restriction);
+ double cost = strategy.count(definition, values, MAX_COST);
+ if (cost < bestCost) {
+ bestDepth = depth;
+ bestValues = values;
+ bestCost = cost;
+ }
}
+ }
- Set<String> values = getValues(restriction);
- double cost = strategy.count(definition, values, MAX_COST);
- if (cost < bestCost) {
- bestDepth = depth;
- bestValues = values;
- bestCost = cost;
+ // OAK-1965: let's see if we can find a (x='...' OR y='...')
+ // constraint where both x and y are covered by this index
+ // TODO: avoid repeated scans through the constraints
+ for (ConstraintImpl constraint
+ : filter.getSelector().getSelectorConstraints()) {
+ if (constraint instanceof OrImpl) {
+ Set<String> values = findMultiProperty((OrImpl) constraint);
+ if (values != null) {
+ double cost = strategy.count(definition, values, MAX_COST);
+ if (cost < bestCost) {
+ bestDepth = 1;
+ bestValues = values;
+ bestCost = cost;
+ }
+ }
}
}
}
@@ -138,6 +168,46 @@ public class PropertyIndexPlan {
this.cost = COST_OVERHEAD + bestCost;
}
+ private Set<String> findMultiProperty(OrImpl or) {
+ Set<String> values = newLinkedHashSet();
+ for (ConstraintImpl constraint : or.getConstraints()) {
+ if (constraint instanceof ComparisonImpl) {
+ ComparisonImpl comparison = (ComparisonImpl) constraint;
+ if (isIndexed(comparison.getOperand1())
+ && comparison.getOperator() == Operator.EQUAL) {
+ values.addAll(encode(comparison.getOperand2().currentValue()));
+ } else {
+ return null;
+ }
+ } else if (constraint instanceof InImpl) {
+ InImpl in = (InImpl) constraint;
+ if (isIndexed(in.getOperand1())) {
+ for (StaticOperandImpl operand : in.getOperand2()) {
+ values.addAll(encode(operand.currentValue()));
+ }
+ } else {
+ return null;
+ }
+ } else {
+ return null;
+ }
+ }
+ return values;
+ }
+
+ /**
+ * Checks whether the given dynamic operand is a property
+ * covered by this index.
+ */
+ private boolean isIndexed(DynamicOperandImpl operand) {
+ if (operand instanceof PropertyValueImpl) {
+ PropertyValueImpl property = (PropertyValueImpl) operand;
+ return properties.contains(property.getPropertyName());
+ } else {
+ return false;
+ }
+ }
+
private Set<String> getValues(PropertyRestriction restriction) {
if (restriction.firstIncluding
&& restriction.lastIncluding
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/BindVariableValueImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/BindVariableValueImpl.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/BindVariableValueImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/BindVariableValueImpl.java Thu Jul 17 14:18:11 2014
@@ -41,7 +41,7 @@ public class BindVariableValueImpl exten
}
@Override
- PropertyValue currentValue() {
+ public PropertyValue currentValue() {
return query.getBindVariableValue(bindVariableName);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/LiteralImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/LiteralImpl.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/LiteralImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/LiteralImpl.java Thu Jul 17 14:18:11 2014
@@ -51,7 +51,7 @@ public class LiteralImpl extends StaticO
}
@Override
- PropertyValue currentValue() {
+ public PropertyValue currentValue() {
// TODO namespace remapping?
return value;
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java Thu Jul 17 14:18:11 2014
@@ -225,6 +225,7 @@ public class OrImpl extends ConstraintIm
DynamicOperandImpl operand = null;
LinkedHashSet<StaticOperandImpl> values = newLinkedHashSet();
+ boolean multiPropertyOr = false;
List<AndImpl> ands = newArrayList();
for (ConstraintImpl constraint : constraints) {
Set<SelectorImpl> selectors = constraint.getSelectors();
@@ -239,7 +240,7 @@ public class OrImpl extends ConstraintIm
operand = o;
values.addAll(in.getOperand2());
} else {
- return;
+ multiPropertyOr = true;
}
} else if (constraint instanceof ComparisonImpl
&& ((ComparisonImpl) constraint).getOperator() == EQUAL) {
@@ -249,14 +250,17 @@ public class OrImpl extends ConstraintIm
operand = o;
values.add(comparison.getOperand2());
} else {
- return;
+ multiPropertyOr = true;
}
} else {
return;
}
}
- if (operand == null) {
+ if (multiPropertyOr && ands.isEmpty()) {
+ s.restrictSelector(this);
+ return;
+ } else if (operand == null) {
return;
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/StaticOperandImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/StaticOperandImpl.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/StaticOperandImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/StaticOperandImpl.java Thu Jul 17 14:18:11 2014
@@ -25,7 +25,7 @@ import org.apache.jackrabbit.oak.api.Pro
*/
public abstract class StaticOperandImpl extends AstElement {
- abstract PropertyValue currentValue();
+ public abstract PropertyValue currentValue();
abstract int getPropertyType();
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java?rev=1611359&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java Thu Jul 17 14:18:11 2014
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.index.property;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.List;
+
+import javax.jcr.query.Query;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.index.IndexUtils;
+import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent;
+import org.apache.jackrabbit.oak.query.AbstractQueryTest;
+import org.apache.jackrabbit.oak.spi.lifecycle.RepositoryInitializer;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.Test;
+
+/**
+ * <code>RelativePathTest</code>...
+ */
+public class MultiPropertyOrTest extends AbstractQueryTest {
+
+ @Override
+ protected ContentRepository createRepository() {
+ return new Oak().with(new InitialContent())
+ .with(new RepositoryInitializer() {
+ @Override
+ public void initialize(NodeBuilder builder) {
+ NodeBuilder index = IndexUtils.getOrCreateOakIndex(builder);
+ IndexUtils.createIndexDefinition(
+ index, "xyz", true, false,
+ ImmutableList.<String>of("x", "y", "z"), null);
+ }
+ })
+ .with(new OpenSecurityProvider())
+ .with(new PropertyIndexProvider())
+ .with(new PropertyIndexEditorProvider())
+ .createContentRepository();
+ }
+
+ @Test
+ public void query() throws Exception {
+ Tree t = root.getTree("/");
+ t.addChild("a").setProperty("x", "foo");
+ t.addChild("b").setProperty("y", "bar");
+ t.addChild("c").setProperty("z", "foo");
+ root.commit();
+ setTravesalEnabled(false);
+ assertQuery("select [jcr:path] from [nt:base] where [x] is not null",
+ ImmutableList.of("/a"));
+
+ List<String> lines = executeQuery(
+ "explain select [jcr:path] from [nt:base] where [x] is not null",
+ Query.JCR_SQL2);
+ assertEquals(1, lines.size());
+ // make sure it used the property index
+ assertTrue(lines.get(0).contains("property xyz IS NOT NULL"));
+
+ lines = executeQuery(
+ "explain select [jcr:path] from [nt:base] where [x] = 'foo' OR [y] = 'foo'",
+ Query.JCR_SQL2);
+ assertEquals(1, lines.size());
+ // make sure it used the property index
+ assertTrue(lines.get(0).contains("property xyz = foo"));
+
+ lines = executeQuery(
+ "explain select [jcr:path] from [nt:base] where [x] = 'foo' OR [y] = 'bar'",
+ Query.JCR_SQL2);
+ assertEquals(1, lines.size());
+ // make sure it used the property index
+ assertTrue(lines.get(0), lines.get(0).contains("property xyz IN (foo, bar)"));
+
+ assertQuery(
+ "select [jcr:path] from [nt:base] where [x] = 'foo' OR [y] = 'foo'",
+ ImmutableList.of("/a"));
+ assertQuery(
+ "select [jcr:path] from [nt:base] where [x] = 'foo' OR [z] = 'foo'",
+ ImmutableList.of("/a", "/c"));
+ assertQuery(
+ "select [jcr:path] from [nt:base] where [x] = 'foo' OR [y] = 'bar'",
+ ImmutableList.of("/a", "/b"));
+ setTravesalEnabled(false);
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java Thu Jul 17 14:18:11 2014
@@ -264,11 +264,11 @@ public abstract class AbstractQueryTest
protected List<String> assertQuery(String sql, String language,
List<String> expected) {
List<String> paths = executeQuery(sql, language, true);
- assertEquals("Result set size is different", expected.size(),
- paths.size());
for (String p : expected) {
- assertTrue(paths.contains(p));
+ assertTrue("Expected path " + p + " not found", paths.contains(p));
}
+ assertEquals("Result set size is different", expected.size(),
+ paths.size());
return paths;
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt?rev=1611359&r1=1611358&r2=1611359&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2_index.txt Thu Jul 17 14:18:11 2014
@@ -109,7 +109,8 @@ explain select excerpt(.)
explain select *
from [nt:base]
where [jcr:uuid]=1 or [b]=2
-[nt:base] as [nt:base] /* traverse "*" */
+[nt:base] as [nt:base] /* traverse "*"
+ where ([nt:base].[jcr:uuid] = 1) or ([nt:base].[b] = 2) */
explain select b.[jcr:uuid]
from [nt:base] as a
@@ -135,7 +136,8 @@ explain select *
from [nt:base]
where length([jcr:uuid])=1 or upper([jcr:uuid])='1' or lower([jcr:uuid])='3'
[nt:base] as [nt:base] /* property uuid IS NOT NULL
- where [nt:base].[jcr:uuid] is not null */
+ where ([nt:base].[jcr:uuid] is not null)
+ and ((length([nt:base].[jcr:uuid]) = 1) or (upper([nt:base].[jcr:uuid]) = '1') or (lower([nt:base].[jcr:uuid]) = '3')) */
explain select *
from [nt:base]