openjpa-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From strub...@apache.org
Subject [openjpa] 04/04: OPENJPA-2733 fix param index.
Date Thu, 14 Feb 2019 14:09:52 GMT
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 <struberg@apache.org>
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();


Mime
View raw message