jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject 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/
Date Wed, 18 Aug 2010 14:00:30 GMT
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);



Mime
View raw message