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 2013/09/05 15:17:19 UTC

svn commit: r1520297 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/query/ oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ oak-core/src/test/resources/org/apache/jackrabbit/oak/query/ oak-jcr/src/test/java/org/...

Author: thomasm
Date: Thu Sep  5 13:17:19 2013
New Revision: 1520297

URL: http://svn.apache.org/r1520297
Log:
OAK-1000 Queries on node name fail if the name starts with a number

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/XPathToSQL2Converter.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeLocalNameImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java
    jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2.txt
    jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java Thu Sep  5 13:17:19 2013
@@ -19,6 +19,15 @@ package org.apache.jackrabbit.oak.query;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.Maps.newHashMap;
 
+import java.math.BigDecimal;
+import java.text.ParseException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.jcr.PropertyType;
+
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -39,18 +48,9 @@ import org.apache.jackrabbit.oak.query.a
 import org.apache.jackrabbit.oak.query.ast.StaticOperandImpl;
 import org.apache.jackrabbit.oak.spi.query.PropertyValues;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.apache.jackrabbit.util.ISO9075;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.PropertyType;
-import java.math.BigDecimal;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * The SQL2 parser can convert a JCR-SQL2 query to a query. The 'old' SQL query
  * language (here named SQL-1) is also supported.
@@ -506,7 +506,7 @@ public class SQL2Parser {
     }
 
     private String readPath() throws ParseException {
-        return ISO9075.decode(readName());
+        return readName();
     }
 
     private DynamicOperandImpl parseDynamicOperand() throws ParseException {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/XPathToSQL2Converter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/XPathToSQL2Converter.java?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/XPathToSQL2Converter.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/XPathToSQL2Converter.java Thu Sep  5 13:17:19 2013
@@ -393,8 +393,11 @@ public class XPathToSQL2Converter {
         if (currentSelector.nodeName != null) {
             Function f = new Function("name");
             f.params.add(new SelectorExpr(currentSelector));
+            String n = currentSelector.nodeName;
+            // encode again, because it will be decoded again
+            n = ISO9075.encode(n);
             Condition c = new Condition(f, "=", 
-                    Literal.newString(currentSelector.nodeName), 
+                    Literal.newString(n), 
                     Expression.PRECEDENCE_CONDITION);
             condition = add(condition, c);
         }
@@ -1148,6 +1151,16 @@ public class XPathToSQL2Converter {
         String getColumnAliasName() {
             return toString();
         }
+        
+        /**
+         * Whether the result of this expression is a name. Names are subject to
+         * ISO9075 encoding.
+         * 
+         * @return whether this expression is a name.
+         */
+        boolean isName() {
+            return false;
+        }
 
     }
 
@@ -1175,21 +1188,23 @@ public class XPathToSQL2Converter {
     static class Literal extends Expression {
 
         final String value;
+        final String rawText;
 
-        Literal(String value) {
+        Literal(String value, String rawText) {
             this.value = value;
+            this.rawText = rawText;
         }
 
         public static Expression newBoolean(boolean value) {
-            return new Literal(String.valueOf(value));
+            return new Literal(String.valueOf(value), String.valueOf(value));
         }
 
         static Literal newNumber(String s) {
-            return new Literal(s);
+            return new Literal(s, s);
         }
 
         static Literal newString(String s) {
-            return new Literal(SQL2Parser.escapeStringLiteral(s));
+            return new Literal(SQL2Parser.escapeStringLiteral(s), s);
         }
 
         @Override
@@ -1265,25 +1280,50 @@ public class XPathToSQL2Converter {
 
         @Override
         public String toString() {
-            StringBuilder buff = new StringBuilder();
-            if (left != null) {
+            String leftExpr;
+            boolean leftExprIsName;
+            if (left == null) {
+                leftExprIsName = false;
+                leftExpr = "";
+            } else {
+                leftExprIsName = left.isName();
+                leftExpr = left.toString();
                 if (left.getPrecedence() < precedence) {
-                    buff.append('(').append(left.toString()).append(')');
-                } else {
-                    buff.append(left.toString());
+                    leftExpr = "(" + leftExpr + ")";
                 }
-                buff.append(' ');
             }
-            buff.append(operator);
-            if (right != null) {
-                buff.append(' ');
-                if (right.getPrecedence() < precedence) {
-                    buff.append('(').append(right.toString()).append(')');
+            boolean impossible = false;
+            String rightExpr;
+            if (right == null) {
+                rightExpr = "";
+            } else {
+                if (leftExprIsName && !"like".equals(operator)) {
+                    // need to de-escape _x0020_ and so on
+                    if (!(right instanceof Literal)) {
+                        throw new IllegalArgumentException(
+                                "Can only compare a name against a string literal, not " + right);
+                    }
+                    Literal l = (Literal) right;
+                    String raw = l.rawText;
+                    String decoded = ISO9075.decode(raw);
+                    String encoded = ISO9075.encode(decoded);
+                    rightExpr = SQL2Parser.escapeStringLiteral(decoded);
+                    if (!encoded.toUpperCase().equals(raw.toUpperCase())) {
+                        // nothing can potentially match
+                        impossible = true;
+                    }
                 } else {
-                    buff.append(right.toString());
+                    rightExpr = right.toString();
+                }
+                if (right.getPrecedence() < precedence) {
+                    rightExpr = "(" + right + ")";
                 }
             }
-            return buff.toString();
+            if (impossible) {
+                // a condition that can not possibly be true
+                return "upper(" + leftExpr + ") = 'never matches'";
+            }
+            return (leftExpr + " " + operator + " " + rightExpr).trim();
         }
 
         @Override
@@ -1323,6 +1363,14 @@ public class XPathToSQL2Converter {
         boolean isCondition() {
             return name.equals("contains") || name.equals("not");
         }
+        
+        @Override
+        boolean isName() {
+            if ("upper".equals(name) || "lower".equals(name)) {
+                return params.get(0).isName();
+            }
+            return "name".equals(name);
+        }
 
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeLocalNameImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeLocalNameImpl.java?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeLocalNameImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeLocalNameImpl.java Thu Sep  5 13:17:19 2013
@@ -28,7 +28,6 @@ import org.apache.jackrabbit.oak.api.Pro
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.query.index.FilterImpl;
 import org.apache.jackrabbit.oak.spi.query.PropertyValues;
-import org.apache.jackrabbit.util.ISO9075;
 
 /**
  * The function "localname(..)".
@@ -69,8 +68,6 @@ public class NodeLocalNameImpl extends D
     @Override
     public PropertyValue currentProperty() {
         String name = PathUtils.getName(selector.currentPath());
-        // Name escaping (convert space to _x0020_)
-        name = ISO9075.encode(name);
         int colon = name.indexOf(':');
         // TODO LOCALNAME: evaluation of local name might not be correct
         String localName = colon < 0 ? name : name.substring(colon + 1);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java Thu Sep  5 13:17:19 2013
@@ -76,8 +76,7 @@ public class NodeNameImpl extends Dynami
     @Override
     public PropertyValue currentProperty() {
         String path = selector.currentPath();
-        // Name escaping (convert space to _x0020_)
-        String name = ISO9075.encode(PathUtils.getName(path));
+        String name = PathUtils.getName(path);
         return PropertyValues.newName(name);
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2.txt?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2.txt (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/sql2.txt Thu Sep  5 13:17:19 2013
@@ -136,6 +136,11 @@ select [jcr:path] from [nt:base] as a wh
 commit / + "test": { "My Documents": { "x" : {}}}
 
 select [jcr:path] from [nt:base] where name() = 'My_x0020_Documents'
+
+select [jcr:path] from [nt:base] where name() like '%My Documents%'
+/test/My Documents
+
+select [jcr:path] from [nt:base] where name() = 'My Documents'
 /test/My Documents
 
 commit / - "test"

Modified: jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/resources/org/apache/jackrabbit/oak/query/xpath.txt Thu Sep  5 13:17:19 2013
@@ -23,6 +23,35 @@
 # * new tests are typically be added on top, after the syntax docs
 # * use ascii character only
 
+# xpath name escaping
+
+xpath2sql //My_x0020_Documents
+select [jcr:path], [jcr:score], * from [nt:base] as a where name(a) = 'My Documents' /* xpath: //My_x0020_Documents */
+
+xpath2sql //*[fn:name() = 'My Documents']
+select [jcr:path], [jcr:score], * from [nt:base] as a where upper(name(a)) = 'never matches' /* xpath: //*[fn:name() = 'My Documents'] */
+
+xpath2sql //*[fn:name() = 'My_x0020_Documents']
+select [jcr:path], [jcr:score], * from [nt:base] as a where name(a) = 'My Documents' /* xpath: //*[fn:name() = 'My_x0020_Documents'] */
+
+xpath2sql //*[fn:name() <> 'My_x0020_Documents']
+select [jcr:path], [jcr:score], * from [nt:base] as a where name(a) <> 'My Documents' /* xpath: //*[fn:name() <> 'My_x0020_Documents'] */
+
+xpath2sql //*[fn:upper-case(fn:name()) > 'MY_x0020_DOCS']
+select [jcr:path], [jcr:score], * from [nt:base] as a where upper(name(a)) > 'MY DOCS' /* xpath: //*[fn:upper-case(fn:name()) > 'MY_x0020_DOCS'] */
+
+xpath2sql //*[fn:lower-case(fn:name()) < 'my_x0020_docs']
+select [jcr:path], [jcr:score], * from [nt:base] as a where lower(name(a)) < 'my docs' /* xpath: //*[fn:lower-case(fn:name()) < 'my_x0020_docs'] */
+
+xpath2sql //*[fn:lower-case(fn:upper-case(fn:name())) >= 'my_x0020_docs']
+select [jcr:path], [jcr:score], * from [nt:base] as a where lower(upper(name(a))) >= 'my docs' /* xpath: //*[fn:lower-case(fn:upper-case(fn:name())) >= 'my_x0020_docs'] */
+
+xpath2sql //*[fn:upper-case(fn:lower-case(fn:name())) <= 'MY_x0020_DOCS']
+select [jcr:path], [jcr:score], * from [nt:base] as a where upper(lower(name(a))) <= 'MY DOCS' /* xpath: //*[fn:upper-case(fn:lower-case(fn:name())) <= 'MY_x0020_DOCS'] */
+
+xpath2sql //*[jcr:like(fn:name(), '%My Documents%')]
+select [jcr:path], [jcr:score], * from [nt:base] as a where name(a) like '%My Documents%' /* xpath: //*[jcr:like(fn:name(), '%My Documents%')] */
+
 # jackrabbit test queries
 
 xpath2sql /*[jcr:contains(., 'hello')]/rep:excerpt(.)

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java?rev=1520297&r1=1520296&r2=1520297&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java Thu Sep  5 13:17:19 2013
@@ -45,7 +45,6 @@ import javax.jcr.query.RowIterator;
 import org.apache.jackrabbit.commons.JcrUtils;
 import org.apache.jackrabbit.oak.jcr.AbstractRepositoryTest;
 import org.apache.jackrabbit.oak.jcr.NodeStoreFixture;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -318,7 +317,6 @@ public class QueryTest extends AbstractR
 
     @SuppressWarnings("deprecation")
     @Test
-    @Ignore
     public void fnNameEncoding() throws Exception {
         Session session = getAdminSession();
         session.getRootNode().addNode("123456_test_name");
@@ -331,7 +329,7 @@ public class QueryTest extends AbstractR
         assertEquals("/123456_test_name", getPaths(q));
 
         q = qm.createQuery("//*[fn:name() = '123456_test_name']", Query.XPATH);
-        assertEquals("/123456_test_name", getPaths(q));
+        assertEquals("", getPaths(q));
     }
 
 }