diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java index ca6e3d54e..38f4173a3 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java @@ -127,16 +127,16 @@ public class Substring BinaryOpExpState bstate = (BinaryOpExpState) state; FilterValue str = new FilterValueImpl(sel, ctx, bstate.state1, _val1); FilterValue start; - FilterValue end = null; + FilterValue length = null; if (_val2 instanceof Args) { FilterValue[] filts = ((Args) _val2).newFilterValues(sel, ctx, bstate.state2); start = filts[0]; - end = filts[1]; + length = filts[1]; } else start = new FilterValueImpl(sel, ctx, bstate.state2, _val2); - ctx.store.getDBDictionary().substring(sql, str, start, end); + ctx.store.getDBDictionary().substring(sql, str, start, length); } public void acceptVisit(ExpressionVisitor visitor) { diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractDB2Dictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractDB2Dictionary.java index b9b32bbff..6d51da451 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractDB2Dictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractDB2Dictionary.java @@ -73,7 +73,7 @@ public abstract class AbstractDB2Dictionary public void indexOf(SQLBuffer buf, FilterValue str, FilterValue find, FilterValue start) { - buf.append("(LOCATE(CAST(("); + buf.append("LOCATE(CAST(("); find.appendTo(buf); buf.append(") AS VARCHAR(").append(Integer.toString(varcharCastLength)) .append(")), CAST(("); @@ -83,37 +83,32 @@ public abstract class AbstractDB2Dictionary if (start != null) { buf.append(", CAST(("); start.appendTo(buf); - buf.append(") AS INTEGER) + 1"); + buf.append(") AS INTEGER)"); } - buf.append(") - 1)"); + buf.append(")"); } + @Override public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { buf.append("SUBSTR(CAST(("); str.appendTo(buf); buf.append(") AS VARCHAR(").append(Integer.toString(varcharCastLength)) .append(")), "); if (start.getValue() instanceof Number) { - long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); + buf.append(Long.toString(toLong(start))); } else { buf.append("CAST(("); start.appendTo(buf); - buf.append(") AS INTEGER) + 1"); + buf.append(") AS INTEGER)"); } - if (end != null) { + if (length != null) { buf.append(", "); - if (start.getValue() instanceof Number - && end.getValue() instanceof Number) { - long startLong = toLong(start); - long endLong = toLong(end); - buf.append(Long.toString(endLong - startLong)); + if (length.getValue() instanceof Number) { + buf.append(Long.toString(toLong(length))); } else { buf.append("CAST(("); - end.appendTo(buf); - buf.append(") AS INTEGER) - CAST(("); - start.appendTo(buf); + length.appendTo(buf); buf.append(") AS INTEGER)"); } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractSQLServerDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractSQLServerDictionary.java index b23f72992..e6fceccc2 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractSQLServerDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractSQLServerDictionary.java @@ -120,23 +120,20 @@ public abstract class AbstractSQLServerDictionary } public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { - if (end != null) - super.substring(buf, str, start, end); + FilterValue length) { + if (length != null) + super.substring(buf, str, start, length); else { - // ### it would be good to change this logic as in DBDictionary to - // ### simplify the generated SQL buf.append("SUBSTRING("); str.appendTo(buf); buf.append(", "); start.appendTo(buf); - buf.append(" + 1, "); - buf.append("LEN("); + buf.append(", LEN("); str.appendTo(buf); buf.append(")"); buf.append(" - ("); start.appendTo(buf); - buf.append("))"); + buf.append(" - 1))"); } } @@ -149,9 +146,9 @@ public abstract class AbstractSQLServerDictionary substring(buf, str, start, null); else str.appendTo(buf); - buf.append(") - 1"); + buf.append(")"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java index fc8874f63..42fd2110e 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java @@ -829,12 +829,12 @@ public class DB2Dictionary public void indexOf(SQLBuffer buf, FilterValue str, FilterValue find, FilterValue start) { if (find.getValue() != null) { // non constants - buf.append("(LOCATE(CAST(("); + buf.append("LOCATE(CAST(("); find.appendTo(buf); buf.append(") AS VARCHAR(1000)), "); } else { // this is a constant - buf.append("(LOCATE("); + buf.append("LOCATE("); find.appendTo(buf); buf.append(", "); } @@ -846,16 +846,16 @@ public class DB2Dictionary str.appendTo(buf); } if (start != null) { - if (start.getValue() == null) { + if (start.getValue() != null) { buf.append(", CAST(("); start.appendTo(buf); - buf.append(") AS INTEGER) + 1"); + buf.append(") AS INTEGER)"); } else { buf.append(", "); start.appendTo(buf); } } - buf.append(") - 1)"); + buf.append(")"); } /** diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java index ce1e04552..a8d100683 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java @@ -2745,37 +2745,30 @@ public class DBDictionary /** * Invoke this database's substring function. + * Numeric parameters are inlined if possible. This is to handle grouping by SUBSTRING - + * most databases do not allow parameter binding in this case. * * @param buf the SQL buffer to write the substring invocation to * @param str a query value representing the target string * @param start a query value representing the start index - * @param end a query value representing the end index, or null for none + * @param length a query value representing the length of substring, or null for none */ public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { buf.append(substringFunctionName).append("("); str.appendTo(buf); buf.append(", "); if (start.getValue() instanceof Number) { - long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); + buf.append(Long.toString(toLong(start))); } else { - buf.append("("); start.appendTo(buf); - buf.append(" + 1)"); } - if (end != null) { + if (length != null) { buf.append(", "); - if (start.getValue() instanceof Number - && end.getValue() instanceof Number) { - long startLong = toLong(start); - long endLong = toLong(end); - buf.append(Long.toString(endLong - startLong)); + if (length.getValue() instanceof Number) { + buf.append(Long.toString(toLong(length))); } else { - end.appendTo(buf); - buf.append(" - ("); - start.appendTo(buf); - buf.append(")"); + length.appendTo(buf); } } buf.append(")"); @@ -2803,9 +2796,9 @@ public class DBDictionary str.appendTo(buf); buf.append("), ("); find.appendTo(buf); - buf.append(")) - 1"); + buf.append("))"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/FirebirdDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/FirebirdDictionary.java index 3fc8bb859..08aafea10 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/FirebirdDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/FirebirdDictionary.java @@ -402,28 +402,26 @@ public class FirebirdDictionary * Parameters are inlined because neither parameter binding nor expressions * are accepted by Firebird here. As a result, an * {@link UnsupportedException} is thrown when something else than a - * constant is used in start or end. + * constant is used in start or length. */ @Override public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { buf.append(substringFunctionName).append("("); str.appendTo(buf); buf.append(" FROM "); if (start.getValue() instanceof Number) { long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); + buf.append(Long.toString(startLong)); } else { throw new UnsupportedException(_loc.get("function-not-supported", getClass(), substringFunctionName + " with non-constants")); } - if (end != null) { + if (length != null) { buf.append(" FOR "); - if (start.getValue() instanceof Number - && end.getValue() instanceof Number) { - long startLong = toLong(start); - long endLong = toLong(end); - buf.append(Long.toString(endLong - startLong)); + if (length.getValue() instanceof Number) { + long lengthLong = toLong(length); + buf.append(Long.toString(lengthLong)); } else { throw new UnsupportedException(_loc.get( "function-not-supported", getClass(), substringFunctionName diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java index a45145c20..6dd07d69a 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java @@ -232,9 +232,9 @@ public class H2Dictionary extends DBDictionary { substring(buf, str, start, null); else str.appendTo(buf); - buf.append(") - 1"); + buf.append(")"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java index 0990db321..9b8644afc 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java @@ -345,16 +345,15 @@ public class HSQLDictionary extends DBDictionary { @Override public void indexOf(SQLBuffer buf, FilterValue str, FilterValue find, FilterValue start) { - buf.append("(LOCATE("); + buf.append("LOCATE("); find.appendTo(buf); buf.append(", "); str.appendTo(buf); if (start != null) { - buf.append(", ("); + buf.append(", "); start.appendTo(buf); - buf.append(" + 1)"); } - buf.append(") - 1)"); + buf.append(")"); } @Override diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/IngresDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/IngresDictionary.java index f10813525..c2721d955 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/IngresDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/IngresDictionary.java @@ -260,7 +260,6 @@ public class IngresDictionary extends DBDictionary { * - a query value representing the start index, or null to * start at the beginning */ - @Override public void indexOf(SQLBuffer buf, FilterValue str, FilterValue find, FilterValue start) { @@ -273,10 +272,10 @@ public class IngresDictionary extends DBDictionary { else str.appendTo(buf); - buf.append(")) - 1"); + buf.append("))"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); @@ -291,30 +290,24 @@ public class IngresDictionary extends DBDictionary { */ @Override public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { buf.append(substringFunctionName).append("("); str.appendTo(buf); buf.append(", "); if (start.getValue() instanceof Number) { - long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); + buf.append(Long.toString(toLong(start))); } else { buf.append("(CAST (("); start.appendTo(buf); - buf.append(" + 1) AS INTEGER))"); + buf.append(") AS INTEGER))"); } - if (end != null) { + if (length != null) { buf.append(", "); - if (start.getValue() instanceof Number - && end.getValue() instanceof Number) { - long startLong = toLong(start); - long endLong = toLong(end); - buf.append(Long.toString(endLong - startLong)); + if (length.getValue() instanceof Number) { + buf.append(Long.toString(toLong(length))); } else { - buf.append("(CAST ("); - end.appendTo(buf); - buf.append(" - ("); - start.appendTo(buf); + buf.append("(CAST (("); + length.appendTo(buf); buf.append(") AS INTEGER))"); } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/JDataStoreDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/JDataStoreDictionary.java index 461c73b18..7d6945120 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/JDataStoreDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/JDataStoreDictionary.java @@ -69,21 +69,21 @@ public class JDataStoreDictionary @Override public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { buf.append("SUBSTRING("); str.appendTo(buf); buf.append(" FROM ("); start.appendTo(buf); - buf.append(" + 1) FOR ("); - if (end == null) { + buf.append(") FOR ("); + if (length == null) { buf.append("CHAR_LENGTH("); str.appendTo(buf); buf.append(")"); } else - end.appendTo(buf); + length.appendTo(buf); buf.append(" - ("); start.appendTo(buf); - buf.append(")))"); + buf.append(" - 1)))"); } @Override @@ -96,9 +96,9 @@ public class JDataStoreDictionary substring(buf, str, start, null); else str.appendTo(buf); - buf.append(") - 1"); + buf.append(")"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PointbaseDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PointbaseDictionary.java index c89f7c480..aeb3aa993 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PointbaseDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PointbaseDictionary.java @@ -103,20 +103,16 @@ public class PointbaseDictionary } public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { + FilterValue length) { // SUBSTRING in Pointbase is of the form: // SELECT SUBSTRING(SOME_COLUMN FROM 1 FOR 5) buf.append("SUBSTRING("); str.appendTo(buf); buf.append(" FROM "); start.appendTo(buf); - buf.append(" + 1"); - if (end != null) { + if (length != null) { buf.append(" FOR "); - end.appendTo(buf); - buf.append(" - ("); - start.appendTo(buf); - buf.append(")"); + length.appendTo(buf); } buf.append(")"); } @@ -130,9 +126,9 @@ public class PointbaseDictionary substring(buf, str, start, null); else str.appendTo(buf); - buf.append(") - 1"); + buf.append(")"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java index 5ccd5895d..1cacb1326 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java @@ -352,9 +352,9 @@ public class PostgresDictionary substring(buf, str, start, null); else str.appendTo(buf); - buf.append(") - 1"); + buf.append(")"); if (start != null) { - buf.append(" + "); + buf.append(" - 1 + "); start.appendTo(buf); } buf.append(")"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SolidDBDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SolidDBDictionary.java index a9bfcef0d..f64b11c37 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SolidDBDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SolidDBDictionary.java @@ -317,20 +317,17 @@ public class SolidDBDictionary @Override public void substring(SQLBuffer buf, FilterValue str, FilterValue start, - FilterValue end) { - if (end != null) { - super.substring(buf, str, start, end); + FilterValue length) { + if (length != null) { + super.substring(buf, str, start, length); } else { buf.append(substringFunctionName).append("("); str.appendTo(buf); buf.append(", "); if (start.getValue() instanceof Number) { - long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); + buf.append(Long.toString(toLong(start))); } else { - buf.append("("); start.appendTo(buf); - buf.append(" + 1)"); } buf.append(", "); if (start.getValue() instanceof Number) { @@ -350,22 +347,15 @@ public class SolidDBDictionary @Override public void indexOf(SQLBuffer buf, FilterValue str, FilterValue find, FilterValue start) { - buf.append("(LOCATE("); + buf.append("LOCATE("); find.appendTo(buf); buf.append(", "); str.appendTo(buf); if (start != null) { buf.append(", "); - if (start.getValue() instanceof Number) { - long startLong = toLong(start); - buf.append(Long.toString(startLong + 1)); - } else { - buf.append("("); - start.appendTo(buf); - buf.append(" + 1)"); - } + start.appendTo(buf); } - buf.append(") - 1)"); + buf.append(")"); } @Override diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ExpressionFactory.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ExpressionFactory.java index 5098a1870..7cd438ce7 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ExpressionFactory.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ExpressionFactory.java @@ -18,8 +18,6 @@ */ package org.apache.openjpa.kernel.exps; -import java.sql.Time; -import java.sql.Timestamp; import java.util.Date; import org.apache.openjpa.meta.ClassMetaData; @@ -332,8 +330,9 @@ public interface ExpressionFactory { public Value abs(Value num); /** - * Return a value representing the {@link String#indexOf} function on + * Return a value representing the indexOf (LOCATE in JPQL) function on * the given target with the given args. + * The optional second argument is one-based. */ public Value indexOf(Value str, Value args); @@ -349,9 +348,9 @@ public interface ExpressionFactory { public Value sqrt(Value num); /** - * Return a value representing the {@link String#substring} function on - * the given target with the given args. As with {@link String#substring}, - * the start index is zero-based, and the second argument is the end index. + * Return a value representing the substring function on + * the given target with the given args. Unlike as with {@link String#substring}, + * the start index is one-based, and the second argument is the length. */ public Value substring(Value str, Value args); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IndexOf.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IndexOf.java index 76e789fc5..c41dbb54c 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IndexOf.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IndexOf.java @@ -22,6 +22,7 @@ import org.apache.openjpa.kernel.StoreContext; /** * Find the index of one string within another. + * Index is 1-based. * * @author Abe White */ @@ -55,9 +56,9 @@ class IndexOf if (arg instanceof Object[]) { Object[] args = (Object[]) arg; idx = str.toString().indexOf(args[0].toString(), - ((Number) args[1]).intValue()); + ((Number) args[1]).intValue() - 1) + 1; } else - idx = str.toString().indexOf(arg.toString()); + idx = str.toString().indexOf(arg.toString()) + 1; return idx; } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Substring.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Substring.java index bd06f8307..bef96aa17 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Substring.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Substring.java @@ -53,12 +53,12 @@ class Substring Object arg = _args.eval(candidate, orig, ctx, params); if (arg instanceof Object[]) { Object[] args = (Object[]) arg; - int start = ((Number) args[0]).intValue(); - int end = ((Number) args[1]).intValue(); + int start = ((Number) args[0]).intValue() - 1; + int length = ((Number) args[1]).intValue(); String string = str == null ? "" : str.toString(); - return string.substring(start, Math.min(end, string.length())); + return string.substring(start, Math.min(start + length, string.length())); } - return str.toString().substring(((Number) arg).intValue()); + return str.toString().substring(((Number) arg).intValue() - 1); } public void acceptVisit(ExpressionVisitor visitor) { diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java index 157d5d054..c22f06180 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java @@ -1304,18 +1304,22 @@ public class JPQLExpressionBuilder return factory.concat(val1, val2); case JJTSUBSTRING: - if (node.children.length == 3) { - val1 = getValue(child(node, 0, 3)); - val2 = getValue(child(node, 1, 3)); - JPQLNode child3 = child(node, 2, 3); - if (child3.id == JJTINTEGERLITERAL) + // Literals are forced to be Integers because PostgreSQL rejects Longs in SUBSTRING parameters. + // This however does not help if an expression like 1+1 is passed as parameter. + val1 = getValue(firstChild(node)); + JPQLNode child2 = secondChild(node); + if (child2.id == JJTINTEGERLITERAL) { + val2 = getIntegerValue(child2); + } else { + val2 = getValue(child2); + } + if (node.getChildCount() == 3) { + JPQLNode child3 = thirdChild(node); + if (child3.id == JJTINTEGERLITERAL) { val3 = getIntegerValue(child3); - else + } else { val3 = getValue(child3); - - } else if (node.children.length == 2) { - val1 = getValue(child(node, 0, 2)); - val2 = getValue(child(node, 1, 2)); + } } setImplicitType(val1, TYPE_STRING); setImplicitType(val2, Integer.TYPE); @@ -1325,12 +1329,11 @@ public class JPQLExpressionBuilder return convertSubstringArguments(factory, val1, val2, val3); case JJTLOCATE: - // as with SUBSTRING (above), the semantics for LOCATE differ - // from ExpressionFactory.indexOf in that LOCATE uses a - // 0-based index, and indexOf uses a 1-based index Value locatePath = getValue(firstChild(node)); Value locateSearch = getValue(secondChild(node)); Value locateFromIndex = null; + // Literals are forced to be Integers because PostgreSQL rejects Longs in POSITION parameters. + // This however does not help if an expression like 1+1 is passed as parameter. if (node.getChildCount() > 2) { // optional start index arg JPQLNode child3 = thirdChild(node); if (child3.id == JJTINTEGERLITERAL) { @@ -1342,16 +1345,11 @@ public class JPQLExpressionBuilder setImplicitType(locateSearch, TYPE_STRING); if (locateFromIndex != null) - setImplicitType(locateFromIndex, TYPE_STRING); + setImplicitType(locateFromIndex, Integer.TYPE); - return factory.add(factory.indexOf(locateSearch, + return factory.indexOf(locateSearch, locateFromIndex == null ? locatePath - : factory.newArgumentList(locatePath, - factory.subtract(locateFromIndex, - factory.newLiteral(1, - Literal.TYPE_NUMBER)))), - factory.newLiteral(1, - Literal.TYPE_NUMBER)); + : factory.newArgumentList(locatePath, locateFromIndex)); case JJTAGGREGATE: // simply pass-through while asserting a single child @@ -1461,14 +1459,6 @@ public class JPQLExpressionBuilder * Converts JPQL substring() function to OpenJPA ExpressionFactory * substring() arguments. * - * The semantics of the JPQL substring() function are that arg2 is the - * 1-based start index, and arg3 is the length of the string to be return. - * This is different than the semantics of the ExpressionFactory's - * substring(), which matches the Java language (0-based start index, - * arg2 is the end index): we perform the translation by adding one to the - * first argument, and then adding the first argument to the second - * argument to get the endIndex. - * * @param val1 the original String * @param val2 the 1-based start index as per JPQL substring() semantics * @param val3 the length of the returned string as per JPQL semantics @@ -1476,35 +1466,10 @@ public class JPQLExpressionBuilder */ public static Value convertSubstringArguments(ExpressionFactory factory, Value val1, Value val2, Value val3) { - Value start = null; - Value end = null; - if (val2 instanceof Literal && - (val3 == null || val3 instanceof Literal)) { - // optimize SQL for the common case of two literals - long jpqlStart = ((Number) ((Literal) val2).getValue()) - .longValue(); - start = factory.newLiteral(Long.valueOf(jpqlStart - 1), - Literal.TYPE_NUMBER); - if (val3 != null) { - long length = ((Number) ((Literal) val3).getValue()) - .longValue(); - long endIndex = length + (jpqlStart - 1); - end = factory.newLiteral(Long.valueOf(endIndex), - Literal.TYPE_NUMBER); - } - } else { - start = factory.subtract(val2, factory.newLiteral - (1, Literal.TYPE_NUMBER)); - if (val3 != null) - end = factory.add(val3, - (factory.subtract(val2, factory.newLiteral - (1, Literal.TYPE_NUMBER)))); - } if (val3 != null) - return factory.substring(val1, factory.newArgumentList(start, end)); + return factory.substring(val1, factory.newArgumentList(val2, val3)); else - return factory.substring(val1, start); - + return factory.substring(val1, val2); } private void assertQueryExtensions(String clause) { OpenJPAConfiguration conf = resolver.getConfiguration(); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestEJBQLFunction.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestEJBQLFunction.java index a74d7b230..2c7ae3c5e 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestEJBQLFunction.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestEJBQLFunction.java @@ -107,7 +107,7 @@ public class TestEJBQLFunction extends AbstractTestCase { em.refresh(user); assertNotNull("the user is null", user); - assertEquals("the users name is not AblahumSeet", "Ablahumeeth", + assertEquals("the users name is not Ablahumeeth", "Ablahumeeth", user.getName()); query = "UPDATE CompUser e SET e.name = " + @@ -121,8 +121,23 @@ public class TestEJBQLFunction extends AbstractTestCase { em.refresh(user); assertNotNull("the user is null", user); - assertEquals("the users name is not Ablahumeeth", "XYZeeth", + assertEquals("the users name is not XYZeeth", "XYZeeth", user.getName()); + + query = "UPDATE CompUser e SET e.name = " + + "CONCAT('CAD', SUBSTRING(e.name, LOCATE('e', e.name, 5))) " + + "WHERE e.name='XYZeeth'"; + result = em.createQuery(query).executeUpdate(); + + assertEquals("the result is not 1", 1, result); + + user = em.find(CompUser.class, userid1); + em.refresh(user); + + assertNotNull("the user is null", user); + assertEquals("the users name is not CADeth", "CADeth", + user.getName()); + endTx(em); endEm(em); } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestLocate.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestLocate.java new file mode 100644 index 000000000..c9fbcbf1c --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestLocate.java @@ -0,0 +1,64 @@ +/* + * 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.openjpa.persistence.query; + +import java.util.List; + +import javax.persistence.EntityManager; + +import org.apache.openjpa.persistence.test.SingleEMTestCase; + +/** + * Test for JPQL LOCATE function. + */ +public class TestLocate extends SingleEMTestCase { + + public void setUp() { + super.setUp(SimpleEntity.class, CLEAR_TABLES); + + EntityManager em1 = emf.createEntityManager(); + em1.getTransaction().begin(); + em1.persist(new SimpleEntity("foo", "bar")); + em1.getTransaction().commit(); + em1.close(); + } + + public void testLocate() { + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where LOCATE('bar', o.value) = 1").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where LOCATE('ar', o.value) = 2").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where LOCATE('zzz', o.value) = 0").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where LOCATE('ar', o.value, 1) = 2").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where LOCATE('ar', o.value, 2) = 2").getSingleResult()); + } + + public void testLocateInMemory() { + List allEntities = em.createQuery("select o from simple o", SimpleEntity.class).getResultList(); + Object inMemoryResult = em.createQuery("select LOCATE('bar', o.value) from simple o") + .setCandidateCollection(allEntities).getSingleResult(); + assertEquals(1, inMemoryResult); + inMemoryResult = em.createQuery("select LOCATE('ar', o.value, 2) from simple o") + .setCandidateCollection(allEntities).getSingleResult(); + assertEquals(2, inMemoryResult); + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java index fda12191f..19e1ba36b 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java @@ -18,8 +18,11 @@ */ package org.apache.openjpa.persistence.query; +import java.util.List; + import javax.persistence.EntityManager; +import org.apache.openjpa.jdbc.sql.PostgresDictionary; import org.apache.openjpa.persistence.test.SingleEMTestCase; public class TestSubstring extends SingleEMTestCase { @@ -28,6 +31,18 @@ public class TestSubstring extends SingleEMTestCase { super.setUp(SimpleEntity.class, CLEAR_TABLES, "openjpa.Compatibility", "JPQL=extended"); + // Expressions as substring parameters fail on PostgreSQL. + // The same problem exists with LOCATE. + // Possible fix: use CAST to integer as we do with DB2. + if ("testSubstringWithExpressionsInWhere".equals(getName())) { + setUnsupportedDatabases(PostgresDictionary.class); + if (isTestsDisabled()) { + getLog().trace(getName() + + " - Skipping test - Expressions as substring parameters fail on PostgreSQL."); + return; + } + } + EntityManager em = emf.createEntityManager(); em.getTransaction().begin(); em.persist(new SimpleEntity("foo", "bar")); @@ -49,6 +64,12 @@ public class TestSubstring extends SingleEMTestCase { "where substring(o.value, 1, 2) = 'ba'").getSingleResult()); assertEquals((long) 1, em.createQuery("select count(o) from simple o " + "where substring(o.value, 2, 2) = 'ar'").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where substring(o.value, 1) = 'bar'").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where substring(o.value, 2) = 'ar'").getSingleResult()); + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where substring(o.value, 3) = 'r'").getSingleResult()); } public void testSubstringInSelect() { @@ -58,5 +79,22 @@ public class TestSubstring extends SingleEMTestCase { "from simple o").getSingleResult()); assertEquals("r", em.createQuery("select substring(o.value, 3, 1) " + "from simple o").getSingleResult()); + assertEquals("ar", em.createQuery("select substring(o.value, 2) " + + "from simple o").getSingleResult()); + } + + public void testSubstringInMemory() { + List allEntities = em.createQuery("select o from simple o", SimpleEntity.class).getResultList(); + Object inMemoryResult = em.createQuery("select substring(o.value, 1, 1) from simple o") + .setCandidateCollection(allEntities).getSingleResult(); + assertEquals("b", inMemoryResult); + inMemoryResult = em.createQuery("select substring(o.value, 2) from simple o") + .setCandidateCollection(allEntities).getSingleResult(); + assertEquals("ar", inMemoryResult); + } + + public void testSubstringWithExpressionsInWhere() { + assertEquals((long) 1, em.createQuery("select count(o) from simple o " + + "where substring(o.value, 1+1, 1+1) = 'ar'").getSingleResult()); } } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java index 0668e7385..6fd6d6833 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java @@ -570,13 +570,10 @@ class Expressions { Value locateSearch = path.toValue(factory, q); Value locateFromIndex = (from == null ? null : Expressions.toValue(from, factory, q)); Value locatePath = Expressions.toValue(pattern, factory, q); - - return factory.add(factory.indexOf(locateSearch, - locateFromIndex == null ? locatePath - : factory.newArgumentList(locatePath, - factory.subtract(locateFromIndex, - factory.newLiteral(Integer.valueOf(1), Literal.TYPE_NUMBER)))), - factory.newLiteral(Integer.valueOf(1), Literal.TYPE_NUMBER)); + + return factory.indexOf(locateSearch, + locateFromIndex == null ? locatePath + : factory.newArgumentList(locatePath, locateFromIndex)); } public void acceptVisit(CriteriaExpressionVisitor visitor) {