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 th...@apache.org on 2014/08/13 09:25:38 UTC
svn commit: r1617670 - in /jackrabbit/oak/branches/1.0/oak-core/src:
main/java/org/apache/jackrabbit/oak/query/xpath/
test/java/org/apache/jackrabbit/oak/query/
test/resources/org/apache/jackrabbit/oak/query/
Author: thomasm
Date: Wed Aug 13 07:25:38 2014
New Revision: 1617670
URL: http://svn.apache.org/r1617670
Log:
OAK-2021 XPath queries with certain combinations of "or" conditions don't use an index
OAK-2022 XPath queries with "order by" are never converted to "union"
Modified:
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Expression.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java
jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Expression.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Expression.java?rev=1617670&r1=1617669&r2=1617670&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Expression.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Expression.java Wed Aug 13 07:25:38 2014
@@ -17,6 +17,8 @@
package org.apache.jackrabbit.oak.query.xpath;
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
import org.apache.jackrabbit.oak.query.SQL2Parser;
import org.apache.jackrabbit.util.ISO9075;
@@ -46,6 +48,15 @@ abstract class Expression {
}
/**
+ * Get the optimized expression.
+ *
+ * @return the optimized expression
+ */
+ Expression optimize() {
+ return this;
+ }
+
+ /**
* Whether this is a condition.
*
* @return true if it is
@@ -55,6 +66,43 @@ abstract class Expression {
}
/**
+ * Whether this is a or contains a full-text condition.
+ *
+ * @return true if it is
+ */
+ boolean containsFullTextCondition() {
+ return false;
+ }
+
+ /**
+ * Get the left-hand-side expression for equality conditions.
+ * For example, for x=1, it is x. If it is not equality, return null.
+ *
+ * @return the left-hand-side expression, or null
+ */
+ String getCommonLeftPart() {
+ return null;
+ }
+
+ /**
+ * Get the left hand side of an expression.
+ *
+ * @return the left hand side
+ */
+ Expression getLeft() {
+ return null;
+ }
+
+ /**
+ * Get the list of the right hand side of an expression.
+ *
+ * @return the list
+ */
+ List<Expression> getRight() {
+ return null;
+ }
+
+ /**
* Pull an OR condition up to the right hand side of an AND condition.
*
* @return the (possibly rotated) expression
@@ -156,19 +204,24 @@ abstract class Expression {
int getPrecedence() {
return precedence;
}
-
- /**
- * Get the left-hand-side expression for equality conditions.
- * For example, for x=1, it is x. If it is not equality, return null.
- *
- * @return the left-hand-side expression, or null
- */
- public String getCommonLeftPart() {
+
+ @Override
+ String getCommonLeftPart() {
if (!"=".equals(operator)) {
return null;
}
return left.toString();
}
+
+ @Override
+ Expression getLeft() {
+ return left;
+ }
+
+ @Override
+ List<Expression> getRight() {
+ return Collections.singletonList(right);
+ }
@Override
public String toString() {
@@ -222,6 +275,11 @@ abstract class Expression {
boolean isCondition() {
return true;
}
+
+ @Override
+ Expression optimize() {
+ return this;
+ }
}
@@ -243,16 +301,87 @@ abstract class Expression {
*/
@Override
public String getCommonLeftPart() {
- if (left instanceof Condition && right instanceof Condition) {
- String l = ((Condition) left).getCommonLeftPart();
- String r = ((Condition) right).getCommonLeftPart();
- if (l != null && r != null && l.equals(r)) {
- return l;
- }
+ String l = left.getCommonLeftPart();
+ String r = right.getCommonLeftPart();
+ if (l != null && r != null && l.equals(r)) {
+ return l;
}
return null;
}
+ @Override
+ Expression optimize() {
+ Expression l = left.optimize();
+ Expression r = right.optimize();
+ if (l != left || r != right) {
+ return new OrCondition(l, r).optimize();
+ }
+ String commonLeft = getCommonLeftPart();
+ if (commonLeft == null) {
+ return this;
+ }
+ // "@x = 1 or @x = 2" is converted to "@x in (1, 2)"
+ ArrayList<Expression> list = new ArrayList<Expression>();
+ list.addAll(left.getRight());
+ list.addAll(right.getRight());
+ Expression le = left.getLeft();
+ InCondition in = new InCondition(le, list);
+ return in.optimize();
+ }
+
+ @Override
+ boolean containsFullTextCondition() {
+ return left.containsFullTextCondition() || right.containsFullTextCondition();
+ }
+
+ }
+
+ /**
+ * An "or" condition.
+ */
+ static class InCondition extends Expression {
+
+ final Expression left;
+ final List<Expression> list;
+
+ InCondition(Expression left, List<Expression> list) {
+ this.left = left;
+ this.list = list;
+ }
+
+ @Override
+ String getCommonLeftPart() {
+ return left.toString();
+ }
+
+ @Override
+ Expression getLeft() {
+ return left;
+ }
+
+ @Override
+ List<Expression> getRight() {
+ return list;
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder buff = new StringBuilder();
+ buff.append(left).append(" in(");
+ for (int i = 0; i < list.size(); i++) {
+ if (i > 0) {
+ buff.append(", ");
+ }
+ buff.append(list.get(i));
+ }
+ return buff.append(')').toString();
+ }
+
+ @Override
+ boolean isCondition() {
+ return true;
+ }
+
}
/**
@@ -263,6 +392,16 @@ abstract class Expression {
AndCondition(Expression left, Expression right) {
super(left, "and", right, Expression.PRECEDENCE_AND);
}
+
+ @Override
+ Expression optimize() {
+ Expression l = left.optimize();
+ Expression r = right.optimize();
+ if (l != left || r != right) {
+ return new AndCondition(l, r);
+ }
+ return this;
+ }
@Override
AndCondition pullOrRight() {
@@ -285,6 +424,11 @@ abstract class Expression {
return this;
}
+ @Override
+ boolean containsFullTextCondition() {
+ return left.containsFullTextCondition() || right.containsFullTextCondition();
+ }
+
}
/**
@@ -320,6 +464,11 @@ abstract class Expression {
}
@Override
+ boolean containsFullTextCondition() {
+ return true;
+ }
+
+ @Override
boolean isName() {
return left.isName();
}
@@ -352,6 +501,11 @@ abstract class Expression {
boolean isCondition() {
return true;
}
+
+ @Override
+ boolean containsFullTextCondition() {
+ return true;
+ }
@Override
boolean isName() {
@@ -520,5 +674,5 @@ abstract class Expression {
}
}
-
+
}
\ No newline at end of file
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java?rev=1617670&r1=1617669&r2=1617670&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java Wed Aug 13 07:25:38 2014
@@ -20,7 +20,6 @@ import java.util.ArrayList;
import org.apache.jackrabbit.oak.query.QueryImpl;
import org.apache.jackrabbit.oak.query.xpath.Expression.AndCondition;
-import org.apache.jackrabbit.oak.query.xpath.Expression.Contains;
import org.apache.jackrabbit.oak.query.xpath.Expression.OrCondition;
import org.apache.jackrabbit.oak.query.xpath.Expression.Property;
@@ -29,8 +28,6 @@ import org.apache.jackrabbit.oak.query.x
*/
public class Statement {
- private String xpathQuery;
-
private boolean explain;
private boolean measure;
@@ -49,15 +46,18 @@ public class Statement {
private Expression where;
- private ArrayList<Order> orderList = new ArrayList<Order>();
+ ArrayList<Order> orderList = new ArrayList<Order>();
+
+ String xpathQuery;
public Statement optimize() {
- if (explain || measure || orderList.size() > 0) {
+ if (explain || measure) {
return this;
}
if (where == null) {
return this;
}
+ where = where.optimize();
ArrayList<Expression> unionList = new ArrayList<Expression>();
addToUnionList(where, unionList);
if (unionList.size() == 1) {
@@ -71,37 +71,29 @@ public class Statement {
s.selectors = selectors;
s.columnList = columnList;
s.where = e;
- if (i == unionList.size() - 1) {
- s.xpathQuery = xpathQuery;
- }
if (union == null) {
union = s;
} else {
union = new UnionStatement(union.optimize(), s.optimize());
}
}
+ union.orderList = orderList;
+ union.xpathQuery = xpathQuery;
return union;
}
private static void addToUnionList(Expression condition, ArrayList<Expression> unionList) {
- if (condition instanceof OrCondition) {
+ if (condition.containsFullTextCondition()) {
+ // do not use union
+ } else if (condition instanceof OrCondition) {
OrCondition or = (OrCondition) condition;
- if (or.getCommonLeftPart() != null) {
- // @x = 1 or @x = 2
- // is automatically converted to
- // @x in (1, 2)
- // within the query engine
- } else if (or.left instanceof Contains && or.right instanceof Contains) {
- // do not optimize "contains"
- } else {
- // conditions of type
- // @x = 1 or @y = 2
- // or similar are converted to
- // (@x = 1) union (@y = 2)
- addToUnionList(or.left, unionList);
- addToUnionList(or.right, unionList);
- return;
- }
+ // conditions of type
+ // @x = 1 or @y = 2
+ // or similar are converted to
+ // (@x = 1) union (@y = 2)
+ addToUnionList(or.left, unionList);
+ addToUnionList(or.right, unionList);
+ return;
} else if (condition instanceof AndCondition) {
// conditions of type
// @a = 1 and (@x = 1 or @y = 2)
@@ -111,19 +103,10 @@ public class Statement {
and = and.pullOrRight();
if (and.right instanceof OrCondition) {
OrCondition or = (OrCondition) and.right;
- if (or.getCommonLeftPart() != null) {
- // @x = 1 or @x = 2
- // is automatically converted to
- // @x in (1, 2)
- // within the query engine
- } else if (or.left instanceof Contains && or.right instanceof Contains) {
- // do not optimize "contains"
- } else {
- // same as above, but with the added "and"
- addToUnionList(new AndCondition(and.left, or.left), unionList);
- addToUnionList(new AndCondition(and.left, or.right), unionList);
- return;
- }
+ // same as above, but with the added "and"
+ addToUnionList(new AndCondition(and.left, or.left), unionList);
+ addToUnionList(new AndCondition(and.left, or.right), unionList);
+ return;
}
}
unionList.add(condition);
@@ -255,7 +238,25 @@ public class Statement {
@Override
public String toString() {
- return s1 + " union " + s2;
+ StringBuilder buff = new StringBuilder();
+ buff.append(s1).append(" union ").append(s2);
+ // order by ...
+ if (orderList != null && !orderList.isEmpty()) {
+ buff.append(" order by ");
+ for (int i = 0; i < orderList.size(); i++) {
+ if (i > 0) {
+ buff.append(", ");
+ }
+ buff.append(orderList.get(i));
+ }
+ }
+ // leave original xpath string as a comment
+ if (xpathQuery != null) {
+ buff.append(" /* xpath: ");
+ buff.append(xpathQuery);
+ buff.append(" */");
+ }
+ return buff.toString();
}
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java?rev=1617670&r1=1617669&r2=1617670&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2ParserTest.java Wed Aug 13 07:25:38 2014
@@ -59,7 +59,7 @@ public class SQL2ParserTest {
public void testUnwrappedOr() throws ParseException {
String q = new XPathToSQL2Converter()
.convert("/jcr:root/home//test/* [@type='t1' or @type='t2' or @type='t3']");
- String token = "and (b.[type] = 't1' or b.[type] = 't2' or b.[type] = 't3')";
+ String token = "and b.[type] in('t1', 't2', 't3')";
assertTrue(q.contains(token));
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt?rev=1617670&r1=1617669&r2=1617670&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt Wed Aug 13 07:25:38 2014
@@ -24,6 +24,97 @@
# * new tests are typically be added on top, after the syntax docs
# * use ascii character only
+# "or" problem
+xpath2sql /jcr:root/content//*[(@i = '1' or @i = '2') or (@t = 'a' or @t = 'b')]
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] in('a', 'b')
+ /* xpath ... */
+
+# "or" problem
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x')) and (@t = 'a' or @t = 'b')]
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] in('a', 'b')
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] in('a', 'b')
+ and [s] = 'x'
+ /* xpath ... */
+
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x')) and (@t = 'a')]
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] = 'a'
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] = 'a'
+ and [s] = 'x'
+ /* xpath ... */
+
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x'))]
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [s] = 'x'
+ /* xpath ... */
+
+# "or" with "order by" problem
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x')) and (@t = 'a' or @t = 'b')] order by @a
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] in('a', 'b')
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] in('a', 'b')
+ and [s] = 'x'
+ order by [a]
+ /* xpath ... */
+
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x')) and (@t = 'a')] order by @a
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] = 'a'
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [t] = 'a'
+ and [s] = 'x'
+ order by [a]
+ /* xpath ... */
+
+xpath2sql /jcr:root/content//*[((@i = '1' or @i = '2') or (@s = 'x'))] order by @a
+select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [i] in('1', '2')
+ union select [jcr:path], [jcr:score], *
+ from [nt:base] as a
+ where isdescendantnode(a, '/content')
+ and [s] = 'x'
+ order by [a]
+ /* xpath ... */
+
# "or" problem (OAK-1432)
xpath2sql /jcr:root/content//element(*, nt:unstructured)
[((@p1 = 'x1' or @p2 = 'x2') or @p3 = 'x3')]
@@ -74,8 +165,7 @@ select d.[jcr:path] as [jcr:path], d.[jc
and isdescendantnode(a, '/testroot')
and name(b) = 'c'
and name(c) = 'd'
- and (d.[jcr:uuid] = '1'
- or d.[jcr:uuid] = '2')
+ and d.[jcr:uuid] in('1', '2')
/* xpath ... */
# property names with missing @
@@ -161,9 +251,7 @@ select [jcr:path], [jcr:score], *
where [jcr:primaryType] = 'nt:unstructured'
and (contains(*, 'hello')
or contains(*, 'world'))
- union select [jcr:path], [jcr:score], *
- from [nt:base] as a
- where [content] = '/data'
+ or [content] = '/data'
and [jcr:primaryType] = 'nt:folder'
/* xpath ... */
@@ -185,16 +273,13 @@ select b.[jcr:path] as [jcr:path], b.[jc
xpath2sql //*[@x = 1 or @x = 2]
select [jcr:path], [jcr:score], *
from [nt:base] as a
- where [x] = 1
- or [x] = 2
+ where [x] in(1, 2)
/* xpath ... */
xpath2sql //*[@x = 1 or @x = 2 or @x = 3]
select [jcr:path], [jcr:score], *
from [nt:base] as a
- where [x] = 1
- or [x] = 2
- or [x] = 3
+ where [x] in(1, 2, 3)
/* xpath ... */
xpath2sql //*[@x = 1 or @y = 2]
@@ -209,8 +294,7 @@ select [jcr:path], [jcr:score], *
xpath2sql //*[@x = 1 or @x = 2 or @y = 3]
select [jcr:path], [jcr:score], *
from [nt:base] as a
- where [x] = 1
- or [x] = 2
+ where [x] in(1, 2)
union select [jcr:path], [jcr:score], *
from [nt:base] as a
where [y] = 3
@@ -296,8 +380,7 @@ select b.[jcr:path] as [jcr:path], b.[jc
from [nt:base] as a
inner join [nt:base] as b
on isdescendantnode(b, a)
- where (a.[jcr:uuid] = '1'
- or a.[jcr:uuid] = '2')
+ where a.[jcr:uuid] in('1', '2')
and b.[jcr:uuid] is not null
/* xpath ... */
@@ -938,10 +1021,10 @@ select [jcr:path], [jcr:score], *
/* xpath ... */
xpath2sql /jcr:root/content/campaigns//element(*, PageContent)
- [(@sling:resourceType = 'teaser'
- or @sling:resourceType = 'newsletter'
- or @teaserPageType = 'newsletter'
- or @teaserPageType = 'tweet')
+ [((@sling:resourceType = 'teaser'
+ or @sling:resourceType = 'newsletter')
+ or (@teaserPageType = 'newsletter'
+ or @teaserPageType = 'tweet'))
and ((@onTime < xs:dateTime('2012-04-01T00:00:00.000+02:00'))
or not(@onTime))
and ((@offTime >= xs:dateTime('2012-02-26T00:00:00.000+01:00'))
@@ -949,15 +1032,52 @@ xpath2sql /jcr:root/content/campaigns//e
order by @onTime
select [jcr:path], [jcr:score], *
from [PageContent] as a
- where ([sling:resourceType] = 'teaser'
- or [sling:resourceType] = 'newsletter'
- or [teaserPageType] = 'newsletter'
- or [teaserPageType] = 'tweet')
- and ([onTime] < cast('2012-04-01T00:00:00.000+02:00' as date)
- or [onTime] is null)
- and ([offTime] >= cast('2012-02-26T00:00:00.000+01:00' as date)
- or [offTime] is null)
+ where [onTime] < cast('2012-04-01T00:00:00.000+02:00' as date)
+ and [offTime] >= cast('2012-02-26T00:00:00.000+01:00' as date)
+ and isdescendantnode(a, '/content/campaigns')
+ and [sling:resourceType] in('teaser', 'newsletter')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] < cast('2012-04-01T00:00:00.000+02:00' as date)
+ and [offTime] >= cast('2012-02-26T00:00:00.000+01:00' as date)
+ and isdescendantnode(a, '/content/campaigns')
+ and [teaserPageType] in('newsletter', 'tweet')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] is null
+ and [offTime] >= cast('2012-02-26T00:00:00.000+01:00' as date)
+ and isdescendantnode(a, '/content/campaigns')
+ and [sling:resourceType] in('teaser', 'newsletter')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] is null
+ and [offTime] >= cast('2012-02-26T00:00:00.000+01:00' as date)
+ and isdescendantnode(a, '/content/campaigns')
+ and [teaserPageType] in('newsletter', 'tweet')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] < cast('2012-04-01T00:00:00.000+02:00' as date)
+ and [offTime] is null
+ and isdescendantnode(a, '/content/campaigns')
+ and [sling:resourceType] in('teaser', 'newsletter')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] < cast('2012-04-01T00:00:00.000+02:00' as date)
+ and [offTime] is null
+ and isdescendantnode(a, '/content/campaigns')
+ and [teaserPageType] in('newsletter', 'tweet')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] is null
+ and [offTime] is null
+ and isdescendantnode(a, '/content/campaigns')
+ and [sling:resourceType] in('teaser', 'newsletter')
+ union select [jcr:path], [jcr:score], *
+ from [PageContent] as a
+ where [onTime] is null
+ and [offTime] is null
and isdescendantnode(a, '/content/campaigns')
+ and [teaserPageType] in('newsletter', 'tweet')
order by [onTime]
/* xpath ... */