JPQL: UPDATE with subquery updates too many rows

[glassfishrobot]

A JPQL UPDATE query with a subquery in the where clause updates too many rows in the database. The following JPQL query is supposed to update the name of customers having an order: UPDATE Customer c SET c.name = 'CHANGED' WHERE (SELECT COUNT(o) FROM c.orders o) > 0 It actually updates all the customers.

The above problem only occus when uisng a subquzery in the WHERE clause. The following equivalent JQPL query returns the correct result: UPDATE Customer c SET c.name = 'CHANGED' WHERE c.orders IS NOT EMPTY

Here is the generated SQL for the JPQL UPDATE with subquery: UPDATE CUSTOMER_TABLE SET VERSION = (VERSION + ?), NAME = ? WHERE EXISTS( SELECT t0.ID FROM CUSTOMER_TABLE t0 WHERE (( SELECT COUNT FROM CUSTOMER_TABLE t2, ORDER_TABLE t1 WHERE (t1.CUST_ID = t2.ID)) > ?) AND t0.ID = CUSTOMER_TABLE.ID)

Adding a clause joining t0 and t2 would return the expected result. Then the WHERE clause of the innermost query would become: WHERE (t1.CUST_ID = t2.ID) AND (t2.ID = t0.ID)) > ?)

We had a similar issue with subqueries in SELECT. We decided that until issue 197 (duplicated tables in generated SQL) is not fixed, the JPQL compiler will connect the ExpressionBuilders for t0 and t2 which then generates the join clause. In the case of the UPDATE query, the toplink queryframework generates the EXISTS query, so the JPQL compiler cannot connect t0 and t2.

Environment

Operating System: All Platform: All

Affected Versions

[9.1pe]

[glassfishrobot]

• Issue Imported From: https://github.com/javaee/glassfish/issues/592
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @yaminikb
• Original Issue Closed By:@glassfishrobot

[glassfishrobot]

@glassfishrobot Commented mb124283 said: Upgrade the priority to P2 since it is data corruption.

[glassfishrobot]

@glassfishrobot Commented tcng said: marked as9-na since this bug only exists in the trunk and not in the FCS branch

[glassfishrobot]

@glassfishrobot Commented pkrogh said: Looking into this.

[glassfishrobot]

@glassfishrobot Commented chris_delahunt said: Reassigned to Andrei

[glassfishrobot]

@glassfishrobot Commented ailitche said: The following TopLink code: ExpressionBuilder builder = new ExpressionBuilder(); UpdateAllQuery updateQuery = new UpdateAllQuery(Employee.class, builder); Expression selExpression = builder.size("phoneNumbers").greaterThan(0); updateQuery.setSelectionCriteria(selExpression); produces the correct sql: UPDATE EMPLOYEE SET VERSION = (VERSION + 1), L_NAME = 'CHANGED' WHERE EXISTS( SELECT t0.EMP_ID FROM EMPLOYEE t0, SALARY t1 WHERE ( ((SELECT COUNT FROM PHONE t2 WHERE (t2.EMP_ID = t0.EMP_ID)) > 0) AND (t1.EMP_ID = t0.EMP_ID)) AND t0.EMP_ID = EMPLOYEE.EMP_ID) In the example Employee class is mapped to EMPLOYEE and SALARY tables and is in 1 to many relationship with Phone class (mapped to PHONE table).

Could the issue be fixed by changing EJBQL parser to generate the kind of TopLink query provided in the example above? If not please post the TopLink query currently generated.

[glassfishrobot]

@glassfishrobot Commented mb124283 said: Hi Andrei,

the query compiler would generate the TopLink code you sepcified for an UPDATE query using the SIZE function in its WHERE clause: UPDATE Customer c SET c.name = 'CHANGED' WHERE SIZE(c.orders) > 0 This equivalent query is much simpler than the original query, since it eliminates the subquery. This works for this particular query. However, I think in general it is not possible to eleminate the subquery and replace it by some other query language constructs.

