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 ca...@apache.org on 2016/01/07 15:56:37 UTC
svn commit: r1723565 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/
oak-core/src/main/java/org/apache/jackrabbit/oak/query/index/
oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/
oak-lucene/src/main/j...
Author: catholicon
Date: Thu Jan 7 14:56:36 2016
New Revision: 1723565
URL: http://svn.apache.org/viewvc?rev=1723565&view=rev
Log:
OAK-3838: IndexPlanner incorrectly lets all full text indices to participate for suggest/spellcheck queries
Index planner, for spellcheck and suggest queries, would now validate selectNodeTypeName=indexRule.nodeTypeName and that at least one property in the index need to supporting the features.
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.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/Filter.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlannerTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexSuggestionTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java Thu Jan 7 14:56:36 2016
@@ -193,6 +193,10 @@ public class SelectorImpl extends Source
return selectorName;
}
+ public String getNodeType() {
+ return nodeTypeName;
+ }
+
public boolean matchesAllTypes() {
return matchesAllTypes;
}
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=1723565&r1=1723564&r2=1723565&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 Jan 7 14:56:36 2016
@@ -223,6 +223,14 @@ public class FilterImpl implements Filte
return selector;
}
+ @Override @Nullable
+ public String getNodeType() {
+ if (selector == null) {
+ return null;
+ }
+ return selector.getNodeType();
+ }
+
@Override
public boolean matchesAllTypes() {
return matchesAllTypes;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Filter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Filter.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Filter.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Filter.java Thu Jan 7 14:56:36 2016
@@ -131,6 +131,14 @@ public interface Filter {
String getPathPlan();
/**
+ * Returns the name of the filter node type.
+ *
+ * @return nodetype name
+ */
+ @Nullable
+ String getNodeType();
+
+ /**
* Checks whether nodes of all types can match this filter.
*
* @return {@code true} iff there are no type restrictions
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java Thu Jan 7 14:56:36 2016
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@Version("4.0.0")
+@Version("5.0.0")
@Export(optional = "provide:=true")
package org.apache.jackrabbit.oak.spi.query;
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java Thu Jan 7 14:56:36 2016
@@ -207,6 +207,10 @@ class IndexDefinition implements Aggrega
private final boolean secureFacets;
+ private final boolean suggestEnabled;
+
+ private final boolean spellcheckEnabled;
+
public IndexDefinition(NodeState root, NodeState defn) {
this(root, defn, null);
}
@@ -274,6 +278,8 @@ class IndexDefinition implements Aggrega
this.saveDirListing = getOptionalValue(defn, LuceneIndexConstants.SAVE_DIR_LISTING, true);
this.suggestAnalyzed = getOptionalValue(defn, LuceneIndexConstants.SUGGEST_ANALYZED, false);
this.secureFacets = defn.hasChildNode(FACETS) && getOptionalValue(defn.getChildNode(FACETS), PROP_SECURE_FACETS, true);
+ this.suggestEnabled = evaluateSuggestionEnabled();
+ this.spellcheckEnabled = evaluateSpellcheckEnabled();
}
public NodeState getDefinitionNodeState() {
@@ -595,25 +601,46 @@ class IndexDefinition implements Aggrega
return ntBaseRule != null;
}
- public boolean isSuggestEnabled() {
- boolean suggestEnabled = false;
+ private boolean evaluateSuggestionEnabled() {
for (IndexingRule indexingRule : definedRules) {
for (PropertyDefinition propertyDefinition : indexingRule.propConfigs.values()) {
if (propertyDefinition.useInSuggest) {
- suggestEnabled = true;
- break;
+ return true;
}
}
for (NamePattern np : indexingRule.namePatterns) {
if (np.getConfig().useInSuggest) {
- suggestEnabled = true;
- break;
+ return true;
}
}
}
+ return false;
+ }
+
+ public boolean isSuggestEnabled() {
return suggestEnabled;
}
+ private boolean evaluateSpellcheckEnabled() {
+ for (IndexingRule indexingRule : definedRules) {
+ for (PropertyDefinition propertyDefinition : indexingRule.propConfigs.values()) {
+ if (propertyDefinition.useInSpellcheck) {
+ return true;
+ }
+ }
+ for (NamePattern np : indexingRule.namePatterns) {
+ if (np.getConfig().useInSpellcheck) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ public boolean isSpellcheckEnabled() {
+ return spellcheckEnabled;
+ }
+
@CheckForNull
public String getIndexPathFromConfig() {
return definition.getString(LuceneIndexConstants.INDEX_PATH);
@@ -730,6 +757,13 @@ class IndexDefinition implements Aggrega
return nodeTypeName;
}
+ /**
+ * @return name of the base node type.
+ */
+ public String getBaseNodeType() {
+ return baseNodeType;
+ }
+
public List<PropertyDefinition> getNullCheckEnabledProperties() {
return nullCheckEnabledProperties;
}
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java Thu Jan 7 14:56:36 2016
@@ -30,6 +30,7 @@ import javax.annotation.CheckForNull;
import com.google.common.collect.Iterables;
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.api.PropertyValue;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.index.lucene.IndexDefinition.IndexingRule;
@@ -139,9 +140,7 @@ class IndexPlanner {
if (definition.hasFunctionDefined()
&& filter.getPropertyRestriction(definition.getFunctionName()) != null) {
- //If native function is handled by this index then ensure
- // that lowest cost if returned
- return defaultPlan().setEstimatedEntryCount(1);
+ return getNativeFunctionPlanBuilder(indexingRule.getBaseNodeType());
}
List<String> indexedProps = newArrayListWithCapacity(filter.getPropertyRestrictions().size());
@@ -227,6 +226,31 @@ class IndexPlanner {
return null;
}
+ private IndexPlan.Builder getNativeFunctionPlanBuilder(String indexingRuleBaseNodeType) {
+ boolean canHandleNativeFunction = true;
+
+ PropertyValue pv = filter.getPropertyRestriction(definition.getFunctionName()).first;
+ String query = pv.getValue(Type.STRING);
+
+ if (query.startsWith("suggest?term=")) {
+ if (definition.isSuggestEnabled()) {
+ canHandleNativeFunction = indexingRuleBaseNodeType.equals(filter.getNodeType());
+ } else {
+ canHandleNativeFunction = false;
+ }
+ } else if (query.startsWith("spellcheck?term=")) {
+ if (definition.isSpellcheckEnabled()) {
+ canHandleNativeFunction = indexingRuleBaseNodeType.equals(filter.getNodeType());
+ } else {
+ canHandleNativeFunction = false;
+ }
+ }
+
+ //If native function can be handled by this index then ensure
+ // that lowest cost if returned
+ return canHandleNativeFunction ? defaultPlan().setEstimatedEntryCount(1) : null;
+ }
+
/**
* Check if there is a mismatch between QueryPaths associated with index
* and path restriction specified in query
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlannerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlannerTest.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlannerTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlannerTest.java Thu Jan 7 14:56:36 2016
@@ -54,6 +54,7 @@ import static javax.jcr.PropertyType.TYP
import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM;
import static org.apache.jackrabbit.oak.api.Type.NAMES;
import static org.apache.jackrabbit.oak.api.Type.STRINGS;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.DECLARING_NODE_TYPES;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
import static org.apache.jackrabbit.oak.plugins.index.PathFilter.PROP_INCLUDED_PATHS;
import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.INDEX_DATA_CHILD_NAME;
@@ -459,6 +460,127 @@ public class IndexPlannerTest {
assertNull(plan);
}
+ //------ Suggestion/spellcheck plan tests
+ @Test
+ public void nonSuggestIndex() throws Exception {
+ //An index which doesn't define any property to support suggestions shouldn't turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:base";
+ boolean enableSuggestionIndex = false;
+ boolean enableSpellcheckIndex = false;
+ boolean queryForSugggestion = true;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNull(plan);
+ }
+
+ @Test
+ public void nonSpellcheckIndex() throws Exception {
+ //An index which doesn't define any property to support spell check shouldn't turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:base";
+ boolean enableSuggestionIndex = false;
+ boolean enableSpellcheckIndex = false;
+ boolean queryForSugggestion = false;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNull(plan);
+ }
+
+ @Test
+ public void simpleSuggestIndexPlan() throws Exception {
+ //An index defining a property for suggestions should turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:base";
+ boolean enableSuggestionIndex = true;
+ boolean enableSpellcheckIndex = false;
+ boolean queryForSugggestion = true;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNotNull(plan);
+ }
+
+ @Test
+ public void simpleSpellcheckIndexPlan() throws Exception {
+ //An index defining a property for spellcheck should turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:base";
+ boolean enableSuggestionIndex = false;
+ boolean enableSpellcheckIndex = true;
+ boolean queryForSugggestion = false;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNotNull(plan);
+ }
+
+ @Test
+ public void suggestionIndexingRuleHierarchy() throws Exception {
+ //An index defining a property for suggestion on a base type shouldn't turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:unstructured";
+ boolean enableSuggestionIndex = true;
+ boolean enableSpellcheckIndex = false;
+ boolean queryForSugggestion = true;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNull(plan);
+ }
+
+ @Test
+ public void spellcheckIndexingRuleHierarchy() throws Exception {
+ //An index defining a property for spellcheck on a base type shouldn't turn up in plan.
+ String indexNodeType = "nt:base";
+ String queryNodeType = "nt:unstructured";
+ boolean enableSuggestionIndex = false;
+ boolean enableSpellcheckIndex = true;
+ boolean queryForSugggestion = false;
+
+ IndexNode node = createSuggestionOrSpellcheckIndex(indexNodeType, enableSuggestionIndex, enableSpellcheckIndex);
+ QueryIndex.IndexPlan plan = getSuggestOrSpellcheckIndexPlan(node, queryNodeType, queryForSugggestion);
+
+ assertNull(plan);
+ }
+
+ private IndexNode createSuggestionOrSpellcheckIndex(String nodeType,
+ boolean enableSuggestion,
+ boolean enableSpellcheck) throws Exception {
+ NodeBuilder defn = newLucenePropertyIndexDefinition(builder, "test", of("foo"), "async");
+ defn.setProperty(DECLARING_NODE_TYPES, nodeType);
+
+ defn = IndexDefinition.updateDefinition(defn.getNodeState().builder());
+ NodeBuilder foob = getNode(defn, "indexRules/" + nodeType + "/properties/foo");
+ foob.setProperty(LuceneIndexConstants.PROP_ANALYZED, true);
+ if (enableSuggestion) {
+ foob.setProperty(LuceneIndexConstants.PROP_USE_IN_SUGGEST, true);
+ } if (enableSpellcheck) {
+ foob.setProperty(LuceneIndexConstants.PROP_USE_IN_SPELLCHECK, true);
+ }
+
+ IndexDefinition indexDefinition = new IndexDefinition(root, defn.getNodeState());
+ return createIndexNode(indexDefinition);
+ }
+
+ private QueryIndex.IndexPlan getSuggestOrSpellcheckIndexPlan(IndexNode indexNode, String nodeType,
+ boolean forSugggestion) throws Exception {
+ FilterImpl filter = createFilter(nodeType);
+ filter.restrictProperty(indexNode.getDefinition().getFunctionName(), Operator.EQUAL,
+ PropertyValues.newString((forSugggestion?"suggest":"spellcheck") + "?term=foo"));
+ IndexPlanner planner = new IndexPlanner(indexNode, "/foo", filter, Collections.<OrderEntry>emptyList());
+
+ return planner.getPlan();
+ }
+ //------ END - Suggestion/spellcheck plan tests
+
private IndexNode createIndexNode(IndexDefinition defn, long numOfDocs) throws IOException {
return new IndexNode("foo", defn, createSampleDirectory(numOfDocs), null);
}
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexSuggestionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexSuggestionTest.java?rev=1723565&r1=1723564&r2=1723565&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexSuggestionTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexSuggestionTest.java Thu Jan 7 14:56:36 2016
@@ -201,7 +201,7 @@ public class LuceneIndexSuggestionTest {
final String indexPropName = "description";
final String indexPropValue = "this is just a sample text which should get some response in suggestions";
final String suggestQueryText = "th";
- final boolean shouldSuggest = true;
+ final boolean shouldSuggest = false;
checkSuggestions(indexNodeType, queryNodeType,
indexPropName, indexPropValue,