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 2013/11/28 11:58:41 UTC
svn commit: r1546341 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/query/ast/
main/java/org/apache/jackrabbit/oak/query/index/
main/java/org/apache/jackrabbit/oak/spi/query/
test/java/org/apache/jackrabbit/oak/query/index/
Author: thomasm
Date: Thu Nov 28 10:58:40 2013
New Revision: 1546341
URL: http://svn.apache.org/r1546341
Log:
OAK-1075 XPath query failures for mvps
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/FilterImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/PropertyValues.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/FilterTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java?rev=1546341&r1=1546340&r2=1546341&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java Thu Nov 28 10:58:40 2013
@@ -118,7 +118,7 @@ public class ComparisonImpl extends Cons
case EQUAL:
return PropertyValues.match(p1, p2);
case NOT_EQUAL:
- return !PropertyValues.match(p1, p2);
+ return PropertyValues.notMatch(p1, p2);
case GREATER_OR_EQUAL:
return p1.compareTo(p2) >= 0;
case GREATER_THAN:
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/FilterImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/FilterImpl.java?rev=1546341&r1=1546340&r2=1546341&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/FilterImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/FilterImpl.java Thu Nov 28 10:58:40 2013
@@ -252,16 +252,17 @@ public class FilterImpl implements Filte
switch (op) {
case EQUAL:
if (x.first != null && x.last == x.first && x.firstIncluding && x.lastIncluding) {
- // there is already an equality condition on this property
- // we will keep this, as it could be a multi-valued property
- // (unlike in databases, "x = 1 and x = 2" can match a node
- // if x is a multi-valued property with value "{1, 2}")
- return;
- }
- x.first = maxValue(oldFirst, v);
- x.firstIncluding = x.first == oldFirst ? x.firstIncluding : true;
- x.last = minValue(oldLast, v);
- x.lastIncluding = x.last == oldLast ? x.lastIncluding : true;
+ // we keep the old equality condition if there is one;
+ // we can not use setAlwaysFalse, as this would not be correct
+ // for multi-valued properties:
+ // unlike in databases, "x = 1 and x = 2" can match a node
+ // if x is a multi-valued property with value {1, 2}
+ } else {
+ // all other conditions (range conditions) are replaced with this one
+ // (we can not use setAlwaysFalse for the same reason as above)
+ x.first = x.last = v;
+ x.firstIncluding = x.lastIncluding = true;
+ }
break;
case NOT_EQUAL:
if (v != null) {
@@ -269,25 +270,40 @@ public class FilterImpl implements Filte
}
break;
case GREATER_THAN:
- x.first = maxValue(oldFirst, v);
- x.firstIncluding = false;
+ // we don't narrow the range because of multi-valued properties
+ if (x.first == null) {
+ x.first = maxValue(oldFirst, v);
+ x.firstIncluding = false;
+ }
break;
case GREATER_OR_EQUAL:
- x.first = maxValue(oldFirst, v);
- x.firstIncluding = x.first == oldFirst ? x.firstIncluding : true;
+ // we don't narrow the range because of multi-valued properties
+ if (x.first == null) {
+ x.first = maxValue(oldFirst, v);
+ x.firstIncluding = x.first == oldFirst ? x.firstIncluding : true;
+ }
break;
case LESS_THAN:
- x.last = minValue(oldLast, v);
- x.lastIncluding = false;
+ // we don't narrow the range because of multi-valued properties
+ if (x.last == null) {
+ x.last = minValue(oldLast, v);
+ x.lastIncluding = false;
+ }
break;
case LESS_OR_EQUAL:
- x.last = minValue(oldLast, v);
- x.lastIncluding = x.last == oldLast ? x.lastIncluding : true;
+ // we don't narrow the range because of multi-valued properties
+ if (x.last == null) {
+ x.last = minValue(oldLast, v);
+ x.lastIncluding = x.last == oldLast ? x.lastIncluding : true;
+ }
break;
case LIKE:
- // LIKE is handled in the fulltext index
- x.isLike = true;
- x.first = v;
+ // we don't narrow the range because of multi-valued properties
+ if (x.first == null) {
+ // LIKE is handled in the fulltext index
+ x.isLike = true;
+ x.first = v;
+ }
break;
case IN:
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/PropertyValues.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/PropertyValues.java?rev=1546341&r1=1546340&r2=1546341&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/PropertyValues.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/PropertyValues.java Thu Nov 28 10:58:40 2013
@@ -167,7 +167,7 @@ public final class PropertyValues {
}
if (!p1.isArray() && p2.isArray()) {
return contains(p2.getValue(Type.BINARIES),
- p2.getValue(Type.BINARY));
+ p1.getValue(Type.BINARY));
}
break;
default:
@@ -185,6 +185,53 @@ public final class PropertyValues {
}
+ public static boolean notMatch(PropertyValue p1, PropertyValue p2) {
+ if (p1.getType().tag() != p2.getType().tag()) {
+ return true;
+ }
+
+ switch (p1.getType().tag()) {
+ case PropertyType.BINARY:
+ if (p1.isArray() && !p2.isArray()) {
+ if (p1.count() > 1) {
+ // a value can not possibly match multiple distinct values
+ return true;
+ }
+ return !contains(p1.getValue(Type.BINARIES),
+ p2.getValue(Type.BINARY));
+ }
+ if (!p1.isArray() && p2.isArray()) {
+ if (p2.count() > 1) {
+ // a value can not possibly match multiple distinct values
+ return true;
+ }
+ return !contains(p2.getValue(Type.BINARIES),
+ p1.getValue(Type.BINARY));
+ }
+ break;
+ default:
+ if (p1.isArray() && !p2.isArray()) {
+ if (p1.count() > 1) {
+ // a value can not possibly match multiple distinct values
+ return true;
+ }
+ return !contains(p1.getValue(Type.STRINGS),
+ p2.getValue(Type.STRING));
+ }
+ if (!p1.isArray() && p2.isArray()) {
+ if (p2.count() > 1) {
+ // a value can not possibly match multiple distinct values
+ return true;
+ }
+ return !contains(p2.getValue(Type.STRINGS),
+ p1.getValue(Type.STRING));
+ }
+ }
+ // both arrays or both single values
+ return p1.compareTo(p2) != 0;
+
+ }
+
// --
/**
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/FilterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/FilterTest.java?rev=1546341&r1=1546340&r2=1546341&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/FilterTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/FilterTest.java Thu Nov 28 10:58:40 2013
@@ -48,12 +48,18 @@ public class FilterTest {
assertEquals("..2]", f.getPropertyRestriction("x").toString());
f.restrictProperty("x", Operator.GREATER_OR_EQUAL, one);
assertEquals("[1..2]", f.getPropertyRestriction("x").toString());
+
+ // further narrowing will not change the restriction,
+ // to account for multi-valued properties
f.restrictProperty("x", Operator.GREATER_THAN, one);
- assertEquals("(1..2]", f.getPropertyRestriction("x").toString());
+ assertEquals("[1..2]", f.getPropertyRestriction("x").toString());
f.restrictProperty("x", Operator.LESS_THAN, two);
- assertEquals("(1..2)", f.getPropertyRestriction("x").toString());
+ assertEquals("[1..2]", f.getPropertyRestriction("x").toString());
+
+ // this should replace the range with an equality
+ // (which is faster, and correct even when using multi-valued properties)
f.restrictProperty("x", Operator.EQUAL, two);
- assertTrue(f.isAlwaysFalse());
+ assertEquals("2", f.getPropertyRestriction("x").toString());
f = new FilterImpl();
f.restrictProperty("x", Operator.EQUAL, one);
@@ -64,22 +70,31 @@ public class FilterTest {
assertEquals("1", f.getPropertyRestriction("x").toString());
f.restrictProperty("x", Operator.LESS_OR_EQUAL, one);
assertEquals("1", f.getPropertyRestriction("x").toString());
+
+ // further narrowing will not change the restriction,
+ // to account for multi-valued properties
f.restrictProperty("x", Operator.GREATER_THAN, one);
- assertTrue(f.isAlwaysFalse());
+ assertEquals("1", f.getPropertyRestriction("x").toString());
f = new FilterImpl();
f.restrictProperty("x", Operator.EQUAL, one);
assertEquals("1", f.getPropertyRestriction("x").toString());
+
+ // further narrowing will not change the restriction,
+ // to account for multi-valued properties
f.restrictProperty("x", Operator.LESS_THAN, one);
- assertTrue(f.isAlwaysFalse());
+ assertEquals("1", f.getPropertyRestriction("x").toString());
f = new FilterImpl();
f.restrictProperty("x", Operator.NOT_EQUAL, null);
assertEquals("", f.getPropertyRestriction("x").toString());
f.restrictProperty("x", Operator.LESS_THAN, one);
assertEquals("..1)", f.getPropertyRestriction("x").toString());
+
+ // this should replace the range with an equality
+ // (which is faster, and correct even when using multi-valued properties)
f.restrictProperty("x", Operator.EQUAL, two);
- assertTrue(f.isAlwaysFalse());
+ assertEquals("2", f.getPropertyRestriction("x").toString());
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java?rev=1546341&r1=1546340&r2=1546341&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/index/TraversingIndexQueryTest.java Thu Nov 28 10:58:40 2013
@@ -16,6 +16,7 @@ package org.apache.jackrabbit.oak.query.
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.api.Type;
import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent;
import org.apache.jackrabbit.oak.query.AbstractQueryTest;
import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
@@ -70,4 +71,89 @@ public class TraversingIndexQueryTest ex
"xpath",
ImmutableList.of("/content/testFullTextTermNameFile.txt"));
}
+
+ @Test
+ public void testMultiNotEqual() throws Exception {
+ Tree c = root.getTree("/").addChild("content");
+
+ c.addChild("one").setProperty("prop", "value");
+ c.addChild("two").setProperty("prop",
+ ImmutableList.of("aaa", "value", "bbb"), Type.STRINGS);
+ c.addChild("three").setProperty("prop",
+ ImmutableList.of("aaa", "bbb", "ccc"), Type.STRINGS);
+ root.commit();
+
+ assertQuery("//*[@prop != 'value']", "xpath",
+ ImmutableList.of("/content/two", "/content/three"));
+ }
+
+ @Test
+ public void testMultiAndEquals() throws Exception {
+ Tree c = root.getTree("/").addChild("content");
+
+ c.addChild("one").setProperty("prop", "aaa");
+ c.addChild("two").setProperty("prop",
+ ImmutableList.of("aaa", "bbb", "ccc"), Type.STRINGS);
+ c.addChild("three").setProperty("prop", ImmutableList.of("aaa", "bbb"),
+ Type.STRINGS);
+ root.commit();
+
+ assertQuery("//*[(@prop = 'aaa' and @prop = 'bbb' and @prop = 'ccc')]",
+ "xpath", ImmutableList.of("/content/two"));
+ }
+
+ @Test
+ public void testMultiAndLike() throws Exception {
+ Tree c = root.getTree("/").addChild("content");
+
+ c.addChild("one").setProperty("prop", "aaaBoom");
+ c.addChild("two").setProperty("prop",
+ ImmutableList.of("aaaBoom", "bbbBoom", "cccBoom"), Type.STRINGS);
+ c.addChild("three").setProperty("prop", ImmutableList.of("aaaBoom", "bbbBoom"),
+ Type.STRINGS);
+ root.commit();
+
+ assertQuery("//*[(jcr:like(@prop, 'aaa%') and jcr:like(@prop, 'bbb%') and jcr:like(@prop, 'ccc%'))]",
+ "xpath", ImmutableList.of("/content/two"));
+ }
+
+ @Test
+ public void testSubPropertyMultiAndEquals() throws Exception {
+ Tree c = root.getTree("/").addChild("content");
+
+ c.addChild("one").addChild("child").setProperty("prop", "aaa");
+ c.addChild("two")
+ .addChild("child")
+ .setProperty("prop", ImmutableList.of("aaa", "bbb", "ccc"),
+ Type.STRINGS);
+ c.addChild("three")
+ .addChild("child")
+ .setProperty("prop", ImmutableList.of("aaa", "bbb"),
+ Type.STRINGS);
+ root.commit();
+
+ assertQuery(
+ "//*[(child/@prop = 'aaa' and child/@prop = 'bbb' and child/@prop = 'ccc')]",
+ "xpath", ImmutableList.of("/content/two"));
+ }
+
+ @Test
+ public void testSubPropertyMultiAndLike() throws Exception {
+ Tree c = root.getTree("/").addChild("content");
+
+ c.addChild("one").addChild("child").setProperty("prop", "aaaBoom");
+ c.addChild("two")
+ .addChild("child")
+ .setProperty("prop", ImmutableList.of("aaaBoom", "bbbBoom", "cccBoom"),
+ Type.STRINGS);
+ c.addChild("three")
+ .addChild("child")
+ .setProperty("prop", ImmutableList.of("aaaBoom", "bbbBoom"),
+ Type.STRINGS);
+ root.commit();
+
+ assertQuery(
+ "//*[(jcr:like(child/@prop, 'aaa%') and jcr:like(child/@prop, 'bbb%') and jcr:like(child/@prop, 'ccc%'))]",
+ "xpath", ImmutableList.of("/content/two"));
+ }
}