Here is the TopLink code I used to reproduce the problem. It creates a ReportQuery instance representing the COUNT subquery. The ReportQuery is wrapped into an expression (Expression.subQuery(ReportQuery), which is the left hand side of an greaterThen(0) call. This expression defines the selection criteria of the update query: UpdateAllQuery update = new UpdateAllQuery(Customer.class); update.setShouldDeferExecutionInUOW(false); ExpressionBuilder cBuilder = new ExpressionBuilder(Customer.class); update.setExpressionBuilder(cBuilder); update.addUpdate(cBuilder.get("name"), new ExpressionBuilder().value("CHANGED")); ReportQuery subquery = new ReportQuery(Customer.class, cBuilder); subquery.addCount("COUNT", cBuilder.anyOf("orders"), Long.class); update.setSelectionCriteria(cBuilder.subQuery(subquery).greaterThan(0)); Object result = em.getActiveSession().executeQuery(update);

Regards Michael

[glassfishrobot]

@glassfishrobot Commented ailitche said: Hi Michael,

If selection criteria from any UpdateAllQuery is used in ReadAllQuery then the ReadAllQuery will return the same set of objects, as UpdateAllQuery would have updated.

When I tried executing ReadAllQuery with the selection criteria you provided it returned all the objects. That means the selection criteria is wrong.

The following ReadAllQuery is correct (only selects Employees with count phones>0): ExpressionBuilder builder = new ExpressionBuilder(); ReadAllQuery query = new ReadAllQuery(Employee.class, builder);

ExpressionBuilder subqueryBuilder = new ExpressionBuilder(); ReportQuery subquery = new ReportQuery(PhoneNumber.class, subqueryBuilder); subquery.addCount("COUNT", subquery.getExpressionBuilder()); subquery.setSelectionCriteria(subquery.getExpressionBuilder().equal (builder.anyOf("phoneNumbers"))); query.setSelectionCriteria(builder.subQuery(subquery).greaterThan(0));

and so is UpdateAllQuery with the same selection criteria: ExpressionBuilder builder = new ExpressionBuilder(); UpdateAllQuery updateQuery = new UpdateAllQuery(Employee.class, builder);

ExpressionBuilder subqueryBuilder = new ExpressionBuilder(); ReportQuery subquery = new ReportQuery(PhoneNumber.class, subqueryBuilder); subquery.addCount("COUNT", subquery.getExpressionBuilder()); subquery.setSelectionCriteria(subquery.getExpressionBuilder().equal (builder.anyOf("phoneNumbers"))); updateQuery.setSelectionCriteria(builder.subQuery(subquery).greaterThan(0));

updateQuery.addUpdate(builder.get("lastName"), "CHANGED");

Thanks,

Andrei

[glassfishrobot]

@glassfishrobot Commented ailitche said: Hi Michael,

Here's an alternative way to fix the TopLink query suggested by Gordon. This one should be easier for you to implement: the only two changes is introduction of innerBuilder and joining it with the original one in subquery's selection criteria.

UpdateAllQuery update = new UpdateAllQuery(Customer.class); update.setShouldDeferExecutionInUOW(false); ExpressionBuilder cBuilder = new ExpressionBuilder(Customer.class); update.setExpressionBuilder(cBuilder); update.addUpdate(cBuilder.get("name"), new ExpressionBuilder().value("CHANGED")); ExpressionBuilder internalBuilder = new ExpressionBuilder(); ReportQuery subquery = new ReportQuery(Customer.class, internalBuilder); subquery.addCount("COUNT", internalBuilder.anyOf("orders"), Long.class); subquery.setSelectionCriteria(internalBuilder.equal(cBuilder)); update.setSelectionCriteria(cBuilder.subQuery(subquery).greaterThan(0)); Object result = em.getActiveSession().executeQuery(update);

Thanks,

Andrei

[glassfishrobot]

@glassfishrobot Commented mb124283 said: Hi Andrei

thanks for the new TopLink sample code. Yes, this is the TopLink code I indented to generate and it is very similar to what gets generated in case of a SELECT query with a subquery.

I think I found the problem. The code to generate the join between the subquery builder and the ExpressionBuilder for the outer query is already in place. It needs some internal data that gets calculated before the DatabaseQuery is populated. This call was omitted in case of an UPDATE query, such that the SubqueryNode did not get any information about the outer ExpressionBuilder. I will send out the fix to the persistence alias before I check it in.

Regards Michael

[glassfishrobot]

@glassfishrobot Commented mb124283 said: Checked in the fix.

cvs commit output module entity-persistence: Checking in src/java/oracle/toplink/essentials/internal/parsing/EJBQLParseTree.java; new revision: 1.13; previous revision: 1.12

cvs commit output module entity-persistence-tests: Checking in src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLModifyTestSuite.java; initial revision: 1.1 Checking in src/java/oracle/toplink/essentials/testing/tests/FullRegressionTestSuite.java; new revision: 1.11; previous revision: 1.10

[glassfishrobot]

@glassfishrobot Commented mm110999 said: Backported to UR1

[glassfishrobot]

@glassfishrobot Commented ds122787 said: change to 9.1pe

[glassfishrobot]

@glassfishrobot Commented ds122787 said:

[glassfishrobot]

@glassfishrobot Commented Was assigned to mb124283

[glassfishrobot]

@glassfishrobot Commented This issue was imported from java.net JIRA GLASSFISH-592

[glassfishrobot]

@glassfishrobot Commented Reported by mb124283

[glassfishrobot]

@glassfishrobot Commented Marked as fixed on Friday, March 9th 2007, 10:42:53 am