main@datanucleus.groups.io | Delete query with IN clause having a single parameter

prashant.mr@...

In our project, a Delete query is showing an error when used with IN clause having a single parameter.

I've forked the test-jpa project [1] and updated the SimpleTest.java [2] to recreate this issue.

The following query shows an error message: "Exception thrown when executing query : DELETE FROM PERSON A0 WHERE A0."NAME" = ? AND A0.ID ="

Query query = em.createQuery("DELETE FROM Person AS p where p.name = ?1 AND p.id IN (?2)");

query.setParameter(1, "Person1");

query.setParameter(2, 1);

query.executeUpdate();

But the following two similar queries work:

DELETE FROM Person AS p where p.id IN (?1) AND p.name = ?2

DELETE FROM Person AS p where p.name = ?1 AND p.id IN (?2, ?3)

I'd really appreciate any help to resolve this issue. Thanks.

[1] https://github.com/prashantbhat/test-jpa
[2] https://github.com/prashantbhat/test-jpa/blob/master/src/test/java/org/datanucleus/test/SimpleTest.java

Andy

#27

Care to post the actual exception + stack trace because it will tell you the problem. The log also tells you what the query is compiled to ... generic query, as well as the SQL.

prashant.mr@...

#29

Sorry, when trying to edit, I deleted the last message posted here.

I'm attaching the complete log file with only single run, this time.

Thanks,

datanucleus.log

Andy

#30

Generic compilation looks fine to me so suggest that you get the code for datanucleus-rdbms and start debugging around QueryToSQLMapper.processInExpression
https://github.com/datanucleus/datanucleus-rdbms/blob/master/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java

prashant.mr@...

#32

Thanks for pointing to the code for debugging. I'm quite new to the internals of DataNucleus, so it may take some time.

prashant.mr@...

#36

After some debugging, following is my analysis and a possible solution:

1) The valueType is not set for the symbol in ParameterExpression for parameter ?2 [1]
2) So, the JavTypeMapping used for the parameter will be null [2]
3) Because the mapping is null, the exprFactory creates a ParameterLiteral [3] which doesn't contain any value in SQLText (which actually should contain "?")

So this results in the query without the "?" appended for the parameter as: "DELETE FROM PERSON A0 WHERE A0."NAME" = ? AND A0.ID ="

I noticed that the valueType is set when the same expression is used as first parameter, 
like "DELETE FROM Person AS p where p.id IN (?1) AND p.name = ?2".

A possible solution:

With the following change in DyadicExpression to detect the valueType for IN and NOTIN operators, the query works properly. 

|| op == Expression.OP_IN || op == Expression.OP_NOTIN

Although the above change solves this particular use case, I'd like your review and feedback. And I'm not sure if this has any impact on other parts of library. 
Please let me know, if I can send a pull request with the above change. Thanks.

[1] https://github.com/datanucleus/datanucleus-core/blob/59e3939509cf72c26e3459ef7ea02b4ea052aa13/src/main/java/org/datanucleus/query/expression/DyadicExpression.java#L194
[2] https://github.com/datanucleus/datanucleus-rdbms/blob/16e5d4cab0d4555c351d9cbc1b3cdd113aeb1431/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L4039
[3] https://github.com/datanucleus/datanucleus-rdbms/blob/16e5d4cab0d4555c351d9cbc1b3cdd113aeb1431/src/main/java/org/datanucleus/store/rdbms/sql/expression/SQLExpressionFactory.java#L389

Andy

#37
Edited

Updating DyadicExpression (or even updating general code in datanucleus-core) is not an option because it may affect other databases handling (and then would mean having to re-run ALL tests on all databases), and also because if the right expression is a LIST parameter then it is wrong.

prashant.mr@...

#38

Ok, I've now reworked on this, by making changes only to QueryToSQLMapper#processInExpression.

As in the processEqExpression, replacing the parameterLiteral with the left/right JavaTypeMapping is working for queries, listed in my original SimpleTest.java

I've included the diff here below for your review.

Show quoted text

diff --git a/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java b/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java
index 5c4bdab9..fa8f6c2c 100644
--- a/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java
+++ b/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java
@@ -4761,7 +4761,14 @@ public class QueryToSQLMapper extends AbstractExpressionEvaluator implements Que
         else if (right.getParameterName() != null)
         {
             setNotPrecompilable();
-
+            if (left instanceof ParameterLiteral && !(right instanceof ParameterLiteral))
+            {
+                left = replaceParameterLiteral((ParameterLiteral)left, right.getJavaTypeMapping());
+            }
+            else if (right instanceof ParameterLiteral && !(left instanceof ParameterLiteral))
+            {
+                right = replaceParameterLiteral((ParameterLiteral)right, left.getJavaTypeMapping());
+            }
             // Single valued parameter, so use equality
             SQLExpression inExpr = new BooleanExpression(left, Expression.OP_EQ, right);
             stack.push(inExpr);

Andy

#39

I've added a variant of that to GitHub master that caters for single-valued parameter at left hand side or right hand side.

prashant.mr@...

#40

Thanks a lot