This is an automated email from the ASF dual-hosted git repository. struberg pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit 0e4ec5b392b978c4515b26c60e485f2b610de94f Author: Mark Struberg <[email protected]> AuthorDate: Thu Feb 14 15:06:50 2019 +0100 OPENJPA-2733 fix param index. Also handle the case if the same Param gets registered multiple times. This eg happens in case of a Criteria Subquery having the same parameter name. --- .../java/org/apache/openjpa/kernel/QueryImpl.java | 4 +- .../apache/openjpa/persistence/criteria/Order.java | 18 +++-- .../persistence/criteria/TestSubqueries.java | 92 ++++++++++++++++------ .../persistence/criteria/CriteriaBuilderImpl.java | 2 +- .../persistence/criteria/CriteriaQueryImpl.java | 30 ++++--- .../criteria/ParameterExpressionImpl.java | 41 +++++++++- .../openjpa/persistence/criteria/SubqueryImpl.java | 3 +- 7 files changed, 139 insertions(+), 51 deletions(-) diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java index 0521a1a..cb50be1 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java @@ -919,7 +919,7 @@ public class QueryImpl implements Query { throw ke; } catch (Exception e) { throw new UserException(_loc.get("query-execution-error", - _query), e); + _query), e); } finally { _broker.endOperation(); } @@ -1306,7 +1306,7 @@ public class QueryImpl implements Query { boolean detach = (_broker.getAutoDetach() & AutoDetach.DETACH_NONTXREAD) > 0 && !_broker.isActive(); boolean lrs = range.lrs && !ex.isAggregate(q) && !ex.hasGrouping(q); - ResultList<?> res = new ListResultList(Collections.emptyList()); + ResultList<?> res; try { res = (!detach && lrs) ? _fc.newResultList(rop) : new EagerResultList(rop); res.setUserObject(new Object[]{rop,ex}); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/Order.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/Order.java index 18758ae..ef1b72f 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/Order.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/Order.java @@ -37,16 +37,18 @@ public class Order { private int id; private int quantity; - private double totalCost; - @Column(name="CNT") - private int count; - private String name; + private double totalCost; - @ManyToOne - private Customer customer; + @Column(name="CNT") + private int count; - @OneToMany(mappedBy="order") - private List<LineItem> lineItems; + private String name; + + @ManyToOne + private Customer customer; + + @OneToMany(mappedBy="order") + private List<LineItem> lineItems; private boolean delivered; diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestSubqueries.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestSubqueries.java index f4e8adb..b2547a8 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestSubqueries.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestSubqueries.java @@ -19,6 +19,7 @@ package org.apache.openjpa.persistence.criteria; import java.sql.Timestamp; +import java.util.List; import javax.persistence.Parameter; import javax.persistence.Tuple; @@ -582,41 +583,52 @@ public class TestSubqueries extends CriteriaTest { } public void testSubquery24() { - em.getTransaction().begin(); - em.createQuery("delete from Order o where o.customer.name = 'Capricorn'").executeUpdate(); - em.createQuery("delete from Order o").executeUpdate(); - em.createQuery("delete from Customer c where c.name = 'Capricorn'").executeUpdate(); + freshCustomerAndOrder(); - em.flush(); + CriteriaQuery<Long> q = cb.createQuery(Long.class); + Root<Customer> root = q.from(Customer.class); + q.select(root.get(Customer_.accountNum)); - Customer c1 = new Customer(); - c1.setAccountNum(156); - c1.setFirstName("John"); - c1.setLastName("Doe"); - c1.setName("Capricorn"); - em.persist(c1); + ParameterExpression<String> testParam = cb.parameter(String.class, "param1"); - Order o1 = new Order(); - o1.setCustomer(c1); - em.persist(o1); - o1 = new Order(); - o1.setCustomer(c1); - em.persist(o1); + Subquery<Customer> sq = q.subquery(Customer.class); + Root<Order> sqRoot = sq.from(Order.class); + sq.where(cb.and( + cb.equal(cb.parameter(String.class, "param2"), sqRoot.get(Order_.customer).get(Customer_.lastName)), + cb.equal(testParam, sqRoot.get(Order_.customer).get(Customer_.name)) + )); + sq.select(sqRoot.get(Order_.customer)); - em.flush(); + q.where(cb.and( + cb.equal(testParam, root.get(Customer_.name)), + cb.in(root).value(sq) + )); - // em.getTransaction().commit(); + // em.createQuery(q).getResultList(); + TypedQuery<Long> tq = em.createQuery(q); + tq.setParameter("param1", "Capricorn"); + tq.setParameter("param2", "Doe"); + + assertEquals(1, tq.getResultList().size()); + + em.getTransaction().rollback(); + + cleanCustomerAndOrder(); + } + + public void testSubquery25() { + em.getTransaction().begin(); - // System.out.println("CUSTOMERS: "+em.createQuery("select count(c) from Customer c").getFirstResult()); - // System.out.println("ORDERS: "+em.createQuery("select count(c) from Order c").getFirstResult()); + freshCustomerAndOrder(); CriteriaQuery<Long> q = cb.createQuery(Long.class); Root<Customer> root = q.from(Customer.class); q.select(root.get(Customer_.accountNum)); ParameterExpression<String> testParam = cb.parameter(String.class, "param1"); + ParameterExpression<String> testParam2 = cb.parameter(String.class, "param1"); Subquery<Customer> sq = q.subquery(Customer.class); Root<Order> sqRoot = sq.from(Order.class); @@ -627,7 +639,7 @@ public class TestSubqueries extends CriteriaTest { sq.select(sqRoot.get(Order_.customer)); q.where(cb.and( - cb.equal(testParam, root.get(Customer_.name)), + cb.equal(testParam2, root.get(Customer_.name)), cb.in(root).value(sq) )); @@ -640,6 +652,42 @@ public class TestSubqueries extends CriteriaTest { em.getTransaction().rollback(); + cleanCustomerAndOrder(); + } + + private void freshCustomerAndOrder() { + cleanCustomerAndOrder(); + + Customer c1 = new Customer(); + c1.setAccountNum(156); + c1.setFirstName("John"); + c1.setLastName("Doe"); + c1.setName("Capricorn"); + em.persist(c1); + + Order o1 = new Order(); + o1.setCustomer(c1); + em.persist(o1); + o1 = new Order(); + o1.setCustomer(c1); + em.persist(o1); + + em.flush(); + } + + private void cleanCustomerAndOrder() { + boolean txActive = em.getTransaction().isActive(); + if (!txActive) { + em.getTransaction().begin(); + } + em.createQuery("delete from Order o where o.customer.name = 'Capricorn'").executeUpdate(); + em.createQuery("delete from Order o").executeUpdate(); + em.createQuery("delete from Customer c where c.name = 'Capricorn'").executeUpdate(); + + em.flush(); + if (!txActive) { + em.getTransaction().commit(); + } } } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java index a3e54ed..968b78d 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java @@ -674,7 +674,7 @@ public class CriteriaBuilderImpl implements OpenJPACriteriaBuilder, ExpressionPa @Override public Predicate or(Expression<Boolean> x, Expression<Boolean> y) { - return new PredicateImpl.Or(x,y); + return new PredicateImpl.Or(x,y); } /** diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaQueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaQueryImpl.java index 7ea8a3f..778ca72 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaQueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaQueryImpl.java @@ -21,7 +21,6 @@ package org.apache.openjpa.persistence.criteria; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; @@ -44,7 +43,6 @@ import javax.persistence.criteria.Selection; import javax.persistence.criteria.Subquery; import javax.persistence.metamodel.EntityType; -import org.apache.openjpa.kernel.StoreQuery; import org.apache.openjpa.kernel.exps.Context; import org.apache.openjpa.kernel.exps.ExpressionFactory; import org.apache.openjpa.kernel.exps.QueryExpressions; @@ -74,7 +72,7 @@ class CriteriaQueryImpl<T> implements OpenJPACriteriaQuery<T>, AliasContext { private Set<Root<?>> _roots; private PredicateImpl _where; private List<Order> _orders; - private OrderedMap<Object, Class<?>> _params; /*<ParameterExpression<?>, Class<?>>*/ + private OrderedMap<Object, Class<?>> _params = new OrderedMap<>(); private Selection<? extends T> _selection; private List<Selection<?>> _selections; private List<Expression<?>> _groups; @@ -115,14 +113,12 @@ class CriteriaQueryImpl<T> implements OpenJPACriteriaQuery<T>, AliasContext { * @param model the metamodel defines the scope of all persistent entity references. * @param delegator the subquery which will delegate to this receiver. */ - CriteriaQueryImpl(MetamodelImpl model, SubqueryImpl<T> delegator, OrderedMap params) { + CriteriaQueryImpl(MetamodelImpl model, SubqueryImpl<T> delegator, OrderedMap<Object, Class<?>> params) { this._model = model; this._resultClass = delegator.getJavaType(); _delegator = delegator; _aliases = getAliases(); - if (params != null) { - this._params = params; - } + _params = params; } /** @@ -237,7 +233,7 @@ class CriteriaQueryImpl<T> implements OpenJPACriteriaQuery<T>, AliasContext { @Override public Set<ParameterExpression<?>> getParameters() { collectParameters(new CriteriaExpressionVisitor.ParameterVisitor(this)); - return _params == null ? Collections.EMPTY_SET : (Set) _params.keySet(); + return (Set) _params.keySet(); } /** @@ -266,9 +262,11 @@ class CriteriaQueryImpl<T> implements OpenJPACriteriaQuery<T>, AliasContext { _groups = null; return this; } + _groups = new ArrayList<>(); - for (Expression<?> e : grouping) + for (Expression<?> e : grouping) { _groups.add(e); + } return this; } @@ -720,14 +718,14 @@ class CriteriaQueryImpl<T> implements OpenJPACriteriaQuery<T>, AliasContext { } private void renderList(StringBuilder buffer, String clause, Collection<?> coll) { - if (coll == null || coll.isEmpty()) - return; + if (coll == null || coll.isEmpty()) + return; - buffer.append(clause); - for (Iterator<?> i = coll.iterator(); i.hasNext(); ) { - buffer.append(((CriteriaExpression)i.next()).asValue(this)); - if (i.hasNext()) buffer.append(", "); - } + buffer.append(clause); + for (Iterator<?> i = coll.iterator(); i.hasNext(); ) { + buffer.append(((CriteriaExpression)i.next()).asValue(this)); + if (i.hasNext()) buffer.append(", "); + } } private void renderJoins(StringBuilder buffer, Collection<Join<?,?>> joins) { diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/ParameterExpressionImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/ParameterExpressionImpl.java index 064cfd5..89dda1e 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/ParameterExpressionImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/ParameterExpressionImpl.java @@ -107,7 +107,11 @@ class ParameterExpressionImpl<T> extends ExpressionImpl<T> org.apache.openjpa.kernel.exps.Parameter param = isCollectionValued ? factory.newCollectionValuedParameter(paramKey, clzz) : factory.newParameter(paramKey, clzz); - param.setIndex(_index); + + int index = _name != null + ? q.getParameterTypes().indexOf(this) + : _index; + param.setIndex(index); return param; } @@ -121,4 +125,39 @@ class ParameterExpressionImpl<T> extends ExpressionImpl<T> public Class<T> getParameterType() { return getJavaType(); } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + + if (o == null || getClass() != o.getClass()) + return false; + + ParameterExpressionImpl<?> that = (ParameterExpressionImpl<?>) o; + + if (_name != null ? !_name.equals(that._name) : that._name != null) + return false; + + // if name is given, then we ignore the index + if (_name == null && _index != that._index) + return false; + + if (getParameterType() != ((ParameterExpressionImpl<?>) o).getParameterType() ) + return false; + + return value != null ? value.equals(that.value) : that.value == null; + } + + @Override + public int hashCode() { + int result = _name != null ? _name.hashCode() : 0; + if (_name == null) { + // if name is given, then we ignore the index + result = 31 * result + _index; + } + result = 31 * result + (getParameterType() != null ? getParameterType().hashCode() : 0); + result = 31 * result + (value != null ? value.hashCode() : 0); + return result; + } } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/SubqueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/SubqueryImpl.java index e8abe19..9e9fb5b 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/SubqueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/SubqueryImpl.java @@ -32,6 +32,7 @@ import javax.persistence.criteria.Expression; import javax.persistence.criteria.Join; import javax.persistence.criteria.ListJoin; import javax.persistence.criteria.MapJoin; +import javax.persistence.criteria.ParameterExpression; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import javax.persistence.criteria.SetJoin; @@ -79,7 +80,7 @@ class SubqueryImpl<T> extends ExpressionImpl<T> implements Subquery<T> { SubqueryImpl(Class<T> cls, AbstractQuery<?> parent) { super(cls); _parent = parent; - OrderedMap params; + OrderedMap<Object, Class<?>> params; if (parent instanceof CriteriaQueryImpl) { _model = ((CriteriaQueryImpl<?>)parent).getMetamodel(); params = ((CriteriaQueryImpl<?>)parent).getParameterTypes();
