### Eclipse Workspace Patch 1.0 #P oak-core Index: src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java (working copy) @@ -50,10 +50,11 @@ */ public static final boolean JACKRABBIT_2_SINGLE_QUOTED_PHRASE = true; - private final String selectorName; + protected final String selectorName; + protected final String propertyName; + protected final StaticOperandImpl fullTextSearchExpression; + private final String relativePath; - private final String propertyName; - private final StaticOperandImpl fullTextSearchExpression; private SelectorImpl selector; public FullTextSearchImpl( @@ -84,6 +85,11 @@ public StaticOperandImpl getFullTextSearchExpression() { return fullTextSearchExpression; } + + @Override + ConstraintImpl not() { + return new NotFullTextSearchImpl(this); + } @Override boolean accept(AstVisitor v) { @@ -139,7 +145,7 @@ p = PathUtils.concat(relativePath, p); } String p2 = normalizePropertyName(p); - String rawText = v.getValue(Type.STRING); + String rawText = getRawFulltextString(v); FullTextExpression e = FullTextParser.parse(p2, rawText); return new FullTextContains(p2, rawText, e); } catch (ParseException e) { @@ -147,11 +153,21 @@ } } + /** + * Get the fulltext string. + * + * @param v the property value + * @return the string + */ + String getRawFulltextString(PropertyValue v) { + return v.getValue(Type.STRING); + } + @Override public Set getSelectors() { return Collections.singleton(selector); } - + @Override public boolean evaluate() { // disable evaluation if a fulltext index is used, @@ -163,10 +179,7 @@ // condition checks out, this takes out some extra rows from the index // aggregation bits if (relativePath == null && propertyName != null) { - PropertyValue p = selector.currentProperty(propertyName); - if (p == null) { - return false; - } + return enforcePropertyExistence(propertyName, selector); } return true; } @@ -259,7 +272,7 @@ p = PathUtils.concat(p, relativePath); } p = normalizePropertyName(p); - f.restrictProperty(p, Operator.NOT_EQUAL, null); + restrictPropertyOnFilter(p, f); } } f.restrictFulltextCondition(fullTextSearchExpression.currentValue().getValue(Type.STRING)); @@ -272,4 +285,31 @@ } } + /** + * restrict the provided property to the property to the provided filter achieving so + * {@code property IS NOT NULL} + * + * @param propertyName + * @param f + */ + void restrictPropertyOnFilter(String propertyName, FilterImpl f) { + ; + f.restrictProperty(propertyName, Operator.NOT_EQUAL, null); + } + + /** + * verify that a property exists in the node. {@code property IS NOT NULL} + * + * @param propertyName the property to check + * @param selector the selector to work with + * @return true if the property is there, false otherwise. + */ + boolean enforcePropertyExistence(String propertyName, SelectorImpl selector) { + PropertyValue p = selector.currentProperty(propertyName); + if (p == null) { + return false; + } + return true; + } + } Index: src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java (revision 1703140) +++ src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java (working copy) @@ -20,8 +20,14 @@ import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.oak.plugins.index.IndexUtils.createIndexDefinition; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.NT_UNSTRUCTURED; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.ArrayList; +import java.util.List; + +import javax.jcr.query.Query; import org.apache.jackrabbit.oak.Oak; import org.apache.jackrabbit.oak.api.ContentRepository; @@ -48,8 +54,10 @@ .createContentRepository(); } - private static void child(Tree t, String n, String type) { - t.addChild(n).setProperty(JCR_PRIMARYTYPE, type, Type.NAME); + private static Tree child(Tree t, String n, String type) { + Tree t1 = t.addChild(n); + t1.setProperty(JCR_PRIMARYTYPE, type, Type.NAME); + return t1; } private static void mixLanguage(Tree t, String n) { @@ -84,4 +92,33 @@ setTraversalEnabled(true); } + + @Test + public void oak3371() throws Exception { + setTraversalEnabled(false); + Tree t, t1; + + NodeUtil n = new NodeUtil(root.getTree("/oak:index")); + createIndexDefinition(n, "nodeType", false, new String[] { + JCR_PRIMARYTYPE }, new String[] { NT_UNSTRUCTURED }); + + root.commit(); + + t = root.getTree("/"); + t = child(t, "test", NT_UNSTRUCTURED); + t1 = child(t, "a", NT_UNSTRUCTURED); + t1.setProperty("foo", "bar"); + child(t, "b", NT_UNSTRUCTURED); + + root.commit(); + + List 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")); + + setTraversalEnabled(true); + } } \ No newline at end of file Index: src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java (working copy) @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("1.0") +@Version("1.1") @Export(optional = "provide:=true") package org.apache.jackrabbit.oak.query.fulltext; Index: src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java (revision 0) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java (working copy) @@ -0,0 +1,68 @@ +/* + * 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.query.ast; + +import org.apache.jackrabbit.oak.api.PropertyValue; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.query.index.FilterImpl; + +public class NotFullTextSearchImpl extends FullTextSearchImpl { + + public NotFullTextSearchImpl(String selectorName, String propertyName, + StaticOperandImpl fullTextSearchExpression) { + super(selectorName, propertyName, fullTextSearchExpression); + } + + 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 getRawFulltextString(PropertyValue v) { + return "-" + super.getRawFulltextString(v); + } + + @Override + public void restrict(FilterImpl f) { + // The "not null" conditions is not used, as "not contains()" can be + // valid if the property is actually not there + f.restrictFulltextCondition(fullTextSearchExpression.currentValue().getValue(Type.STRING)); + } + + @Override + void restrictPropertyOnFilter(String propertyName, FilterImpl f) { + // Intentionally left empty. A NOT CONTAINS() can be valid if the property is actually not + // there. + } + + @Override + public String toString() { + return "not " + super.toString(); + } + + @Override + boolean enforcePropertyExistence(String propertyName, SelectorImpl selector) { + // in case of NOT CONTAINS we want to match nodes without the property as well. In this way + // we don't care whether the property is there or not. + return true; + } +} Index: src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java (working copy) @@ -89,4 +89,13 @@ */ public abstract boolean accept(FullTextVisitor v); + /** + * 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. + */ + public boolean isNot() { + return false; + } } \ No newline at end of file Index: src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextTerm.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextTerm.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextTerm.java (working copy) @@ -197,6 +197,7 @@ return boost; } + @Override public boolean isNot() { return not; } Index: src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java (working copy) @@ -37,6 +37,16 @@ } /** + * 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 Index: src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java (working copy) @@ -75,4 +75,8 @@ return rawText; } + @Override + public boolean isNot() { + return base.isNot(); + } } Index: src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java (working copy) @@ -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 @@ @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 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 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 Index: src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java (working copy) @@ -129,6 +129,16 @@ } @Override + ConstraintImpl not() { + // not (X or Y) == (not X) and (not Y) + List list = newArrayList(); + for (ConstraintImpl constraint : getConstraints()) { + list.add(new NotImpl(constraint)); + } + return new AndImpl(list).simplify(); + } + + @Override public Set getPropertyExistenceConditions() { // for the condition "x=1 or x=2", the existence condition // "x is not null" be be derived Index: src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java (revision 1703140) +++ src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java (working copy) @@ -84,6 +84,16 @@ } @Override + ConstraintImpl not() { + // not (X and Y) == (not X) or (not Y) + List list = newArrayList(); + for (ConstraintImpl constraint : constraints) { + list.add(new NotImpl(constraint)); + } + return new OrImpl(list).simplify(); + } + + @Override public Set getPropertyExistenceConditions() { Set result = newHashSet(); for (ConstraintImpl constraint : constraints) {