Index: src/test/java/org/apache/jackrabbit/core/query/OrderByTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/core/query/OrderByTest.java (revision 1205336) +++ src/test/java/org/apache/jackrabbit/core/query/OrderByTest.java (working copy) @@ -16,19 +16,20 @@ */ package org.apache.jackrabbit.core.query; -import javax.jcr.Node; -import javax.jcr.NodeIterator; -import javax.jcr.PropertyType; -import javax.jcr.RepositoryException; -import javax.jcr.Value; -import javax.jcr.query.Query; -import javax.jcr.query.QueryResult; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; import java.util.Collections; import java.util.List; +import javax.jcr.Node; +import javax.jcr.PropertyType; +import javax.jcr.RepositoryException; +import javax.jcr.Value; +import javax.jcr.query.QueryResult; + +import org.apache.jackrabbit.commons.JcrUtils; + /** * Tests queries with order by. */ @@ -46,18 +47,40 @@ n3.setProperty("text", "ccc"); n3.setProperty("value", 2); - testRootNode.save(); + testRootNode.getSession().save(); String sql = "SELECT value FROM nt:unstructured WHERE " + "jcr:path LIKE '" + testRoot + "/%' ORDER BY jcr:score, value"; - Query q = session.getWorkspace().getQueryManager().createQuery(sql, Query.SQL); - QueryResult result = q.execute(); - checkResult(result, 3); + checkResult(executeQuery(sql), 3); String xpath = "/" + testRoot + "/*[@jcr:primaryType='nt:unstructured'] order by jcr:score(), @value"; - q = session.getWorkspace().getQueryManager().createQuery(xpath, Query.XPATH); - result = q.execute(); - checkResult(result, 3); + checkResult(executeQuery(xpath), 3); + } + + /** + * Test for JCR-2906 + */ + public void testOrderByMVP() throws RepositoryException { + Node n1 = testRootNode.addNode("node1"); + Node n2 = testRootNode.addNode("node2"); + Node n3 = testRootNode.addNode("node3"); + Node n4 = testRootNode.addNode("node4"); + Node n5 = testRootNode.addNode("node5"); + + n1.setProperty("text", new String[] { "ccc" }); + n2.setProperty("text", new String[] { "eee", "bbb" }); + n3.setProperty("text", new String[] { "aaa" }); + n4.setProperty("text", new String[] { "bbb", "aaa" }); + n5.setProperty("text", new String[] { "eee", "aaa" }); + + testRootNode.getSession().save(); + + String sql = "SELECT value FROM nt:unstructured WHERE " + + "jcr:path LIKE '" + testRoot + "/%' ORDER BY text"; + checkResultSequence(executeQuery(sql).getRows(), new Node[] { n3, n4, n1, n5, n2 }); + + String xpath = "/" + testRoot + "/*[@jcr:primaryType='nt:unstructured'] order by jcr:score(), @text"; + checkResultSequence(executeQuery(xpath).getRows(), new Node[] { n3, n4, n1, n5, n2 }); } public void testOrderByUpperCase() throws RepositoryException { @@ -69,12 +92,10 @@ n2.setProperty("text", "barents"); n3.setProperty("text", "Wegener"); - testRootNode.save(); + testRootNode.getSession().save(); String xpath = "/" + testRoot + "/*[@jcr:primaryType='nt:unstructured'] order by fn:upper-case(@text)"; - Query q = session.getWorkspace().getQueryManager().createQuery(xpath, Query.XPATH); - QueryResult result = q.execute(); - checkResult(result, new Node[]{n1, n2, n3}); + checkResultSequence(executeQuery(xpath).getRows(), new Node[] { n1, n2, n3 }); } public void testOrderByLowerCase() throws RepositoryException { @@ -86,28 +107,26 @@ n2.setProperty("text", "barents"); n3.setProperty("text", "Wegener"); - testRootNode.save(); + testRootNode.getSession().save(); String xpath = "/" + testRoot + "/*[@jcr:primaryType='nt:unstructured'] order by fn:lower-case(@text)"; - Query q = session.getWorkspace().getQueryManager().createQuery(xpath, Query.XPATH); - QueryResult result = q.execute(); - checkResult(result, new Node[]{n1, n2, n3}); + checkResultSequence(executeQuery(xpath).getRows(), new Node[] { n1, n2, n3 }); } public void testChildAxisString() throws RepositoryException { - checkChildAxis(new Value[]{getValue("a"), getValue("b"), getValue("c")}); + checkChildAxis(new Value[] { getValue("a"), getValue("b"), getValue("c") }); } public void testChildAxisLong() throws RepositoryException { - checkChildAxis(new Value[]{getValue(1), getValue(2), getValue(3)}); + checkChildAxis(new Value[] { getValue(1), getValue(2), getValue(3) }); } public void testChildAxisDouble() throws RepositoryException { - checkChildAxis(new Value[]{getValue(1.0), getValue(2.0), getValue(3.0)}); + checkChildAxis(new Value[] { getValue(1.0), getValue(2.0), getValue(3.0) }); } public void testChildAxisBoolean() throws RepositoryException { - checkChildAxis(new Value[]{getValue(false), getValue(true)}); + checkChildAxis(new Value[] { getValue(false), getValue(true) }); } public void testChildAxisCalendar() throws RepositoryException { @@ -116,15 +135,15 @@ c2.add(Calendar.MINUTE, 1); Calendar c3 = Calendar.getInstance(); c3.add(Calendar.MINUTE, 2); - checkChildAxis(new Value[]{getValue(c1), getValue(c2), getValue(c3)}); + checkChildAxis(new Value[] { getValue(c1), getValue(c2), getValue(c3) }); } public void testChildAxisName() throws RepositoryException { - checkChildAxis(new Value[]{getNameValue("a"), getNameValue("b"), getNameValue("c")}); + checkChildAxis(new Value[] { getNameValue("a"), getNameValue("b"), getNameValue("c") }); } public void testChildAxisPath() throws RepositoryException { - checkChildAxis(new Value[]{getPathValue("a"), getPathValue("b"), getPathValue("c")}); + checkChildAxis(new Value[] { getPathValue("a"), getPathValue("b"), getPathValue("c") }); } public void testChildAxisDeep() throws RepositoryException { @@ -134,9 +153,9 @@ n2.addNode("a").addNode("b").addNode("c").setProperty("prop", "a"); Node n3 = testRootNode.addNode("node2"); n3.addNode("a").addNode("b").addNode("c").setProperty("prop", "b"); - testRootNode.save(); + testRootNode.getSession().save(); - List expected = Arrays.asList(new String[]{n1.getPath(), n2.getPath(), n3.getPath()}); + List expected = Arrays.asList(new String[] { n1.getPath(), n2.getPath(), n3.getPath() }); String xpath = testPath + "/* order by a/b/c/@prop"; assertEquals(expected, collectPaths(executeQuery(xpath))); @@ -151,9 +170,9 @@ n1.addNode("child").setProperty("prop", "a"); Node n2 = testRootNode.addNode("node2"); n2.addNode("child"); - testRootNode.save(); + testRootNode.getSession().save(); - List expected = Arrays.asList(new String[]{n2.getPath(), n1.getPath()}); + List expected = Arrays.asList(new String[] { n2.getPath(), n1.getPath() }); String xpath = testPath + "/* order by child/@prop"; assertEquals(expected, collectPaths(executeQuery(xpath))); @@ -165,7 +184,7 @@ // reverse order in content n1.getNode("child").getProperty("prop").remove(); n2.getNode("child").setProperty("prop", "a"); - testRootNode.save(); + testRootNode.getSession().save(); Collections.reverse(expected); assertEquals(expected, collectPaths(executeQuery(xpath))); @@ -175,7 +194,7 @@ // when differing types are used then the class name of the type // is used for comparison: // java.lang.Double < java.lang.Integer - checkChildAxis(new Value[]{getValue(2.0), getValue(1)}); + checkChildAxis(new Value[] { getValue(2.0), getValue(1) }); } //------------------------------< helper >---------------------------------- @@ -236,22 +255,22 @@ */ private void checkChildAxis(Value[] values, String child, String property) throws RepositoryException { - List vals = new ArrayList(); + List vals = new ArrayList(); // add initial value null -> property not set // inexistent property is always less than any property value set vals.add(null); vals.addAll(Arrays.asList(values)); - List expected = new ArrayList(); + List expected = new ArrayList(); for (int i = 0; i < vals.size(); i++) { Node n = testRootNode.addNode("node" + i); expected.add(n.getPath()); Node c = n.addNode(child); if (vals.get(i) != null) { - c.setProperty(property, (Value) vals.get(i)); + c.setProperty(property, vals.get(i)); } } - testRootNode.save(); + testRootNode.getSession().save(); String xpath = testPath + "/* order by " + child + "/@" + property; assertEquals(expected, collectPaths(executeQuery(xpath))); @@ -266,17 +285,17 @@ Node c = testRootNode.getNode("node" + i).getNode(child); c.setProperty(property, (Value) vals.get(i)); } - testRootNode.save(); + testRootNode.getSession().save(); Collections.reverse(expected); assertEquals(expected, collectPaths(executeQuery(xpath))); } - private static List collectPaths(QueryResult result) + private static List collectPaths(QueryResult result) throws RepositoryException { - List paths = new ArrayList(); - for (NodeIterator it = result.getNodes(); it.hasNext(); ) { - paths.add(it.nextNode().getPath()); + List paths = new ArrayList(); + for (Node n : JcrUtils.getNodes(result)) { + paths.add(n.getPath()); } return paths; } Index: src/test/java/org/apache/jackrabbit/core/query/SQL2OrderByTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/core/query/SQL2OrderByTest.java (revision 1205336) +++ src/test/java/org/apache/jackrabbit/core/query/SQL2OrderByTest.java (working copy) @@ -73,6 +73,30 @@ } + /** + * SQL2 Test for JCR-2906 + */ + public void testOrderByMVP() throws RepositoryException { + Node n1 = testRootNode.addNode("node1"); + Node n2 = testRootNode.addNode("node2"); + Node n3 = testRootNode.addNode("node3"); + Node n4 = testRootNode.addNode("node4"); + Node n5 = testRootNode.addNode("node5"); + + n1.setProperty("text", new String[] { "ccc" }); + n2.setProperty("text", new String[] { "eee", "bbb" }); + n3.setProperty("text", new String[] { "aaa" }); + n4.setProperty("text", new String[] { "bbb", "aaa" }); + n5.setProperty("text", new String[] { "eee", "aaa" }); + + testRootNode.getSession().save(); + + String sql = "SELECT value FROM [nt:unstructured] WHERE ISCHILDNODE([" + + testRoot + "]) ORDER BY text"; + + checkSeq(executeSQL2Query(sql), new Node[] { n3, n4, n1, n5, n2 }); + } + public void testOrderByVal() throws RepositoryException { Node n1 = testRootNode.addNode("node1"); Index: src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorDecorator.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorDecorator.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorDecorator.java (working copy) @@ -46,17 +46,17 @@ } @Override - protected Comparable sortValue(int doc) { + protected Comparable sortValue(int doc) { return base.sortValue(doc); } @Override - protected Comparable getValue(int slot) { + protected Comparable getValue(int slot) { return base.getValue(slot); } @Override - protected void setValue(int slot, Comparable value) { + protected void setValue(int slot, Comparable value) { base.setValue(slot, value); } } Index: src/main/java/org/apache/jackrabbit/core/query/lucene/sort/RowComparator.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/sort/RowComparator.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/sort/RowComparator.java (working copy) @@ -34,7 +34,7 @@ */ public class RowComparator implements Comparator { - private final ValueComparator comparator = new ValueComparator(); + private final static ValueComparator comparator = new ValueComparator(); private final Ordering[] orderings; @@ -51,7 +51,7 @@ Operand operand = ordering.getOperand(); Value[] va = evaluator.getValues(operand, a); Value[] vb = evaluator.getValues(operand, b); - int d = compare(va, vb); + int d = comparator.compare(va, vb); if (d != 0) { if (JCR_ORDER_DESCENDING.equals(ordering.getOrder())) { return -d; @@ -66,15 +66,4 @@ + b, e); } } - - private int compare(Value[] a, Value[] b) { - for (int i = 0; i < a.length && i < b.length; i++) { - int d = comparator.compare(a[i], b[i]); - if (d != 0) { - return d; - } - } - return a.length - b.length; - } - } \ No newline at end of file Index: src/main/java/org/apache/jackrabbit/core/query/lucene/sort/ValueComparableWrapper.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/sort/ValueComparableWrapper.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/sort/ValueComparableWrapper.java (working copy) @@ -21,6 +21,7 @@ import org.apache.jackrabbit.core.query.lucene.join.ValueComparator; class ValueComparableWrapper implements Comparable { + private final static ValueComparator comparator = new ValueComparator(); private final Value[] v; @@ -30,16 +31,6 @@ } public int compareTo(ValueComparableWrapper o) { - return compare(v, o.v); - } - - private int compare(Value[] a, Value[] b) { - for (int i = 0; i < a.length && i < b.length; i++) { - int d = comparator.compare(a[i], b[i]); - if (d != 0) { - return d; - } - } - return a.length - b.length; + return comparator.compare(v, o.v); } } Index: src/main/java/org/apache/jackrabbit/core/query/lucene/LowerCaseSortComparator.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/LowerCaseSortComparator.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/LowerCaseSortComparator.java (working copy) @@ -27,6 +27,7 @@ */ public class LowerCaseSortComparator extends FieldComparatorSource { + private static final long serialVersionUID = 6223379665982155209L; /** * The base comparator. */ @@ -48,8 +49,8 @@ return new FieldComparatorDecorator((FieldComparatorBase) comparator) { @Override - protected Comparable sortValue(int doc) { - Comparable c = super.sortValue(doc); + protected Comparable sortValue(int doc) { + Comparable c = super.sortValue(doc); return c == null ? null : c.toString().toLowerCase(); } }; Index: src/main/java/org/apache/jackrabbit/core/query/lucene/UpperCaseSortComparator.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/UpperCaseSortComparator.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/UpperCaseSortComparator.java (working copy) @@ -27,6 +27,8 @@ */ public class UpperCaseSortComparator extends FieldComparatorSource { + private static final long serialVersionUID = -8589227502290863712L; + /** * The base sort comparator. */ @@ -48,8 +50,8 @@ return new FieldComparatorDecorator((FieldComparatorBase) comparator) { @Override - protected Comparable sortValue(int doc) { - Comparable c = super.sortValue(doc); + protected Comparable sortValue(int doc) { + Comparable c = super.sortValue(doc); return c == null ? null : c.toString().toUpperCase(); } }; Index: src/main/java/org/apache/jackrabbit/core/query/lucene/join/ValueComparator.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/join/ValueComparator.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/join/ValueComparator.java (working copy) @@ -42,6 +42,19 @@ public class ValueComparator implements Comparator { /** + * Compares two arrays of values. + */ + public int compare(Value[] a, Value[] b) { + for (int i = 0; i < a.length && i < b.length; i++) { + int d = compare(a[i], b[i]); + if (d != 0) { + return d; + } + } + return a.length - b.length; + } + + /** * Compares two values. */ public int compare(Value a, Value b) { Index: src/main/java/org/apache/jackrabbit/core/query/lucene/Util.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/Util.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/Util.java (working copy) @@ -21,6 +21,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.index.Term; import org.apache.lucene.index.IndexReader; +import org.apache.jackrabbit.core.query.lucene.join.ValueComparator; import org.apache.jackrabbit.core.value.InternalValue; import org.slf4j.LoggerFactory; import org.slf4j.Logger; @@ -236,6 +237,29 @@ } /** + * Compares two arrays of comparables in the same style as + * {@link ValueComparator#compare(Value[], Value[])} + * + * The 2 methods *have* to be the same in order for the sort to work + * properly + */ + public static int compare(Comparable[] c1, Comparable[] c2) { + if (c1 == null) { + return -1; + } + if (c2 == null) { + return 1; + } + for (int i = 0; i < c1.length && i < c2.length; i++) { + int d = compare(c1[i], c2[i]); + if (d != 0) { + return d; + } + } + return c1.length - c2.length; + } + + /** * Compares the two values. If the values have differing types, then an * attempt is made to convert the second value into the type of the first * value. Index: src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java (revision 1205392) +++ src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java (working copy) @@ -17,6 +17,7 @@ package org.apache.jackrabbit.core.query.lucene; import java.io.IOException; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.WeakHashMap; @@ -112,6 +113,41 @@ } } + private static class ComparableArray implements Comparable { + + private Comparable[] c = new Comparable[0]; + + public ComparableArray(Comparable item, int index) { + insert(item, index); + } + + public int compareTo(ComparableArray o) { + return Util.compare(c, o.c); + } + + public ComparableArray insert(Comparable item, int index) { + if (index >= c.length) { + c = Arrays.copyOf(c, index + 1); + } + c[index] = item; + return this; + } + + /* + * This is needed by {@link UpperCaseSortComparator} and {@link LowerCaseSortComparator} + */ + @Override + public String toString() { + if (c == null) { + return null; + } + if (c.length == 1) { + return c[0].toString(); + } + return Arrays.toString(c); + } + } + /** * Reference to the single instance of SharedFieldCache. */ @@ -152,9 +188,10 @@ field = field.intern(); ValueIndex ret = lookup(reader, field, prefix); if (ret == null) { - Comparable[] retArray = new Comparable[reader.maxDoc()]; + final int maxDocs = reader.maxDoc(); + ComparableArray[] retArray = new ComparableArray[maxDocs]; int setValues = 0; - if (retArray.length > 0) { + if (maxDocs > 0) { IndexFormatVersion version = IndexFormatVersion.getVersion(reader); boolean hasPayloads = version.isAtLeast(IndexFormatVersion.V3); TermDocs termDocs; @@ -167,8 +204,6 @@ termDocs = reader.termDocs(); } TermEnum termEnum = reader.terms(new Term(field, prefix)); - - char[] tmp = new char[16]; try { if (termEnum.term() == null) { throw new RuntimeException("no terms in field " + field); @@ -178,30 +213,28 @@ if (term.field() != field || !term.text().startsWith(prefix)) { break; } - - // make sure term is compacted - String text = term.text(); - int len = text.length() - prefix.length(); - if (tmp.length < len) { - // grow tmp - tmp = new char[len]; - } - text.getChars(prefix.length(), text.length(), tmp, 0); - String value = new String(tmp, 0, len); - - termDocs.seek(termEnum); + final String value = termValueAsString(term, prefix); + termDocs.seek(term); while (termDocs.next()) { + int termPosition = 0; type = PropertyType.UNDEFINED; if (hasPayloads) { TermPositions termPos = (TermPositions) termDocs; - termPos.nextPosition(); + termPosition = termPos.nextPosition(); if (termPos.isPayloadAvailable()) { payload = termPos.getPayload(payload, 0); type = PropertyMetaData.fromByteArray(payload).getPropertyType(); } } setValues++; - retArray[termDocs.doc()] = getValue(value, type); + Comparable v = getValue(value, type); + int doc = termDocs.doc(); + ComparableArray ca = retArray[doc]; + if (ca == null) { + retArray[doc] = new ComparableArray(v, termPosition); + } else { + retArray[doc] = ca.insert(v, termPosition); + } } } while (termEnum.next()); } finally { @@ -217,6 +250,22 @@ } /** + * Extracts the value from a given Term as a String + * + * @param term + * @param prefix + * @return string value contained in the term + */ + private static String termValueAsString(Term term, String prefix) { + // make sure term is compacted + String text = term.text(); + int length = text.length() - prefix.length(); + char[] tmp = new char[length]; + text.getChars(prefix.length(), text.length(), tmp, 0); + return new String(tmp, 0, length); + } + + /** * See if a ValueIndex object is in the cache. */ ValueIndex lookup(IndexReader reader, String field, String prefix) { @@ -280,7 +329,7 @@ /** * Creates Key for ValueIndex lookup. */ - Key(String field, String prefix) { + Key(String field, String prefix) { this.field = field.intern(); this.prefix = prefix.intern(); } @@ -292,8 +341,7 @@ public boolean equals(Object o) { if (o instanceof Key) { Key other = (Key) o; - return other.field == field - && other.prefix == prefix; + return other.field == field && other.prefix == prefix; } return false; }