Hi Wonseok,
The logic of the new methods
buildDeleteAllStatementsForTempTable and buildDeleteAllStatementsForMappingsWithTempTable
mirrors to some extent the logic of existing methods
prepareDeleteAll and buildDeleteAllStatementsForMappings.
I would add "cross-referencing" comments to each pair of "logically-related" methods stating something like:
"A similar pattern also used in method ...: if you are updating this method consider applying a similar update to method ... as well."
Great fix, please check it in.
Thanks,
Andrei
----- Original Message -----
From: Andrei Ilitchev
To: persistence_at_glassfish.dev.java.net
Sent: Monday, April 02, 2007 3:37 PM
Subject: Re: Code review for Issue 2744 MySQL: Bulk Delete query for entity with joined inheritance fails
Hi Wonseok,
Just to let you know that I am looking at your fix right now and it will take some time for me to understand the code.
Thanks,
Andrei
----- Original Message -----
From: Wonseok Kim
To: persistence_at_glassfish.dev.java.net
Sent: Monday, April 02, 2007 7:46 AM
Subject: Code review for Issue 2744 MySQL: Bulk Delete query for entity with joined inheritance fails
Hi Tom,
Please review the attached fix.
https://glassfish.dev.java.net/issues/show_bug.cgi?id=2744
The summary of fix:
DeleteAllQuery with temporary table should handle inheritance hierachy with multiple tables similarly to DeleteAllQuery without temporary table.
However, unlike prepareDeleteAll() in ExpressionQueryMechanism buildStatementsForDeleteAllForTempTables() which generates DELETE SQLs using temporary table had not considered this. I fixed this by modifying buildStatementsForDeleteAllForTempTables() to handle multiple table child descriptors and many-to-many mappings like prepareDeleteAll(). The logic is very similar to the one in prepareDeleteAll() which calls recursively itself for child descriptors.
With this fix the following tests which had failed passes in MySQL.
Testcase: testDeleteAllProjectsWithNullTeamLeader took 0.045 sec
Testcase: testDeleteAllSmallProjectsWithNullTeamLeader took 0.046 sec
Testcase: testUpdateAllSmallProjects took 0.083 sec
Testcase: testUpdateAllLargeProjects took 0.082 sec
Testcase: testUpdateAllProjects took 0.096 sec
Testcase: testUpdateAllSmallProjectsWithName took 0.047 sec
Testcase: testUpdateAllLargeProjectsWithName took 0.031 sec
Testcase: testUpdateAllProjectsWithName took 0.034 sec
Testcase: testUpdateAllSmallProjectsWithNullTeamLeader took 0.031 sec
Testcase: testUpdateAllLargeProjectsWithNullTeamLeader took 0.033 sec
Testcase: testUpdateAllProjectsWithNullTeamLeader took 0.039 sec
Now all tests except one test are passing. The following test shows another issue, which I will handle as a separate one.
Testcase: updateUnqualifiedAttributeInWhere took 1.822 sec
Caused an ERROR
Internal Exception: com.mysql.jdbc.exceptions.MySQLIntegrityConstraintViolationException: Duplicate entry '659' for key 1
Error Code: 1062
Call: INSERT INTO TL_CMP3_EMPLOYEE (EMP_ID, VERSION, F_NAME) SELECT t2.EMP_ID, (t0.VERSION + ?), ? FROM CMP3_SALARY t3, CMP3_EMPLOYEE t2, CMP3_SALARY t1, CMP3_EMPLOYEE t0 WHERE (((t2.F_NAME = ?) AND (t3.EMP_ID = t2.EMP_ID)) AND (t1.EMP_ID = t0.EMP_ID))
bind => [1, CHANGED, Bob]
Thanks,
-Wonseok