diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java index e2ec02f..74d578f 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java @@ -104,6 +104,9 @@ public class AstElementFactory { } public NotImpl not(ConstraintImpl constraint) { + if (constraint instanceof FullTextSearchImpl) { + return new NotFullTextImpl((FullTextSearchImpl) constraint); + } return new NotImpl(constraint); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java index d1dc40f..346390d 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java @@ -50,10 +50,10 @@ public class FullTextSearchImpl extends ConstraintImpl { */ public static final boolean JACKRABBIT_2_SINGLE_QUOTED_PHRASE = true; - private final String selectorName; + final String selectorName; private final String relativePath; - private final String propertyName; - private final StaticOperandImpl fullTextSearchExpression; + final String propertyName; + final StaticOperandImpl fullTextSearchExpression; private SelectorImpl selector; public FullTextSearchImpl( @@ -139,7 +139,7 @@ public class FullTextSearchImpl extends ConstraintImpl { p = PathUtils.concat(relativePath, p); } String p2 = normalizePropertyName(p); - String rawText = v.getValue(Type.STRING); + String rawText = getRawText(v); FullTextExpression e = FullTextParser.parse(p2, rawText); return new FullTextContains(p2, rawText, e); } catch (ParseException e) { @@ -147,11 +147,30 @@ public class FullTextSearchImpl extends ConstraintImpl { } } + String getRawText(PropertyValue v) { + return v.getValue(Type.STRING); + } + @Override public Set getSelectors() { return Collections.singleton(selector); } + /** + * 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; + } + @Override public boolean evaluate() { // disable evaluation if a fulltext index is used, @@ -163,10 +182,7 @@ public class FullTextSearchImpl extends ConstraintImpl { // 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 +275,7 @@ public class FullTextSearchImpl extends ConstraintImpl { 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 +288,14 @@ public class FullTextSearchImpl extends ConstraintImpl { } } + /** + * 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); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextImpl.java new file mode 100644 index 0000000..52db82d --- /dev/null +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextImpl.java @@ -0,0 +1,56 @@ +/* + * 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 static com.google.common.base.Preconditions.checkNotNull; + +import javax.annotation.Nonnull; + +import org.apache.jackrabbit.oak.query.fulltext.FullTextExpression; +import org.apache.jackrabbit.oak.query.index.FilterImpl; + +/** + * class used to "wrap" a {@code NOT CONTAINS} clause. The main differences with a {@link NotImpl} + * reside in the {@link NotImpl#evaluate()} and restricts. + */ +public class NotFullTextImpl extends NotImpl { + public NotFullTextImpl(@Nonnull FullTextSearchImpl constraint) { + super(new NotFullTextSearchImpl(checkNotNull(constraint))); + } + + @Override + public boolean evaluate() { + return getConstraint().evaluate(); + } + + @Override + public void restrict(FilterImpl f) { + if (!f.getSelector().isOuterJoinRightHandSide()) { + getConstraint().restrict(f); + } + } + + @Override + public void restrictPushDown(SelectorImpl s) { + getConstraint().restrictPushDown(s); + } + + @Override + public FullTextExpression getFullTextConstraint(SelectorImpl s) { + return getConstraint().getFullTextConstraint(s); + } +} diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java new file mode 100644 index 0000000..79558ba --- /dev/null +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotFullTextSearchImpl.java @@ -0,0 +1,55 @@ +/* + * 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.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 + String getRawText(PropertyValue v) { + return "-" + super.getRawText(v); + } + + @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; + } +} diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java index f5a72e8..715a405 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextContains.java @@ -75,4 +75,8 @@ public class FullTextContains extends FullTextExpression { return rawText; } + @Override + public boolean isNot() { + return base.isNot(); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java index 8d65c07..695e77c 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/FullTextExpression.java @@ -89,4 +89,13 @@ public abstract class FullTextExpression { */ 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 diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java index 8c02f6d..a072685 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/fulltext/package-info.java @@ -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; diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java index 605a59d..2c5dc5d 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexQueryTest.java @@ -20,8 +20,14 @@ import static com.google.common.collect.ImmutableList.of; 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 @@ public class NodeTypeIndexQueryTest extends AbstractQueryTest { .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 @@ public class NodeTypeIndexQueryTest extends AbstractQueryTest { 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 diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java index 8bdb3fd..8e11122 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java @@ -70,7 +70,6 @@ import org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction; import org.apache.jackrabbit.oak.spi.query.IndexRow; import org.apache.jackrabbit.oak.spi.query.PropertyValues; import org.apache.jackrabbit.oak.spi.query.QueryConstants; -import org.apache.jackrabbit.oak.spi.query.QueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndex.AdvanceFulltextQueryIndex; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.lucene.analysis.Analyzer; @@ -588,14 +587,8 @@ public class LuceneIndex implements AdvanceFulltextQueryIndex { if (qs.size() == 0) { return new LuceneRequestFacade(new MatchAllDocsQuery()); } - if (qs.size() == 1) { - return new LuceneRequestFacade(qs.get(0)); - } - BooleanQuery bq = new BooleanQuery(); - for (Query q : qs) { - bq.add(q, MUST); - } - return new LuceneRequestFacade(bq); + + return LucenePropertyIndex.performAdditionalWraps(qs); } private static void addNonFullTextConstraints(List qs, diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java index 3d3754e..76bd3cc 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java @@ -29,6 +29,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.jcr.PropertyType; @@ -37,6 +38,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Queues; import com.google.common.collect.Sets; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.api.Result.SizePrecision; import org.apache.jackrabbit.oak.api.Type; @@ -102,6 +104,7 @@ import org.apache.lucene.util.Version; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Lists.newArrayListWithCapacity; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; @@ -654,11 +657,43 @@ public class LucenePropertyIndex implements AdvancedQueryIndex, QueryIndex, Nati throw new IllegalStateException("No query created for filter " + filter); } + return performAdditionalWraps(qs); + } + + /** + * Perform additional wraps on the list of queries to allow, for example, the NOT CONTAINS to + * play properly when sent to lucene. + * + * @param qs the list of queries. Cannot be null. + * @return + */ + @Nonnull + public static LuceneRequestFacade performAdditionalWraps(@Nonnull List qs) { + checkNotNull(qs); if (qs.size() == 1) { + Query q = qs.get(0); + if (q instanceof BooleanQuery) { + BooleanQuery ibq = (BooleanQuery) q; + if ((ibq.getClauses().length == 1) && + (ibq.getClauses()[0].getOccur() == BooleanClause.Occur.MUST_NOT)) { + ibq.add(new MatchAllDocsQuery(), BooleanClause.Occur.SHOULD); + } + } return new LuceneRequestFacade(qs.get(0)); } BooleanQuery bq = new BooleanQuery(); for (Query q : qs) { + /* Only unwrap the clause if MUST_NOT(x) i.e. if there is boolean query + * with single MUST_NOT clause then extract it and pace it in outer clause*/ + if (q instanceof BooleanQuery) { + BooleanQuery ibq = (BooleanQuery) q; + if ((ibq.getClauses().length == 1) && + (ibq.getClauses()[0].getOccur() == BooleanClause.Occur.MUST_NOT)) { + bq.add(ibq.getClauses()[0]); + continue; + } + } + bq.add(q, MUST); } return new LuceneRequestFacade(bq); @@ -1028,7 +1063,7 @@ public class LucenePropertyIndex implements AdvancedQueryIndex, QueryIndex, Nati @Override public boolean visit(FullTextContains contains) { - visitTerm(contains.getPropertyName(), contains.getRawText(), null, false); + visitTerm(contains.getPropertyName(), contains.getRawText(), null, contains.isNot()); return true; } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAggregationTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAggregationTest.java index f996634..7082985 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAggregationTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAggregationTest.java @@ -25,18 +25,23 @@ import static org.apache.jackrabbit.JcrConstants.JCR_DATA; import java.util.ArrayList; import java.util.Calendar; +import java.util.List; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.CommitFailedException; 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.index.IndexConstants; import org.apache.jackrabbit.oak.plugins.index.aggregate.NodeAggregator; import org.apache.jackrabbit.oak.plugins.index.aggregate.SimpleNodeAggregator; import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.newNodeAggregator; import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.useV2; import static org.apache.jackrabbit.oak.plugins.memory.BinaryPropertyState.binaryProperty; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent; import org.apache.jackrabbit.oak.query.AbstractQueryTest; @@ -46,6 +51,7 @@ import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; import org.junit.Test; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; public class LuceneIndexAggregationTest extends AbstractQueryTest { @@ -412,4 +418,51 @@ public class LuceneIndexAggregationTest extends AbstractQueryTest { "xpath", ImmutableList.of("/myFolder", "/myFolder/myFile", "/myFolder/myFile/jcr:content")); } + @Test + public void oak3371AggregateV2() throws CommitFailedException { + oak3371(); + } + + @Test + public void oak3371AggregateV1() throws CommitFailedException { + + Tree indexdef = root.getTree("/oak:index/" + TEST_INDEX_NAME); + assertNotNull(indexdef); + assertTrue(indexdef.exists()); + indexdef.setProperty(LuceneIndexConstants.COMPAT_MODE, 1L); + indexdef.setProperty(IndexConstants.REINDEX_PROPERTY_NAME, true); + root.commit(); + + oak3371(); + } + + private void oak3371() throws CommitFailedException { + setTraversalEnabled(false); + List contains = newArrayList(), notContains = newArrayList(); + Tree test, t; + + test = root.getTree("/").addChild("test"); + t = test.addChild("a"); + t.setProperty(JCR_PRIMARYTYPE, NT_FOLDER, Type.NAME); + t.setProperty("foo", "bar"); + contains.add(t.getPath()); + t = test.addChild("b"); + t.setProperty(JCR_PRIMARYTYPE, NT_FOLDER, Type.NAME); + t.setProperty("foo", "cat"); + notContains.add(t.getPath()); + t = test.addChild("c"); + t.setProperty(JCR_PRIMARYTYPE, NT_FOLDER, Type.NAME); + notContains.add(t.getPath()); + root.commit(); + + assertQuery( + "SELECT * FROM [nt:folder] WHERE ISDESCENDANTNODE('/test') AND CONTAINS(foo, 'bar')", + contains); + + assertQuery( + "SELECT * FROM [nt:folder] WHERE ISDESCENDANTNODE('/test') AND NOT CONTAINS(foo, 'bar')", + notContains); + + setTraversalEnabled(true); + } } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java index 5b12eb8..daeb817 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java @@ -514,4 +514,36 @@ public class LuceneIndexQueryTest extends AbstractQueryTest { walktree(t1); } } + + private static Tree child(Tree t, String n, String type) { + Tree t1 = t.addChild(n); + t1.setProperty(JCR_PRIMARYTYPE, type, Type.NAME); + return t1; + } + + @Test + public void oak3371() throws Exception { + setTraversalEnabled(false); + Tree t, t1; + + t = root.getTree("/"); + t = child(t, "test", NT_UNSTRUCTURED); + t1 = child(t, "a", NT_UNSTRUCTURED); + t1.setProperty("foo", "bar"); + t1 = child(t, "b", NT_UNSTRUCTURED); + t1.setProperty("foo", "cat"); + t1 = child(t, "c", NT_UNSTRUCTURED); + + root.commit(); + + assertQuery( + "SELECT * FROM [nt:unstructured] WHERE ISDESCENDANTNODE('/test') AND CONTAINS(foo, 'bar')", + of("/test/a")); + + assertQuery( + "SELECT * FROM [nt:unstructured] WHERE ISDESCENDANTNODE('/test') AND NOT CONTAINS(foo, 'bar')", + of("/test/b", "/test/c")); + + setTraversalEnabled(true); + } }