You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2010/08/18 16:00:30 UTC
svn commit: r986686 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/query/lucene/
main/java/org/apache/jackrabbit/core/query/lucene/join/
test/java/org/apache/jackrabbit/core/query/
Author: jukka
Date: Wed Aug 18 14:00:30 2010
New Revision: 986686
URL: http://svn.apache.org/viewvc?rev=986686&view=rev
Log:
JCR-2718: Incorrect results from joins on multivalued properties
Use full term lookups in EquiJoin instead of relying on comparators for the equality tests.
Added:
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java (with props)
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/JoinQuery.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/LuceneQueryFactoryImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryObjectModelImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/EquiJoin.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/Join.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/TestAll.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/JoinQuery.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/JoinQuery.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/JoinQuery.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/JoinQuery.java Wed Aug 18 14:00:30 2010
@@ -23,7 +23,6 @@ import org.apache.jackrabbit.core.Hierar
import org.apache.jackrabbit.core.query.lucene.join.Join;
import org.apache.jackrabbit.spi.commons.query.qom.JoinConditionImpl;
import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.search.SortComparatorSource;
/**
* <code>JoinQuery</code> implements a query that performs a join.
@@ -51,9 +50,9 @@ public class JoinQuery implements MultiC
private final JoinConditionImpl joinCondition;
/**
- * The sort comparator source of the index.
+ * Namespace mappings of this index.
*/
- private final SortComparatorSource scs;
+ private final NamespaceMappings nsMappings;
/**
* The hierarchy manager of the workspace.
@@ -67,20 +66,20 @@ public class JoinQuery implements MultiC
* @param right the right side of the query.
* @param joinType the join type.
* @param joinCondition the join condition.
- * @param scs the sort comparator source of the index.
+ * @param nsMappings namespace mappings of this index
* @param hmgr the hierarchy manager of the workspace.
*/
public JoinQuery(MultiColumnQuery left,
MultiColumnQuery right,
JoinType joinType,
JoinConditionImpl joinCondition,
- SortComparatorSource scs,
+ NamespaceMappings nsMappings,
HierarchyManager hmgr) {
this.left = left;
this.right = right;
this.joinType = joinType;
this.joinCondition = joinCondition;
- this.scs = scs;
+ this.nsMappings = nsMappings;
this.hmgr = hmgr;
}
@@ -95,6 +94,6 @@ public class JoinQuery implements MultiC
HierarchyResolver resolver = (HierarchyResolver) reader;
return Join.create(left.execute(searcher, orderings, resultFetchHint),
right.execute(searcher, orderings, resultFetchHint),
- joinType, joinCondition, reader, resolver, scs, hmgr);
+ joinType, joinCondition, reader, resolver, nsMappings, hmgr);
}
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/LuceneQueryFactoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/LuceneQueryFactoryImpl.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/LuceneQueryFactoryImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/LuceneQueryFactoryImpl.java Wed Aug 18 14:00:30 2010
@@ -50,7 +50,6 @@ import org.apache.lucene.queryParser.Que
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Query;
-import org.apache.lucene.search.SortComparatorSource;
/**
* <code>LuceneQueryFactoryImpl</code> implements a lucene query factory.
@@ -63,11 +62,6 @@ public class LuceneQueryFactoryImpl impl
private final SessionImpl session;
/**
- * The source comparator source.
- */
- private final SortComparatorSource scs;
-
- /**
* The hierarchy manager.
*/
private final HierarchyManager hmgr;
@@ -115,7 +109,6 @@ public class LuceneQueryFactoryImpl impl
* @param bindVariables the bind variable values of the query
*/
public LuceneQueryFactoryImpl(SessionImpl session,
- SortComparatorSource scs,
HierarchyManager hmgr,
NamespaceMappings nsMappings,
Analyzer analyzer,
@@ -123,7 +116,6 @@ public class LuceneQueryFactoryImpl impl
IndexFormatVersion version,
Map<Name, Value> bindVariables) {
this.session = session;
- this.scs = scs;
this.hmgr = hmgr;
this.nsMappings = nsMappings;
this.analyzer = analyzer;
@@ -277,6 +269,6 @@ public class LuceneQueryFactoryImpl impl
MultiColumnQuery left = create((SourceImpl) join.getLeft());
MultiColumnQuery right = create((SourceImpl) join.getRight());
return new JoinQuery(left, right, join.getJoinTypeInstance(),
- (JoinConditionImpl) join.getJoinCondition(), scs, hmgr);
+ (JoinConditionImpl) join.getJoinCondition(), nsMappings, hmgr);
}
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryObjectModelImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryObjectModelImpl.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryObjectModelImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryObjectModelImpl.java Wed Aug 18 14:00:30 2010
@@ -95,8 +95,7 @@ public class QueryObjectModelImpl extend
throws RepositoryException {
SessionImpl session = sessionContext.getSessionImpl();
LuceneQueryFactory factory = new LuceneQueryFactoryImpl(
- session, index.getSortComparatorSource(),
- index.getContext().getHierarchyManager(),
+ session, index.getContext().getHierarchyManager(),
index.getNamespaceMappings(), index.getTextAnalyzer(),
index.getSynonymProvider(), index.getIndexFormatVersion(),
getBindVariableValues());
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/EquiJoin.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/EquiJoin.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/EquiJoin.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/EquiJoin.java Wed Aug 18 14:00:30 2010
@@ -16,15 +16,24 @@
*/
package org.apache.jackrabbit.core.query.lucene.join;
+import static org.apache.jackrabbit.core.query.lucene.FieldNames.PROPERTIES;
+
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.jackrabbit.core.query.lucene.FieldNames;
import org.apache.jackrabbit.core.query.lucene.MultiColumnQueryHits;
+import org.apache.jackrabbit.core.query.lucene.NamespaceMappings;
import org.apache.jackrabbit.core.query.lucene.ScoreNode;
import org.apache.jackrabbit.spi.Name;
+import org.apache.jackrabbit.spi.commons.conversion.IllegalNameException;
import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.search.SortComparatorSource;
-import org.apache.lucene.search.ScoreDocComparator;
-import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.index.TermDocs;
+import org.apache.lucene.index.TermEnum;
/**
* <code>EquiJoin</code> implements an equi join condition.
@@ -32,24 +41,14 @@ import org.apache.lucene.search.ScoreDoc
public class EquiJoin extends AbstractCondition {
/**
- * Reusable score doc for value lookups.
- */
- private final ScoreDoc sDoc = new ScoreDoc(-1, 1.0f);
-
- /**
* The index reader.
*/
private final IndexReader reader;
- /**
- * Map of inner score nodes indexed by the value of their join property.
- */
- private final ScoreNodeMap innerScoreNodes = new ScoreNodeMap();
+ private final Term outerTerm;
- /**
- * The score doc comparator for the outer query hits.
- */
- private final ScoreDocComparator outerLookup;
+ private final Map<String, List<ScoreNode[]>> rowsByInnerNodeValue =
+ new HashMap<String, List<ScoreNode[]>>();
/**
* Creates a new equi join condition.
@@ -63,26 +62,61 @@ public class EquiJoin extends AbstractCo
* @param outerProperty the name of the property of the outer query
* hits.
* @throws IOException if an error occurs while reading from the index.
+ * @throws IllegalNameException
*/
- public EquiJoin(MultiColumnQueryHits inner,
- int innerScoreNodeIndex,
- SortComparatorSource scs,
- IndexReader reader,
- Name innerProperty,
- Name outerProperty) throws IOException {
+ public EquiJoin(
+ MultiColumnQueryHits inner, int innerScoreNodeIndex,
+ NamespaceMappings nsMappings, IndexReader reader,
+ Name innerProperty, Name outerProperty)
+ throws IOException, IllegalNameException {
super(inner);
this.reader = reader;
- this.outerLookup = scs.newComparator(reader, outerProperty.toString());
- ScoreDocComparator comparator = scs.newComparator(reader, innerProperty.toString());
- ScoreNode[] nodes;
+
+ Term innerTerm = new Term(PROPERTIES, FieldNames.createNamedValue(
+ nsMappings.translateName(innerProperty), ""));
+ this.outerTerm = new Term(PROPERTIES, FieldNames.createNamedValue(
+ nsMappings.translateName(outerProperty), ""));
+
// create lookup map
- while ((nodes = inner.nextScoreNodes()) != null) {
- sDoc.doc = nodes[innerScoreNodeIndex].getDoc(reader);
- Comparable value = comparator.sortValue(sDoc);
- if (value != null) {
- innerScoreNodes.addScoreNodes(value, nodes);
+ Map<Integer, List<ScoreNode[]>> rowsByInnerDocument =
+ new HashMap<Integer, List<ScoreNode[]>>();
+ ScoreNode[] row = inner.nextScoreNodes();
+ while (row != null) {
+ int document = row[innerScoreNodeIndex].getDoc(reader);
+ List<ScoreNode[]> rows = rowsByInnerDocument.get(document);
+ if (rows == null) {
+ rows = new ArrayList<ScoreNode[]>();
+ rowsByInnerDocument.put(document, rows);
}
+ rows.add(row);
+ row = inner.nextScoreNodes();
}
+
+ // Build the rowsByInnerNodeValue map for efficient lookup in
+ // the getMatchingScoreNodes() method
+ TermEnum terms = reader.terms(innerTerm);
+ do {
+ Term term = terms.term();
+ if (term == null
+ || !term.field().equals(innerTerm.field())
+ || !term.text().startsWith(innerTerm.text())) {
+ break;
+ }
+
+ String value = term.text().substring(innerTerm.text().length());
+ TermDocs docs = reader.termDocs(terms.term());
+ while (docs.next()) {
+ List<ScoreNode[]> match = rowsByInnerDocument.get(docs.doc());
+ if (match != null) {
+ List<ScoreNode[]> rows = rowsByInnerNodeValue.get(value);
+ if (rows == null) {
+ rows = new ArrayList<ScoreNode[]>();
+ rowsByInnerNodeValue.put(value, rows);
+ }
+ rows.addAll(match);
+ }
+ }
+ } while (terms.next());
}
/**
@@ -90,8 +124,32 @@ public class EquiJoin extends AbstractCo
*/
public ScoreNode[][] getMatchingScoreNodes(ScoreNode outer)
throws IOException {
- sDoc.doc = outer.getDoc(reader);
- Comparable value = outerLookup.sortValue(sDoc);
- return innerScoreNodes.getScoreNodes(value);
+ List<ScoreNode[]> list = new ArrayList<ScoreNode[]>();
+
+ int document = outer.getDoc(reader);
+ TermEnum terms = reader.terms(outerTerm);
+ do {
+ Term term = terms.term();
+ if (term == null
+ || !term.field().equals(outerTerm.field())
+ || !term.text().startsWith(outerTerm.text())) {
+ break;
+ }
+
+ List<ScoreNode[]> rows = rowsByInnerNodeValue.get(
+ terms.term().text().substring(outerTerm.text().length()));
+ if (rows != null) {
+ TermDocs docs = reader.termDocs(terms.term());
+ while (docs.next()) {
+ if (docs.doc() == document) {
+ list.addAll(rows);
+ break;
+ }
+ }
+ }
+ } while (terms.next());
+
+ return list.toArray(new ScoreNode[list.size()][]);
}
+
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/Join.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/Join.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/Join.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/join/Join.java Wed Aug 18 14:00:30 2010
@@ -25,6 +25,7 @@ import org.apache.jackrabbit.commons.que
import org.apache.jackrabbit.core.HierarchyManager;
import org.apache.jackrabbit.core.query.lucene.HierarchyResolver;
import org.apache.jackrabbit.core.query.lucene.MultiColumnQueryHits;
+import org.apache.jackrabbit.core.query.lucene.NamespaceMappings;
import org.apache.jackrabbit.core.query.lucene.ScoreNode;
import org.apache.jackrabbit.spi.Name;
import org.apache.jackrabbit.spi.Path;
@@ -35,7 +36,6 @@ import org.apache.jackrabbit.spi.commons
import org.apache.jackrabbit.spi.commons.query.qom.JoinConditionImpl;
import org.apache.jackrabbit.spi.commons.query.qom.SameNodeJoinConditionImpl;
import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.search.SortComparatorSource;
/**
* <code>Join</code> implements the result of a join.
@@ -113,7 +113,7 @@ public class Join implements MultiColumn
* @param condition the QOM join condition.
* @param reader the index reader.
* @param resolver the hierarchy resolver.
- * @param scs the sort comparator source of the index.
+ * @param nsMappings namespace mappings of this index
* @param hmgr the hierarchy manager of the workspace.
* @return the join result.
* @throws IOException if an error occurs while executing the join.
@@ -124,7 +124,7 @@ public class Join implements MultiColumn
final JoinConditionImpl condition,
final IndexReader reader,
final HierarchyResolver resolver,
- final SortComparatorSource scs,
+ final NamespaceMappings nsMappings,
final HierarchyManager hmgr)
throws IOException {
try {
@@ -183,8 +183,9 @@ public class Join implements MultiColumn
outerPropName = node.getProperty2QName();
}
- Condition c = new EquiJoin(inner, getIndex(inner, innerName),
- scs, reader, innerPropName, outerPropName);
+ Condition c = new EquiJoin(
+ inner, getIndex(inner, innerName), nsMappings,
+ reader, innerPropName, outerPropName);
return new Join(outer, outerIdx, isInner, c);
}
Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java?rev=986686&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java Wed Aug 18 14:00:30 2010
@@ -0,0 +1,71 @@
+/*
+ * 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.core.query;
+
+import javax.jcr.Node;
+import javax.jcr.PropertyType;
+import javax.jcr.nodetype.NodeType;
+import javax.jcr.query.Query;
+import javax.jcr.query.QueryResult;
+
+/**
+ * Test case for
+ * <a href="https://issues.apache.org/jira/browse/JCR-2718">JCR-2718</a>
+ */
+public class JoinTest extends AbstractQueryTest {
+
+ private Node node;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ node = testRootNode.addNode("jointest");
+
+ Node n1 = node.addNode("node1");
+ n1.addMixin(NodeType.MIX_REFERENCEABLE);
+ testRootNode.getSession().save();
+
+ Node n2 = node.addNode("node2");
+ n2.addMixin(NodeType.MIX_REFERENCEABLE);
+ testRootNode.getSession().save();
+
+ Node n3 = node.addNode("node3");
+ n3.addMixin(NodeType.MIX_REFERENCEABLE);
+ n3.setProperty(
+ "testref",
+ new String[] { n1.getIdentifier(), n2.getIdentifier() },
+ PropertyType.REFERENCE);
+ testRootNode.getSession().save();
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ node.remove();
+ testRootNode.getSession().save();
+ super.tearDown();
+ }
+
+ public void testMultiValuedReferenceJoin() throws Exception {
+ String join =
+ "SELECT a.*, b.*"
+ + " FROM [nt:base] AS a"
+ + " INNER JOIN [nt:base] AS b ON a.[jcr:uuid] = b.testref";
+ QueryResult result = qm.createQuery(join, Query.JCR_SQL2).execute();
+ checkResult(result, 2);
+ }
+
+}
Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/JoinTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/TestAll.java?rev=986686&r1=986685&r2=986686&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/TestAll.java Wed Aug 18 14:00:30 2010
@@ -42,6 +42,7 @@ public class TestAll extends TestCase {
suite.addTestSuite(FulltextQueryTest.class);
suite.addTestSuite(SelectClauseTest.class);
suite.addTestSuite(SQLTest.class);
+ suite.addTestSuite(JoinTest.class);
suite.addTestSuite(OrderByTest.class);
suite.addTestSuite(XPathAxisTest.class);
suite.addTestSuite(SkipDeletedNodesTest.class);