From ff3d0c8a17704913d2647586415b592fa2ecc490 Mon Sep 17 00:00:00 2001 From: Jan-Willem Gmelig Meyling Date: Fri, 4 Jun 2021 00:29:48 +0200 Subject: [PATCH] Fix various issues caused by invalid constant assumptions Reverts #2354 Fixes #2326 Fixes #2816 Fixes #1413 Fixes #1429 Closes #2000 --- .github/touched-file-so-ci-runs | 0 .github/touched-file-so-ci-runs2 | 0 CHANGELOG.md | 7 ++ .../querydsl/core/support/SerializerBase.java | 103 ++++++++++-------- .../com/querydsl/jdo/AbstractJDOQuery.java | 2 +- .../com/querydsl/jdo/JDOQLSerializer.java | 20 +--- .../com/querydsl/jdo/dml/JDODeleteClause.java | 2 +- .../java/com/querydsl/jpa/JPQLSerializer.java | 20 +--- .../com/querydsl/jpa/NativeSQLSerializer.java | 12 +- .../jpa/hibernate/AbstractHibernateQuery.java | 6 +- .../jpa/hibernate/HibernateDeleteClause.java | 18 ++- .../jpa/hibernate/HibernateInsertClause.java | 3 +- .../jpa/hibernate/HibernateUpdateClause.java | 22 ++-- .../querydsl/jpa/hibernate/HibernateUtil.java | 48 ++------ .../sql/AbstractHibernateSQLQuery.java | 7 +- .../querydsl/jpa/impl/AbstractJPAQuery.java | 6 +- .../querydsl/jpa/impl/JPADeleteClause.java | 5 +- .../querydsl/jpa/impl/JPAInsertClause.java | 3 +- .../querydsl/jpa/impl/JPAUpdateClause.java | 3 +- .../java/com/querydsl/jpa/impl/JPAUtil.java | 21 ++-- .../querydsl/jpa/sql/AbstractJPASQLQuery.java | 6 +- .../java/com/querydsl/jpa/FeaturesTest.java | 2 +- .../com/querydsl/jpa/IntegrationBase.java | 3 +- .../com/querydsl/jpa/JPAIntegrationBase.java | 2 +- .../com/querydsl/jpa/JPQLSerializerTest.java | 16 +-- .../test/java/com/querydsl/jpa/MathTest.java | 6 +- .../java/com/querydsl/sql/SQLSerializer.java | 17 ++- 27 files changed, 159 insertions(+), 201 deletions(-) create mode 100644 .github/touched-file-so-ci-runs create mode 100644 .github/touched-file-so-ci-runs2 diff --git a/.github/touched-file-so-ci-runs b/.github/touched-file-so-ci-runs new file mode 100644 index 000000000..e69de29bb diff --git a/.github/touched-file-so-ci-runs2 b/.github/touched-file-so-ci-runs2 new file mode 100644 index 000000000..e69de29bb diff --git a/CHANGELOG.md b/CHANGELOG.md index 628afc3a7..c121be66a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,10 @@ A huge thanks goes out to all contributors that made this release possible in th * [#2663](https://github.com/querydsl/querydsl/issues/2663) - Fix issues with the JPA implementation of `InsertClause`. * [#2706](https://github.com/querydsl/querydsl/pull/2706) - Fix a memory leak in `TemplateFactory`. * [#2467](https://github.com/querydsl/querydsl/issues/2467) - Prevent `ExtendedBeanSerializer` from generating `toString` method twice +* [#2326](https://github.com/querydsl/querydsl/issues/2326) - Use JPA indexed parameters instead of HQL's legacy positional parameters +* [#2816](https://github.com/querydsl/querydsl/issues/2816) - Generated JPA query with incorrect argument binding indexes +* [#1413](https://github.com/querydsl/querydsl/issues/1413) - Incorrect parameter values with Hibernate custom types +* [#1429](https://github.com/querydsl/querydsl/issues/1429) - Reusing of constants in JPQL generation causes issues with hibernate query caching #### Breaking changes @@ -91,6 +95,8 @@ A huge thanks goes out to all contributors that made this release possible in th * Removal of `HibernateDomainExporter` in `querysql-jpa-codegen`. `HibernateDomainExporter` only supported Hibernate 4, which QueryDSL no longer actively supports. Instead, use the `JPADomainExporter` with Hibernate. * `ComparableExpression#coalesce` (and subtypes) no longer return a mutable `Coalesce` expression, but instead return a typed expression. If you need the Coalesce builder, use `new Coalesce().add(expression)` instead. +* `getConstantToNamedLabel`, `getConstantToNumberedLabel` and `getConstantToAllLabels` that were temporarily introduced to `SerializerBase` and `JPQLSerializer` + in QueryDSL 4.3.0 to eventually replace `getConstantToLabel` are now removed in favor of `getConstants`. #### Deprecations * `AbstractJPAQuery#fetchResults` and `AbstractJPAQuery#fetchCount` are now deprecated for queries that have multiple group by @@ -99,6 +105,7 @@ A huge thanks goes out to all contributors that made this release possible in th If you want a reliable way of computing the result count for a paginated result for even the most complicated queries, we recommend using the [Blaze-Persistence QueryDSL integration](https://persistence.blazebit.com/documentation/1.5/core/manual/en_US/#querydsl-integration). `BlazeJPAQuery` properly implements both `fetchResults` and `fetchCount` and even comes with a `page` method. +* `getConstantToLabel` which was deprecated in QueryDSL 4.3.0 is no longer deprecated. #### Dependency updates diff --git a/querydsl-core/src/main/java/com/querydsl/core/support/SerializerBase.java b/querydsl-core/src/main/java/com/querydsl/core/support/SerializerBase.java index 4247170f2..d9d04e6ef 100644 --- a/querydsl-core/src/main/java/com/querydsl/core/support/SerializerBase.java +++ b/querydsl-core/src/main/java/com/querydsl/core/support/SerializerBase.java @@ -13,20 +13,32 @@ */ package com.querydsl.core.support; +import com.querydsl.core.JoinFlag; +import com.querydsl.core.QueryFlag; +import com.querydsl.core.types.Constant; +import com.querydsl.core.types.Expression; +import com.querydsl.core.types.FactoryExpression; +import com.querydsl.core.types.Operation; +import com.querydsl.core.types.Operator; +import com.querydsl.core.types.Ops; +import com.querydsl.core.types.ParamExpression; +import com.querydsl.core.types.Path; +import com.querydsl.core.types.PathType; +import com.querydsl.core.types.Template; +import com.querydsl.core.types.TemplateExpression; +import com.querydsl.core.types.Templates; +import com.querydsl.core.types.Visitor; +import org.jetbrains.annotations.NotNull; + import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; -import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import com.querydsl.core.JoinFlag; -import com.querydsl.core.QueryFlag; -import com.querydsl.core.types.*; - -import static java.util.Collections.unmodifiableMap; - /** * {@code SerializerBase} is a stub for Serializer implementations which serialize query metadata to Strings * @@ -46,8 +58,9 @@ public abstract class SerializerBase> implements Vis private String anonParamPrefix = "_"; - private Map constantToNamedLabel; - private Map constantToNumberedLabel; + protected final List constants = new LinkedList<>(); + + protected final Map constantToLabel = new IdentityHashMap<>(); @SuppressWarnings("unchecked") private final S self = (S) this; @@ -79,35 +92,8 @@ public abstract class SerializerBase> implements Vis return constantPrefix; } - @Deprecated public Map getConstantToLabel() { - return getConstantToAllLabels(); - } - - public Map getConstantToNamedLabel() { - if (constantToNamedLabel == null) { - constantToNamedLabel = new HashMap(4); - } - return constantToNamedLabel; - } - - public Map getConstantToNumberedLabel() { - if (constantToNumberedLabel == null) { - constantToNumberedLabel = new HashMap(4); - } - return constantToNumberedLabel; - } - - public Map getConstantToAllLabels() { - Map namedLabels = getConstantToNamedLabel(); - Map numberedLabels = getConstantToNumberedLabel(); - - Map allLabels = new HashMap(namedLabels); - for (Map.Entry entry : numberedLabels.entrySet()) { - allLabels.put(entry.getKey(), entry.getValue().toString()); - } - - return unmodifiableMap(allLabels); + return constantToLabel; } protected int getLength() { @@ -223,13 +209,37 @@ public abstract class SerializerBase> implements Vis } public void visitConstant(Object constant) { - if (!getConstantToAllLabels().containsKey(constant)) { - final String constLabel = constantPrefix + (getConstantToNamedLabel().size() + 1); - getConstantToNamedLabel().put(constant, constLabel); - append(constLabel); - } else { - append(getConstantToNamedLabel().get(constant)); - } + final String constantLabel = getConstantToLabel().computeIfAbsent(constant, this::getConstantLabel); + constants.add(constant); + serializeConstant(constants.size(), constantLabel); + } + + /** + * Serialize the constant as parameter to the query. The default implementation writes the + * label name for the constants. Some dialects may replace this by indexed based or + * positional parameterization. + * Dialects may also use this to prefix the parameter with for example ":" or "?". + * + * @param parameterIndex index at which this constant occurs in {@link #getConstants()} + * @param constantLabel label under which this constant occurs in {@link #getConstantToLabel()} + */ + protected void serializeConstant(int parameterIndex, String constantLabel) { + append(constantLabel); + } + + /** + * Generate a constant value under which to register a new constant in {@link #getConstantToLabel()}. + * + * @param value the constant value or parameter to create a constant for + * @return the generated label + */ + @NotNull + protected String getConstantLabel(Object value) { + return constantPrefix + (getConstantToLabel().size() + 1); + } + + public List getConstants() { + return constants; } @Override @@ -240,8 +250,9 @@ public abstract class SerializerBase> implements Vis } else { paramLabel = paramPrefix + param.getName(); } - getConstantToNamedLabel().put(param, paramLabel); - append(paramLabel); + getConstantToLabel().put(param, paramLabel); + constants.add(param); + serializeConstant(constants.size(), paramLabel); return null; } diff --git a/querydsl-jdo/src/main/java/com/querydsl/jdo/AbstractJDOQuery.java b/querydsl-jdo/src/main/java/com/querydsl/jdo/AbstractJDOQuery.java index 5cc8885ce..5a1d74904 100644 --- a/querydsl-jdo/src/main/java/com/querydsl/jdo/AbstractJDOQuery.java +++ b/querydsl-jdo/src/main/java/com/querydsl/jdo/AbstractJDOQuery.java @@ -135,7 +135,7 @@ public abstract class AbstractJDOQuery> exte JDOQLSerializer serializer = new JDOQLSerializer(getTemplates(), source); serializer.serialize(queryMixin.getMetadata(), forCount, false); - logQuery(serializer.toString(), serializer.getConstantToAllLabels()); + logQuery(serializer.toString(), serializer.getConstantToLabel()); // create Query Query query = persistenceManager.newQuery(serializer.toString()); diff --git a/querydsl-jdo/src/main/java/com/querydsl/jdo/JDOQLSerializer.java b/querydsl-jdo/src/main/java/com/querydsl/jdo/JDOQLSerializer.java index e5742e943..971509cc1 100644 --- a/querydsl-jdo/src/main/java/com/querydsl/jdo/JDOQLSerializer.java +++ b/querydsl-jdo/src/main/java/com/querydsl/jdo/JDOQLSerializer.java @@ -79,29 +79,20 @@ public final class JDOQLSerializer extends SerializerBase { public JDOQLSerializer(JDOQLTemplates templates, Expression candidate) { super(templates); this.candidatePath = candidate; - this.constantToLabel.push(new HashMap()); + this.constantToLabel.push(new LinkedHashMap<>()); } public Expression getCandidatePath() { return candidatePath; } + @Override public List getConstants() { return constants; } @Override public Map getConstantToLabel() { - return getConstantToAllLabels(); - } - - @Override - public Map getConstantToAllLabels() { - return constantToLabel.peek(); - } - - @Override - public Map getConstantToNamedLabel() { return constantToLabel.peek(); } @@ -114,7 +105,7 @@ public final class JDOQLSerializer extends SerializerBase { final Predicate having = metadata.getHaving(); final List> orderBy = metadata.getOrderBy(); - constantToLabel.push(new HashMap()); + constantToLabel.push(new LinkedHashMap()); // select boolean skippedSelect = false; @@ -197,7 +188,7 @@ public final class JDOQLSerializer extends SerializerBase { } // parameters - if (!getConstantToAllLabels().isEmpty()) { + if (!getConstantToLabel().isEmpty()) { insert(position, serializeParameters(metadata.getParams())); } @@ -222,8 +213,7 @@ public final class JDOQLSerializer extends SerializerBase { final StringBuilder b = new StringBuilder(); b.append(PARAMETERS); boolean first = true; - final List> entries = new ArrayList>(getConstantToAllLabels().entrySet()); - entries.sort(comparator); + final Collection> entries = getConstantToLabel().entrySet(); for (Map.Entry entry : entries) { if (!first) { b.append(COMMA); diff --git a/querydsl-jdo/src/main/java/com/querydsl/jdo/dml/JDODeleteClause.java b/querydsl-jdo/src/main/java/com/querydsl/jdo/dml/JDODeleteClause.java index 40e92339c..9beb6ccf6 100644 --- a/querydsl-jdo/src/main/java/com/querydsl/jdo/dml/JDODeleteClause.java +++ b/querydsl-jdo/src/main/java/com/querydsl/jdo/dml/JDODeleteClause.java @@ -64,7 +64,7 @@ public class JDODeleteClause implements DeleteClause { JDOQLSerializer serializer = new JDOQLSerializer(templates, entity); serializer.handle(metadata.getWhere()); query.setFilter(serializer.toString()); - Map constToLabel = serializer.getConstantToAllLabels(); + Map constToLabel = serializer.getConstantToLabel(); try { if (!constToLabel.isEmpty()) { diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/JPQLSerializer.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/JPQLSerializer.java index 4be9c0e3d..06c0a1c75 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/JPQLSerializer.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/JPQLSerializer.java @@ -379,14 +379,7 @@ public class JPQLSerializer extends SerializerBase { if (wrap) { append("("); } - append("?"); - if (!getConstantToAllLabels().containsKey(constant)) { - Integer constLabel = getConstantToNumberedLabel().size() + 1; - getConstantToNumberedLabel().put(constant, constLabel); - append(constLabel.toString()); - } else { - append(getConstantToAllLabels().get(constant)); - } + super.visitConstant(constant); if (wrap) { append(")"); } @@ -398,16 +391,9 @@ public class JPQLSerializer extends SerializerBase { } @Override - public Void visit(ParamExpression param, Void context) { + protected void serializeConstant(int parameterIndex, String constantLabel) { append("?"); - if (!getConstantToAllLabels().containsKey(param)) { - final Integer paramLabel = getConstantToNumberedLabel().size() + 1; - getConstantToNumberedLabel().put(param, paramLabel); - append(paramLabel.toString()); - } else { - append(getConstantToAllLabels().get(param)); - } - return null; + append(Integer.toString(parameterIndex)); } @Override diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/NativeSQLSerializer.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/NativeSQLSerializer.java index 9452f0bce..6f2f24a57 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/NativeSQLSerializer.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/NativeSQLSerializer.java @@ -197,15 +197,17 @@ public final class NativeSQLSerializer extends SQLSerializer { first = false; } append(")"); - } else if (!getConstantToAllLabels().containsKey(constant)) { - final Integer constLabel = getConstantToNumberedLabel().size() + 1; - getConstantToNumberedLabel().put(constant, constLabel); - append("?" + constLabel); } else { - append("?" + getConstantToAllLabels().get(constant)); + super.visitConstant(constant); } } + @Override + protected void serializeConstant(int parameterIndex, String constantLabel) { + append("?"); + append(Integer.toString(parameterIndex)); + } + @Override protected void visitOperation(Class type, Operator operator, List> args) { if (operator == SQLOps.ALL diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/AbstractHibernateQuery.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/AbstractHibernateQuery.java index 8f0fed6c8..203944717 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/AbstractHibernateQuery.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/AbstractHibernateQuery.java @@ -100,9 +100,9 @@ public abstract class AbstractHibernateQuery 0) { query.setFetchSize(fetchSize); @@ -208,7 +208,7 @@ public abstract class AbstractHibernateQuery parameters) { + protected void logQuery(String queryString) { if (logger.isLoggable(Level.FINE)) { String normalizedQuery = queryString.replace('\n', ' '); logger.fine(normalizedQuery); diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateDeleteClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateDeleteClause.java index fea23136e..efc563fec 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateDeleteClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateDeleteClause.java @@ -13,14 +13,6 @@ */ package com.querydsl.jpa.hibernate; -import java.util.HashMap; -import java.util.Map; - -import org.hibernate.LockMode; -import org.hibernate.query.Query; -import org.hibernate.Session; -import org.hibernate.StatelessSession; - import com.querydsl.core.JoinType; import com.querydsl.core.dml.DeleteClause; import com.querydsl.core.support.QueryMixin; @@ -31,6 +23,13 @@ import com.querydsl.jpa.HQLTemplates; import com.querydsl.jpa.JPAQueryMixin; import com.querydsl.jpa.JPQLSerializer; import com.querydsl.jpa.JPQLTemplates; +import org.hibernate.LockMode; +import org.hibernate.Session; +import org.hibernate.StatelessSession; +import org.hibernate.query.Query; + +import java.util.HashMap; +import java.util.Map; /** * DeleteClause implementation for Hibernate @@ -75,8 +74,7 @@ public class HibernateDeleteClause implements DeleteClause, LockMode> entry : lockModes.entrySet()) { query.setLockMode(entry.getKey().toString(), entry.getValue()); } - HibernateUtil.setConstants(query, serializer.getConstantToNamedLabel(), serializer.getConstantToNumberedLabel(), - queryMixin.getMetadata().getParams()); + HibernateUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateInsertClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateInsertClause.java index 85a61c30b..9a9dd9af6 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateInsertClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateInsertClause.java @@ -86,13 +86,12 @@ public class HibernateInsertClause implements public long execute() { JPQLSerializer serializer = new JPQLSerializer(templates, null); serializer.serializeForInsert(queryMixin.getMetadata(), inserts.isEmpty() ? columns : inserts.keySet(), values, subQuery, inserts); - Map constants = serializer.getConstantToLabel(); Query query = session.createQuery(serializer.toString()); for (Map.Entry, LockMode> entry : lockModes.entrySet()) { query.setLockMode(entry.getKey().toString(), entry.getValue()); } - HibernateUtil.setConstants(query, constants, queryMixin.getMetadata().getParams()); + HibernateUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUpdateClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUpdateClause.java index 5909bd16e..5e8a951f3 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUpdateClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUpdateClause.java @@ -13,16 +13,6 @@ */ package com.querydsl.jpa.hibernate; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; - -import org.hibernate.LockMode; -import org.hibernate.query.Query; -import org.hibernate.Session; -import org.hibernate.StatelessSession; - import com.querydsl.core.JoinType; import com.querydsl.core.dml.UpdateClause; import com.querydsl.core.support.QueryMixin; @@ -35,6 +25,15 @@ import com.querydsl.jpa.HQLTemplates; import com.querydsl.jpa.JPAQueryMixin; import com.querydsl.jpa.JPQLSerializer; import com.querydsl.jpa.JPQLTemplates; +import org.hibernate.LockMode; +import org.hibernate.Session; +import org.hibernate.StatelessSession; +import org.hibernate.query.Query; + +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; /** * UpdateClause implementation for Hibernate @@ -83,8 +82,7 @@ public class HibernateUpdateClause implements for (Map.Entry, LockMode> entry : lockModes.entrySet()) { query.setLockMode(entry.getKey().toString(), entry.getValue()); } - HibernateUtil.setConstants(query, serializer.getConstantToNamedLabel(), serializer.getConstantToNumberedLabel(), - queryMixin.getMetadata().getParams()); + HibernateUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUtil.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUtil.java index d35c321b7..7a1adc427 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUtil.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/HibernateUtil.java @@ -17,7 +17,6 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.util.*; -import org.hibernate.query.Query; import org.hibernate.type.*; import com.querydsl.core.types.ParamExpression; @@ -61,47 +60,26 @@ public final class HibernateUtil { } public static void setConstants( - Query query, - Map namedConstants, + org.hibernate.query.Query query, + List constants, Map, Object> params ) { - setConstants(query, namedConstants, new HashMap(), params); - } + for (int i = 0; i < constants.size(); i++) { + Object val = constants.get(i); - public static void setConstants( - Query query, - Map namedConstants, - Map numberedConstants, - Map, Object> params - ) { - for (Map.Entry entry : namedConstants.entrySet()) { - String key = entry.getValue(); - Object val = entry.getKey(); if (val instanceof Param) { + Param param = (Param) val; val = params.get(val); if (val == null) { - throw new ParamNotSetException((Param) entry.getKey()); + throw new ParamNotSetException(param); } } - setValueWithNamedLabel(query, key, val); - } - - for (Map.Entry entry : numberedConstants.entrySet()) { - Integer key = entry.getValue(); - Object val = entry.getKey(); - if (val instanceof Param) { - val = params.get(val); - if (val == null) { - throw new ParamNotSetException((Param) entry.getKey()); - } - } - - setValueWithNumberedLabel(query, key, val); + setValueWithNumberedLabel(query, i + 1, val); } } - private static void setValueWithNamedLabel(Query query, String key, Object val) { + private static void setValueWithNumberedLabel(org.hibernate.query.Query query, Integer key, Object val) { if (val instanceof Collection) { query.setParameterList(key, (Collection) val); } else if (val instanceof Object[] && !BUILT_IN.contains(val.getClass())) { @@ -113,16 +91,8 @@ public final class HibernateUtil { } } - private static void setValueWithNumberedLabel(Query query, Integer key, Object val) { - if (val instanceof Number && TYPES.containsKey(val.getClass())) { - query.setParameter(key, val, getType(val.getClass())); - } else { - query.setParameter(key, val); - } - } - - public static Type getType(Class clazz) { return TYPES.get(clazz); } + } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/sql/AbstractHibernateSQLQuery.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/sql/AbstractHibernateSQLQuery.java index 69664afb4..ccb1bca5e 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/sql/AbstractHibernateSQLQuery.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/hibernate/sql/AbstractHibernateSQLQuery.java @@ -84,11 +84,10 @@ public abstract class AbstractHibernateSQLQuery, List> aliases = serializer.getAliases(); @@ -201,7 +200,7 @@ public abstract class AbstractHibernateSQLQuery parameters) { + protected void logQuery(String queryString) { if (logger.isLoggable(Level.FINE)) { String normalizedQuery = queryString.replace('\n', ' '); logger.fine(normalizedQuery); diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/AbstractJPAQuery.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/AbstractJPAQuery.java index 6961f54b2..0305391fd 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/AbstractJPAQuery.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/AbstractJPAQuery.java @@ -128,9 +128,9 @@ public abstract class AbstractJPAQuery> exte protected Query createQuery(@Nullable QueryModifiers modifiers, boolean forCount) { JPQLSerializer serializer = serialize(forCount); String queryString = serializer.toString(); - logQuery(queryString, serializer.getConstantToAllLabels()); + logQuery(queryString); Query query = entityManager.createQuery(queryString); - JPAUtil.setConstants(query, serializer.getConstantToAllLabels(), getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), getMetadata().getParams()); if (modifiers != null && modifiers.isRestricting()) { Integer limit = modifiers.getLimitAsInteger(); Integer offset = modifiers.getOffsetAsInteger(); @@ -306,7 +306,7 @@ public abstract class AbstractJPAQuery> exte } - protected void logQuery(String queryString, Map parameters) { + protected void logQuery(String queryString) { if (logger.isLoggable(Level.FINEST)) { String normalizedQuery = queryString.replace('\n', ' '); logger.finest(normalizedQuery); diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPADeleteClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPADeleteClause.java index bc7c9e8eb..ad4de0af2 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPADeleteClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPADeleteClause.java @@ -13,8 +13,6 @@ */ package com.querydsl.jpa.impl; -import java.util.Map; - import org.jetbrains.annotations.Nullable; import javax.persistence.EntityManager; import javax.persistence.LockModeType; @@ -60,13 +58,12 @@ public class JPADeleteClause implements DeleteClause { public long execute() { JPQLSerializer serializer = new JPQLSerializer(templates, entityManager); serializer.serializeForDelete(queryMixin.getMetadata()); - Map constants = serializer.getConstantToAllLabels(); Query query = entityManager.createQuery(serializer.toString()); if (lockMode != null) { query.setLockMode(lockMode); } - JPAUtil.setConstants(query, constants, queryMixin.getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAInsertClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAInsertClause.java index 8f4888149..2c5d6dc9a 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAInsertClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAInsertClause.java @@ -75,13 +75,12 @@ public class JPAInsertClause implements InsertClause { public long execute() { JPQLSerializer serializer = new JPQLSerializer(templates, entityManager); serializer.serializeForInsert(queryMixin.getMetadata(), inserts.isEmpty() ? columns : inserts.keySet(), values, subQuery, inserts); - Map constants = serializer.getConstantToLabel(); Query query = entityManager.createQuery(serializer.toString()); if (lockMode != null) { query.setLockMode(lockMode); } - JPAUtil.setConstants(query, constants, queryMixin.getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUpdateClause.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUpdateClause.java index 686b5d5ae..7e508867c 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUpdateClause.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUpdateClause.java @@ -67,13 +67,12 @@ public class JPAUpdateClause implements UpdateClause { public long execute() { JPQLSerializer serializer = new JPQLSerializer(templates, entityManager); serializer.serializeForUpdate(queryMixin.getMetadata(), updates); - Map constants = serializer.getConstantToAllLabels(); Query query = entityManager.createQuery(serializer.toString()); if (lockMode != null) { query.setLockMode(lockMode); } - JPAUtil.setConstants(query, constants, queryMixin.getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); return query.executeUpdate(); } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUtil.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUtil.java index 8ac4e8141..36b26c13f 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUtil.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/impl/JPAUtil.java @@ -13,6 +13,7 @@ */ package com.querydsl.jpa.impl; +import java.util.List; import java.util.Map; import javax.persistence.Parameter; @@ -33,19 +34,22 @@ public final class JPAUtil { private JPAUtil() { } - public static void setConstants(Query query, Map constants, Map, Object> params) { + public static void setConstants(Query query, List constants, Map, Object> params) { boolean hasParameters = !query.getParameters().isEmpty(); - for (Map.Entry entry : constants.entrySet()) { - String key = entry.getValue(); - Object val = entry.getKey(); - if (Param.class.isInstance(val)) { + + for (int i = 0; i < constants.size(); i++) { + Object val = constants.get(i); + + if (val instanceof Param) { + Param param = (Param) val; val = params.get(val); if (val == null) { - throw new ParamNotSetException((Param) entry.getKey()); + throw new ParamNotSetException(param); } } + if (hasParameters) { - Parameter parameter = query.getParameter(Integer.parseInt(key)); + Parameter parameter = query.getParameter(i + 1); Class parameterType = parameter != null ? parameter.getParameterType() : null; if (parameterType != null && !parameterType.isInstance(val)) { if (val instanceof Number && Number.class.isAssignableFrom(parameterType)) { @@ -53,7 +57,8 @@ public final class JPAUtil { } } } - query.setParameter(Integer.parseInt(key), val); + + query.setParameter(i + 1, val); } } diff --git a/querydsl-jpa/src/main/java/com/querydsl/jpa/sql/AbstractJPASQLQuery.java b/querydsl-jpa/src/main/java/com/querydsl/jpa/sql/AbstractJPASQLQuery.java index 9e89d2e31..1431d54e7 100644 --- a/querydsl-jpa/src/main/java/com/querydsl/jpa/sql/AbstractJPASQLQuery.java +++ b/querydsl-jpa/src/main/java/com/querydsl/jpa/sql/AbstractJPASQLQuery.java @@ -94,7 +94,7 @@ public abstract class AbstractJPASQLQuery private Query createQuery(boolean forCount) { NativeSQLSerializer serializer = (NativeSQLSerializer) serialize(forCount); String queryString = serializer.toString(); - logQuery(queryString, serializer.getConstantToAllLabels()); + logQuery(queryString); Expression projection = queryMixin.getMetadata().getProjection(); Query query; @@ -152,7 +152,7 @@ public abstract class AbstractJPASQLQuery // set constants - JPAUtil.setConstants(query, serializer.getConstantToAllLabels(), queryMixin.getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), queryMixin.getMetadata().getParams()); this.projection = null; // necessary when query is reused if (!forCount && projection instanceof FactoryExpression) { @@ -280,7 +280,7 @@ public abstract class AbstractJPASQLQuery } - protected void logQuery(String queryString, Map parameters) { + protected void logQuery(String queryString) { if (logger.isLoggable(Level.FINE)) { String normalizedQuery = queryString.replace('\n', ' '); logger.fine(normalizedQuery); diff --git a/querydsl-jpa/src/test/java/com/querydsl/jpa/FeaturesTest.java b/querydsl-jpa/src/test/java/com/querydsl/jpa/FeaturesTest.java index cc8891d6e..bac490f86 100644 --- a/querydsl-jpa/src/test/java/com/querydsl/jpa/FeaturesTest.java +++ b/querydsl-jpa/src/test/java/com/querydsl/jpa/FeaturesTest.java @@ -57,7 +57,7 @@ public class FeaturesTest extends AbstractQueryTest { public void argumentHandling() { // Kitty is reused, so it should be used via one named parameter assertToString( - "cat.name = ?1 or cust.name.firstName = ?2 or kitten.name = ?1", + "cat.name = ?1 or cust.name.firstName = ?2 or kitten.name = ?3", cat.name.eq("Kitty").or(cust.name.firstName.eq("Hans")).or(kitten.name.eq("Kitty"))); } diff --git a/querydsl-jpa/src/test/java/com/querydsl/jpa/IntegrationBase.java b/querydsl-jpa/src/test/java/com/querydsl/jpa/IntegrationBase.java index 79355131b..6c3c04847 100644 --- a/querydsl-jpa/src/test/java/com/querydsl/jpa/IntegrationBase.java +++ b/querydsl-jpa/src/test/java/com/querydsl/jpa/IntegrationBase.java @@ -55,8 +55,7 @@ public class IntegrationBase extends ParsingTest implements HibernateTest { JPQLSerializer serializer = new JPQLSerializer(HQLTemplates.DEFAULT); serializer.serialize(getMetadata(), false, null); Query query = session.createQuery(serializer.toString()); - HibernateUtil.setConstants(query, serializer.getConstantToNamedLabel(), - serializer.getConstantToNumberedLabel(), getMetadata().getParams()); + HibernateUtil.setConstants(query, serializer.getConstants(), getMetadata().getParams()); query.list(); } catch (Exception e) { e.printStackTrace(); diff --git a/querydsl-jpa/src/test/java/com/querydsl/jpa/JPAIntegrationBase.java b/querydsl-jpa/src/test/java/com/querydsl/jpa/JPAIntegrationBase.java index e63d20733..44efa70f2 100644 --- a/querydsl-jpa/src/test/java/com/querydsl/jpa/JPAIntegrationBase.java +++ b/querydsl-jpa/src/test/java/com/querydsl/jpa/JPAIntegrationBase.java @@ -51,7 +51,7 @@ public class JPAIntegrationBase extends ParsingTest implements JPATest { JPQLSerializer serializer = new JPQLSerializer(templates); serializer.serialize(getMetadata(), false, null); Query query = em.createQuery(serializer.toString()); - JPAUtil.setConstants(query, serializer.getConstantToAllLabels(), getMetadata().getParams()); + JPAUtil.setConstants(query, serializer.getConstants(), getMetadata().getParams()); try { query.getResultList(); } catch (Exception e) { diff --git a/querydsl-jpa/src/test/java/com/querydsl/jpa/JPQLSerializerTest.java b/querydsl-jpa/src/test/java/com/querydsl/jpa/JPQLSerializerTest.java index 2cf3b002c..e50a621f4 100644 --- a/querydsl-jpa/src/test/java/com/querydsl/jpa/JPQLSerializerTest.java +++ b/querydsl-jpa/src/test/java/com/querydsl/jpa/JPQLSerializerTest.java @@ -58,7 +58,7 @@ public class JPQLSerializerTest { .when(cat.toes.eq(3)).then(3) .otherwise(4); serializer.handle(expr); - assertEquals("case when (cat.toes = ?1) then ?1 when (cat.toes = ?2) then ?2 else ?3 end", serializer.toString()); + assertEquals("case when (cat.toes = ?1) then ?2 when (cat.toes = ?3) then ?4 else ?5 end", serializer.toString()); } @Test @@ -69,7 +69,7 @@ public class JPQLSerializerTest { .when(cat.toes.eq(3)).then(3) .otherwise(4); serializer.handle(expr); - assertEquals("case when (cat.toes = ?1) then ?1 when (cat.toes = ?2) then ?2 else 4 end", serializer.toString()); + assertEquals("case when (cat.toes = ?1) then ?2 when (cat.toes = ?3) then ?4 else 4 end", serializer.toString()); } @Test @@ -80,7 +80,7 @@ public class JPQLSerializerTest { .when(cat.toes.eq(3)).then(cat.id.multiply(3)) .otherwise(4); serializer.handle(expr); - assertEquals("case when (cat.toes = ?1) then (cat.id * ?1) when (cat.toes = ?2) then (cat.id * ?2) else ?3 end", serializer.toString()); + assertEquals("case when (cat.toes = ?1) then (cat.id * ?2) when (cat.toes = ?3) then (cat.id * ?4) else ?5 end", serializer.toString()); } @Test @@ -91,7 +91,7 @@ public class JPQLSerializerTest { .when(cat.toes.eq(3)).then(cat.id.multiply(3)) .otherwise(4); serializer.handle(expr); - assertEquals("case when (cat.toes = ?1) then (cat.id * ?1) when (cat.toes = ?2) then (cat.id * ?2) else 4 end", serializer.toString()); + assertEquals("case when (cat.toes = ?1) then (cat.id * ?2) when (cat.toes = ?3) then (cat.id * ?4) else 4 end", serializer.toString()); } @Test @@ -140,7 +140,7 @@ public class JPQLSerializerTest { serializer.handle(doublePath.add(1)); serializer.handle(doublePath.between((float) 1.0, 1L)); serializer.handle(doublePath.lt((byte) 1)); - for (Object constant : serializer.getConstantToAllLabels().keySet()) { + for (Object constant : serializer.getConstants()) { assertEquals(Double.class, constant.getClass()); } } @@ -194,7 +194,7 @@ public class JPQLSerializerTest { JPQLSerializer serializer = new JPQLSerializer(HQLTemplates.DEFAULT); serializer.handle(Expressions.stringPath("str").contains("abc!")); assertEquals("str like ?1 escape '!'", serializer.toString()); - assertEquals("%abc!!%", serializer.getConstantToAllLabels().keySet().iterator().next().toString()); + assertEquals("%abc!!%", serializer.getConstants().get(0).toString()); } @Test @@ -202,7 +202,7 @@ public class JPQLSerializerTest { JPQLSerializer serializer = new JPQLSerializer(HQLTemplates.DEFAULT); serializer.handle(Expressions.stringPath("str").containsIgnoreCase("ABc!")); assertEquals("lower(str) like ?1 escape '!'", serializer.toString()); - assertEquals("%abc!!%", serializer.getConstantToAllLabels().keySet().iterator().next().toString()); + assertEquals("%abc!!%", serializer.getConstants().get(0).toString()); } @Test @@ -210,7 +210,7 @@ public class JPQLSerializerTest { JPQLSerializer serializer = new JPQLSerializer(HQLTemplates.DEFAULT); QCat cat = QCat.cat; serializer.handle(cat.name.substring(cat.name.length().subtract(1), 1)); - assertEquals("substring(cat.name,length(cat.name) + ?1,?2 - (length(cat.name) - ?2))", serializer.toString()); + assertEquals("substring(cat.name,length(cat.name) + ?1,?2 - (length(cat.name) - ?3))", serializer.toString()); } @Test diff --git a/querydsl-jpa/src/test/java/com/querydsl/jpa/MathTest.java b/querydsl-jpa/src/test/java/com/querydsl/jpa/MathTest.java index 49cf29fc1..3fdcf8fc7 100644 --- a/querydsl-jpa/src/test/java/com/querydsl/jpa/MathTest.java +++ b/querydsl-jpa/src/test/java/com/querydsl/jpa/MathTest.java @@ -50,17 +50,17 @@ public class MathTest extends AbstractQueryTest { @Test public void add_and_compare() { - assertToString("cat.bodyWeight + ?1 < ?1", cat.bodyWeight.add(10.0).lt(10.0)); + assertToString("cat.bodyWeight + ?1 < ?2", cat.bodyWeight.add(10.0).lt(10.0)); } @Test public void subtract_and_compare() { - assertToString("cat.bodyWeight - ?1 < ?1", cat.bodyWeight.subtract(10.0).lt(10.0)); + assertToString("cat.bodyWeight - ?1 < ?2", cat.bodyWeight.subtract(10.0).lt(10.0)); } @Test public void multiply_and_compare() { - assertToString("cat.bodyWeight * ?1 < ?1", cat.bodyWeight.multiply(10.0).lt(10.0)); + assertToString("cat.bodyWeight * ?1 < ?2", cat.bodyWeight.multiply(10.0).lt(10.0)); } @Test diff --git a/querydsl-sql/src/main/java/com/querydsl/sql/SQLSerializer.java b/querydsl-sql/src/main/java/com/querydsl/sql/SQLSerializer.java index 38d64d3a6..018d37857 100644 --- a/querydsl-sql/src/main/java/com/querydsl/sql/SQLSerializer.java +++ b/querydsl-sql/src/main/java/com/querydsl/sql/SQLSerializer.java @@ -47,8 +47,6 @@ public class SQLSerializer extends SerializerBase { protected final LinkedList> constantPaths = new LinkedList>(); - protected final List constants = new ArrayList(); - protected final Set> withAliases = new HashSet<>(); protected final boolean dml; @@ -105,10 +103,6 @@ public class SQLSerializer extends SerializerBase { append(templates.quoteIdentifier(table, precededByDot)); } - public List getConstants() { - return constants; - } - public List> getConstantPaths() { return constantPaths; } @@ -801,7 +795,7 @@ public class SQLSerializer extends SerializerBase { if (!first) { append(COMMA); } - append("?"); + serializeConstant(constants.size() + 1, null); constants.add(o); if (first && (constantPaths.size() < constants.size())) { constantPaths.add(null); @@ -823,7 +817,7 @@ public class SQLSerializer extends SerializerBase { Expression type = Expressions.constant(typeName); super.visitOperation(constant.getClass(), SQLOps.CAST, Arrays.> asList(Q, type)); } else { - append("?"); + serializeConstant(constants.size() + 1, null); } constants.add(constant); if (constantPaths.size() < constants.size()) { @@ -834,14 +828,19 @@ public class SQLSerializer extends SerializerBase { @Override public Void visit(ParamExpression param, Void context) { - append("?"); constants.add(param); + serializeConstant(constants.size(), null); if (constantPaths.size() < constants.size()) { constantPaths.add(null); } return null; } + @Override + protected void serializeConstant(int parameterIndex, String constantLabel) { + append("?"); + } + @Override public Void visit(Path path, Void context) { if (dml) {