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 2015/10/09 15:02:20 UTC
svn commit: r1707715 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/query/ast/
main/java/org/apache/jackrabbit/oak/query/fulltext/
test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/
Author: thomasm
Date: Fri Oct 9 13:02:19 2015
New Revision: 1707715
URL: http://svn.apache.org/viewvc?rev=1707715&view=rev
Log:
OAK-3486 Wrong evaluation of NOT NOT clause (see OAK-3371)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.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/fulltext/FullTextExpression.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java Fri Oct 9 13:02:19 2015
@@ -84,6 +84,16 @@ public class AndImpl extends ConstraintI
}
@Override
+ ConstraintImpl not() {
+ // not (X and Y) == (not X) or (not Y)
+ List<ConstraintImpl> list = newArrayList();
+ for (ConstraintImpl constraint : constraints) {
+ list.add(new NotImpl(constraint));
+ }
+ return new OrImpl(list).simplify();
+ }
+
+ @Override
public Set<PropertyExistenceImpl> getPropertyExistenceConditions() {
Set<PropertyExistenceImpl> result = newHashSet();
for (ConstraintImpl constraint : constraints) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java Fri Oct 9 13:02:19 2015
@@ -104,9 +104,6 @@ public class AstElementFactory {
}
public NotImpl not(ConstraintImpl constraint) {
- if (constraint instanceof FullTextSearchImpl) {
- return new NotFullTextImpl((FullTextSearchImpl) constraint);
- }
return new NotImpl(constraint);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java Fri Oct 9 13:02:19 2015
@@ -37,6 +37,16 @@ public abstract class ConstraintImpl ext
}
/**
+ * Get the negative constraint, if it is simpler, or null. For example,
+ * "not x = 1" returns "x = 1", but "x = 1" returns null.
+ *
+ * @return the negative constraint, or null
+ */
+ ConstraintImpl not() {
+ return null;
+ }
+
+ /**
* Evaluate the result using the currently set values.
*
* @return true if the constraint matches
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java Fri Oct 9 13:02:19 2015
@@ -86,6 +86,11 @@ public class FullTextSearchImpl extends
}
@Override
+ ConstraintImpl not() {
+ return new NotFullTextSearchImpl(this);
+ }
+
+ @Override
boolean accept(AstVisitor v) {
return v.visit(this);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java Fri Oct 9 13:02:19 2015
@@ -40,7 +40,13 @@ public class NotFullTextSearchImpl exten
public NotFullTextSearchImpl(FullTextSearchImpl ft) {
this(ft.selectorName, ft.propertyName, ft.fullTextSearchExpression);
}
-
+
+ @Override
+ ConstraintImpl not() {
+ return new FullTextSearchImpl(this.selectorName, this.propertyName,
+ this.fullTextSearchExpression);
+ }
+
@Override
String getRawText(PropertyValue v) {
Iterable<String> terms = SPACE_SPLITTER.split(super.getRawText(v));
@@ -55,7 +61,7 @@ public class NotFullTextSearchImpl exten
return raw.toString().trim();
}
- private boolean isKeyword(@Nonnull String term) {
+ private static boolean isKeyword(@Nonnull String term) {
return KEYWORDS.contains(checkNotNull(term).toLowerCase());
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java Fri Oct 9 13:02:19 2015
@@ -18,10 +18,7 @@
*/
package org.apache.jackrabbit.oak.query.ast;
-import static com.google.common.collect.Lists.newArrayList;
-
import java.util.Collections;
-import java.util.List;
import java.util.Set;
import org.apache.jackrabbit.oak.query.index.FilterImpl;
@@ -47,30 +44,19 @@ public class NotImpl extends ConstraintI
@Override
public ConstraintImpl simplify() {
ConstraintImpl simple = constraint.simplify();
- if (simple instanceof AndImpl) {
- // not (X and Y) == (not X) or (not Y)
- AndImpl and = (AndImpl) simple;
- List<ConstraintImpl> constraints = newArrayList();
- for (ConstraintImpl constraint : and.getConstraints()) {
- constraints.add(new NotImpl(constraint));
- }
- return new OrImpl(constraints).simplify();
- } else if (simple instanceof OrImpl) {
- // not (X or Y) == (not X) and (not Y)
- OrImpl or = (OrImpl) simple;
- List<ConstraintImpl> constraints = newArrayList();
- for (ConstraintImpl constraint : or.getConstraints()) {
- constraints.add(new NotImpl(constraint));
- }
- return new AndImpl(constraints).simplify();
- } else if (simple instanceof NotImpl) {
- // not not X == X
- return ((NotImpl) simple).constraint;
+ ConstraintImpl not = simple.not();
+ if (not != null) {
+ return not.simplify();
} else if (simple != constraint) {
return new NotImpl(simple);
- } else {
- return this;
}
+ return this;
+ }
+
+ @Override
+ ConstraintImpl not() {
+ // not not X == X -> X == X
+ return constraint;
}
@Override
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=1707715&r1=1707714&r2=1707715&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 Fri Oct 9 13:02:19 2015
@@ -129,6 +129,16 @@ public class OrImpl extends ConstraintIm
}
@Override
+ ConstraintImpl not() {
+ // not (X or Y) == (not X) and (not Y)
+ List<ConstraintImpl> list = newArrayList();
+ for (ConstraintImpl constraint : getConstraints()) {
+ list.add(new NotImpl(constraint));
+ }
+ return new AndImpl(list).simplify();
+ }
+
+ @Override
public Set<PropertyExistenceImpl> getPropertyExistenceConditions() {
// for the condition "x=1 or x=2", the existence condition
// "x is not null" be be derived
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java Fri Oct 9 13:02:19 2015
@@ -90,7 +90,7 @@ public abstract class FullTextExpression
public abstract boolean accept(FullTextVisitor v);
/**
- * whether the current {@link FullTextExpression} is a {@code NOT} condition or not. Default is
+ * Whether the current {@link FullTextExpression} is a {@code NOT} condition or not. Default is
* false
*
* @return true if the current condition represent a NOT, false otherwise.
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java?rev=1707715&r1=1707714&r2=1707715&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java Fri Oct 9 13:02:19 2015
@@ -112,12 +112,43 @@ public class NodeTypeIndexQueryTest exte
root.commit();
- List<String> plan = executeQuery(
- "explain SELECT * FROM [nt:unstructured] WHERE ISDESCENDANTNODE([/test]) AND NOT CONTAINS(foo, 'bar')",
- Query.JCR_SQL2, false);
+ List<String> plan;
+
+ plan = executeQuery(
+ "explain SELECT * FROM [nt:unstructured] " +
+ "WHERE ISDESCENDANTNODE([/test]) " +
+ "AND CONTAINS(foo, 'bar')",
+ Query.JCR_SQL2, false);
+ assertEquals(1, plan.size());
+ assertTrue(plan.get(0).contains("no-index"));
+ assertEquals("[nt:unstructured] as [nt:unstructured] /* no-index\n" +
+ " where (isdescendantnode([nt:unstructured], [/test]))\n" +
+ " and (contains([nt:unstructured].[foo], 'bar')) */",
+ plan.get(0));
+
+ plan = executeQuery(
+ "explain SELECT * FROM [nt:unstructured] " +
+ "WHERE ISDESCENDANTNODE([/test]) " +
+ "AND NOT CONTAINS(foo, 'bar')",
+ Query.JCR_SQL2, false);
+ assertEquals(1, plan.size());
+ assertTrue(plan.get(0).contains("no-index"));
+ assertEquals("[nt:unstructured] as [nt:unstructured] /* no-index\n" +
+ " where (isdescendantnode([nt:unstructured], [/test]))\n" +
+ " and (not contains([nt:unstructured].[foo], 'bar')) */",
+ plan.get(0));
+ plan = executeQuery(
+ "explain SELECT * FROM [nt:unstructured] " +
+ "WHERE ISDESCENDANTNODE([/test]) " +
+ "AND NOT NOT CONTAINS(foo, 'bar')",
+ Query.JCR_SQL2, false);
assertEquals(1, plan.size());
assertTrue(plan.get(0).contains("no-index"));
+ assertEquals("[nt:unstructured] as [nt:unstructured] /* no-index\n" +
+ " where (isdescendantnode([nt:unstructured], [/test]))\n" +
+ " and (contains([nt:unstructured].[foo], 'bar')) */",
+ plan.get(0));
setTraversalEnabled(true);
}