Index: src/test/java/org/apache/jackrabbit/core/query/SQL2OuterJoinTest.java
===================================================================
--- src/test/java/org/apache/jackrabbit/core/query/SQL2OuterJoinTest.java (revision 0)
+++ src/test/java/org/apache/jackrabbit/core/query/SQL2OuterJoinTest.java (revision 0)
@@ -0,0 +1,151 @@
+/*
+ * 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 static javax.jcr.query.Query.JCR_SQL2;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+
+import javax.jcr.Node;
+import javax.jcr.nodetype.NodeTypeManager;
+
+import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.jackrabbit.commons.cnd.CndImporter;
+
+/**
+ * Test case for OUTER JOIN queries with JCR_SQL2
+ *
+ * Inspired by JCR-2933
+ *
+ */
+public class SQL2OuterJoinTest extends AbstractQueryTest {
+
+ private Node n2;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+
+ NodeTypeManager manager = superuser.getWorkspace().getNodeTypeManager();
+ if (!manager.hasNodeType("test:SamplePage")) {
+ StringBuilder defs = new StringBuilder();
+ defs.append("[test:SamplePage]\n");
+ defs.append(" - n1prop1\n");
+ defs.append(" + node2\n");
+ defs.append("[test:SampleContent]\n");
+ defs.append(" - n2prop1");
+ Reader cndReader = new InputStreamReader(new ByteArrayInputStream(
+ defs.toString().getBytes()));
+ CndImporter.registerNodeTypes(cndReader, superuser);
+ }
+
+ Node n1 = testRootNode.addNode("node1", "test:SamplePage");
+ n1.setProperty("n1prop1", "page1");
+
+ n2 = n1.addNode("node2", "test:SampleContent");
+ n2.setProperty("n2prop1", "content1");
+
+ testRootNode.getSession().save();
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ for (Node c : JcrUtils.getChildNodes(testRootNode)) {
+ testRootNode.getSession().removeItem(c.getPath());
+ }
+ testRootNode.getSession().save();
+ super.tearDown();
+ }
+
+ public void testOuterJoin() throws Exception {
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page)");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 1);
+ }
+
+ /**
+ * Test case for JCR-2933
+ *
+ * Outer Join that works OOTB
+ */
+ public void testOuterJoinWithCondition() throws Exception {
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page) where page.n1prop1 = 'page1' and content.n2prop1 = 'content1' ");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 1);
+ }
+
+ /**
+ * Test case for JCR-2933
+ *
+ * Outer Join that does not work on missing where clause, hence the jira
+ * issue
+ */
+ public void testOuterJoinMissingProperty() throws Exception {
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page) where page.n1prop1 = 'page1' and content.n2prop1 = 'XXX' ");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 0);
+ }
+
+ /**
+ * Test case for JCR-2933
+ *
+ * Outer Join that does not work on missing child node, no WHERE condition
+ */
+ public void testOuterJoinMissingNode() throws Exception {
+ testRootNode.getSession().removeItem(n2.getPath());
+ testRootNode.getSession().save();
+
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page)");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 1);
+ }
+
+ public void testOuterJoinMissingNodeWithCondition() throws Exception {
+ testRootNode.getSession().removeItem(n2.getPath());
+ testRootNode.getSession().save();
+
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page) where page.n1prop1 = 'page1' and content.n2prop1 = 'XXX' ");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 0);
+ }
+
+ public void testOuterJoinExtraNode() throws Exception {
+ Node n3 = testRootNode.addNode("node3", "test:SamplePage");
+ n3.setProperty("n1prop1", "page1");
+ testRootNode.getSession().save();
+
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page)");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 2);
+ }
+
+ public void testOuterJoinExtraNodeWithCondition() throws Exception {
+ Node n3 = testRootNode.addNode("node3", "test:SamplePage");
+ n3.setProperty("n1prop1", "page1");
+ testRootNode.getSession().save();
+
+ StringBuilder join = new StringBuilder();
+ join.append(" Select * from [test:SamplePage] as page left outer join [test:SampleContent] as content on ISDESCENDANTNODE(content,page) where page.n1prop1 = 'page1' and content.n2prop1 = 'XXX' ");
+ checkResult(qm.createQuery(join.toString(), JCR_SQL2).execute(), 0);
+ }
+}
Index: src/main/java/org/apache/jackrabbit/core/query/lucene/join/QueryEngine.java
===================================================================
--- src/main/java/org/apache/jackrabbit/core/query/lucene/join/QueryEngine.java (revision 1086159)
+++ src/main/java/org/apache/jackrabbit/core/query/lucene/join/QueryEngine.java (working copy)
@@ -132,49 +132,78 @@
this.evaluator = new OperandEvaluator(valueFactory, variables);
}
- public QueryResult execute(
- Column[] columns, Source source, Constraint constraint,
- Ordering[] orderings, long offset, long limit)
+ public QueryResult execute(Column[] columns, Source source,
+ Constraint constraint, Ordering[] orderings, long offset, long limit)
throws RepositoryException {
if (source instanceof Selector) {
- Selector selector = (Selector) source;
- return execute(
- columns, selector, constraint, orderings, offset, limit);
- } else if (source instanceof Join) {
- Join join = (Join) source;
- if (join.getJoinType() == JCR_JOIN_TYPE_RIGHT_OUTER) {
- // Swap the join sources to normalize all outer joins to left
- join = qomFactory.join(
- join.getRight(), join.getLeft(),
- JCR_JOIN_TYPE_LEFT_OUTER, join.getJoinCondition());
- }
- return execute(
- columns, join, constraint, orderings, offset, limit);
- } else {
- throw new UnsupportedRepositoryOperationException(
- "Unknown source type: " + source);
+ return execute(columns, (Selector) source, constraint, orderings,
+ offset, limit);
+ }
+ if (source instanceof Join) {
+ return execute(columns, (Join) source, constraint, orderings,
+ offset, limit);
}
+ throw new UnsupportedRepositoryOperationException(
+ "Unknown source type: " + source);
}
- protected QueryResult execute(
- Column[] columns, Join join, Constraint constraint,
- Ordering[] orderings, long offset, long limit)
+ protected QueryResult execute(Column[] columns, Join join,
+ Constraint constraint, Ordering[] orderings, long offset, long limit)
throws RepositoryException {
- JoinMerger merger = JoinMerger.getJoinMerger(
- join, getColumnMap(columns, getSelectorNames(join)),
- evaluator, qomFactory);
- ConstraintSplitter splitter = new ConstraintSplitter(
- constraint, qomFactory,
- merger.getLeftSelectors(), merger.getRightSelectors());
+ // Swap the join sources to normalize all outer joins to left
+ if (JCR_JOIN_TYPE_RIGHT_OUTER.equalsIgnoreCase(join.getJoinType())) {
+ Join betterJoin = qomFactory.join(join.getRight(), join.getLeft(),
+ JCR_JOIN_TYPE_LEFT_OUTER, join.getJoinCondition());
+ return execute(columns, betterJoin, constraint, orderings, offset,
+ limit);
+ }
+ JoinMerger merger = JoinMerger.getJoinMerger(join,
+ getColumnMap(columns, getSelectorNames(join)), evaluator,
+ qomFactory);
+ ConstraintSplitter splitter = new ConstraintSplitter(constraint,
+ qomFactory, merger.getLeftSelectors(),
+ merger.getRightSelectors());
+ ConstraintSplitInfo csInfo = splitter.getConstraintSplitInfo();
Source left = join.getLeft();
- Set leftRows = buildLeftRowsJoin(left, splitter.getConstraintSplitInfo());
+ Set leftRows = buildLeftRowsJoin(left, csInfo);
+
+ // The join constraint information is split into:
+ // - rightConstraints selects just the 'ON' constraints
+ // - csInfo has the 'WHERE' constraints
+ //
+ // So, in the case of an OUTER JOIN we'll run 2 queries, one with 'ON'
+ // and one with 'ON' + 'WHERE' conditions
+ // this way, at merge time in case of an outer join we can tell if
+ // it's a 'null' row, or a bad row -> one that must not be returned.
+ // This way at the end we'll have:
+ // - rightRowsSet containing the 'ON' dataset
+ // - excludingOuterJoinRowsSet: the 'ON' + 'WHERE' condition dataset, or
+ // NULL if there is no 'WHERE' condition
Source right = join.getRight();
- List rightConstraints = merger.getRightJoinConstraints(leftRows);
- RowIterator rightRows = new RowIteratorAdapter(buildRightRowsJoin(right, splitter.getConstraintSplitInfo(), rightConstraints));
+ List rightConstraints = merger
+ .getRightJoinConstraints(leftRows);
+
+ boolean isOuterJoin = JCR_JOIN_TYPE_LEFT_OUTER.equalsIgnoreCase(join
+ .getJoinType());
+
+ Set rightRows = buildRightRowsJoin(right, csInfo,
+ rightConstraints, true);
+
+ // this *has* to be initialized as null
+ Set excludingOuterJoinRowsSet = null;
+ if (isOuterJoin && csInfo.getRightConstraint() != null) {
+ excludingOuterJoinRowsSet = new HashSet(buildRightRowsJoin(
+ right, csInfo, rightConstraints, false));
+ }
+ System.out.println("LEFT: " + leftRows);
+ System.out.println("RIGHT: " + rightRows);
+ System.out.println("EXCLUDING " + excludingOuterJoinRowsSet);
- QueryResult result = merger.merge(new RowIteratorAdapter(leftRows), rightRows);
+ // merge left with right datasets
+ QueryResult result = merger.merge(new RowIteratorAdapter(leftRows),
+ new RowIteratorAdapter(rightRows), excludingOuterJoinRowsSet);
return sort(result, orderings, offset, limit);
}
@@ -214,8 +243,20 @@
return leftRows;
}
+ /**
+ * @param right
+ * contains the fields to be selected for right-side dataset of
+ * the join operation
+ * @param csi
+ * contains 'WHERE' constraints.
+ * @param rightConstraints
+ * contains 'ON' constraints
+ * @return the right-side dataset of the join operation
+ * @throws RepositoryException
+ */
private Set buildRightRowsJoin(Source right, ConstraintSplitInfo csi,
- List rightConstraints) throws RepositoryException {
+ List rightConstraints, boolean ignoreWhereConstraints)
+ throws RepositoryException {
if (csi.isMultiple()) {
// this *needs* to merge automatically multiple sets of nodes
@@ -223,7 +264,7 @@
buildSimplePathRowComparator());
for (ConstraintSplitInfo child : csi.getInnerConstraints()) {
rightRows.addAll(buildRightRowsJoin(right, child,
- rightConstraints));
+ rightConstraints, ignoreWhereConstraints));
}
return rightRows;
}
@@ -234,6 +275,10 @@
Constraint rightConstraint = Constraints.and(qomFactory,
Constraints.or(qomFactory, rightConstraints),
csi.getRightConstraint());
+ if (ignoreWhereConstraints) {
+ rightConstraint = Constraints.or(qomFactory, rightConstraints);
+ }
+
QueryResult rightResult = execute(null, right, rightConstraint,
null, 0, -1);
for (Row row : JcrUtils.getRows(rightResult)) {
@@ -253,6 +298,12 @@
Math.min(i + 500,
rightConstraints.size()))),
csi.getRightConstraint());
+ if (ignoreWhereConstraints) {
+ rightConstraint = Constraints.or(
+ qomFactory,
+ rightConstraints.subList(i,
+ Math.min(i + 500, rightConstraints.size())));
+ }
QueryResult rightResult = execute(null, right, rightConstraint,
null, 0, -1);
for (Row row : JcrUtils.getRows(rightResult)) {
Index: src/main/java/org/apache/jackrabbit/core/query/lucene/join/JoinMerger.java
===================================================================
--- src/main/java/org/apache/jackrabbit/core/query/lucene/join/JoinMerger.java (revision 1087224)
+++ src/main/java/org/apache/jackrabbit/core/query/lucene/join/JoinMerger.java (working copy)
@@ -17,6 +17,7 @@
package org.apache.jackrabbit.core.query.lucene.join;
import static javax.jcr.query.qom.QueryObjectModelConstants.JCR_JOIN_TYPE_LEFT_OUTER;
+import static javax.jcr.query.qom.QueryObjectModelConstants.JCR_JOIN_TYPE_INNER;
import java.util.ArrayList;
import java.util.Collection;
@@ -161,50 +162,124 @@
}
}
- public QueryResult merge(RowIterator leftRows, RowIterator rightRows)
- throws RepositoryException {
- RowIterator joinRows;
+ /**
+ * Merges the left and right dataset of a join query. Take special
+ * precaution for outer joins, as extra checks are needed to distinguish
+ * 'null' nodes vs 'not to be included' nodes
+ *
+ *
+ * @param leftRows
+ * the left dataset of the join
+ * @param rightRows
+ * the right dataset of the join
+ * @param excludingOuterJoinRowsSet
+ * if not null must be taken into consideration when
+ * merging OUTER JOINs
+ * @return a QueryResult that has the final JOIN resultset
+ * @throws RepositoryException
+ */
+ public QueryResult merge(RowIterator leftRows, RowIterator rightRows,
+ Set excludingOuterJoinRowsSet) throws RepositoryException {
+ Map> map = buildRightRowValues(rightRows);
- Map> map = new HashMap>();
- for (Row row : new RowIterable(rightRows)) {
- for (String value : getRightValues(row)) {
- List rows = map.get(value);
- if (rows == null) {
- rows = new ArrayList();
- map.put(value, rows);
+ if (JCR_JOIN_TYPE_INNER.equals(type) && !map.isEmpty()) {
+ List rows = new ArrayList();
+ for (Row leftRow : new RowIterable(leftRows)) {
+ for (String value : getLeftValues(leftRow)) {
+ List matchingRows = map.get(value);
+ if (matchingRows != null) {
+ for (Row rightRow : matchingRows) {
+ rows.add(mergeRow(leftRow, rightRow));
+ }
+ }
}
- rows.add(row);
}
+ return asQueryResult(new RowIteratorAdapter(rows));
}
- if (!map.isEmpty()) {
+ if (JCR_JOIN_TYPE_LEFT_OUTER.equals(type)) {
+ // there are no RIGHT dataset values
+ if (map.isEmpty()) {
+ // if there are no WHERE conditions, just return everything
+ // else return an empty set
+ if (excludingOuterJoinRowsSet == null) {
+ return asQueryResult(new RowIteratorAdapter(leftRows) {
+ @Override
+ public Object next() {
+ return mergeRow((Row) super.next(), null);
+ }
+ });
+ }
+ return asQueryResult(new RowIteratorAdapter(
+ Collections.emptySet()));
+ }
+
List rows = new ArrayList();
for (Row leftRow : new RowIterable(leftRows)) {
for (String value : getLeftValues(leftRow)) {
List matchingRows = map.get(value);
if (matchingRows != null) {
for (Row rightRow : matchingRows) {
- rows.add(mergeRow(leftRow, rightRow));
+ // I have possible WHERE clauses on the join that I
+ // need to look at for each rightRow
+ if (excludingOuterJoinRowsSet == null) {
+ rows.add(mergeRow(leftRow, rightRow));
+ } else {
+ boolean isIncluded = false;
+ // apparently
+ // 'excludingOuterJoinRowsSet.contains' fails to
+ // match rows
+
+ // TODO can 'rightRow.getNode()' break because
+ // of joins that are bigger than 2 way?
+ // how does this perform for 3 way joins ?
+ for (Row r : excludingOuterJoinRowsSet) {
+ if (r.getNode()
+ .getIdentifier()
+ .equalsIgnoreCase(
+ rightRow.getNode()
+ .getIdentifier())) {
+ isIncluded = true;
+ break;
+ }
+ }
+ if (isIncluded) {
+ rows.add(mergeRow(leftRow, rightRow));
+ }
+ }
}
- } else if (JCR_JOIN_TYPE_LEFT_OUTER.equals(type)) {
- // No matches in an outer join -> add a null row
- rows.add(mergeRow(leftRow, null));
+ } else {
+ // No matches in an outer join -> add a null row, if
+ // there are no 'WHERE' conditions
+ if (excludingOuterJoinRowsSet == null) {
+ rows.add(mergeRow(leftRow, null));
+ }
}
}
}
- joinRows = new RowIteratorAdapter(rows);
- } else if (JCR_JOIN_TYPE_LEFT_OUTER.equals(type)) {
- joinRows = new RowIteratorAdapter(leftRows) {
- @Override
- public Object next() {
- return mergeRow((Row) super.next(), null);
- }
- };
- } else {
- joinRows = new RowIteratorAdapter(Collections.emptySet());
+ return asQueryResult(new RowIteratorAdapter(rows));
}
+ return asQueryResult(new RowIteratorAdapter(Collections.emptySet()));
+ }
- return new SimpleQueryResult(columnNames, selectorNames, joinRows);
+ private QueryResult asQueryResult(RowIterator rowIterator) {
+ return new SimpleQueryResult(columnNames, selectorNames, rowIterator);
+ }
+
+ private Map> buildRightRowValues(RowIterator rightRows)
+ throws RepositoryException {
+ Map> map = new HashMap>();
+ for (Row row : new RowIterable(rightRows)) {
+ for (String value : getRightValues(row)) {
+ List rows = map.get(value);
+ if (rows == null) {
+ rows = new ArrayList();
+ map.put(value, rows);
+ }
+ rows.add(row);
+ }
+ }
+ return map;
}
/**