persistence@glassfish.java.net

Re: Code review for Issue 2744 MySQL: Bulk Delete query for entity with joined inheritance fails

From: Andrei Ilitchev <andrei.ilitchev_at_oracle.com>
Date: Mon, 2 Apr 2007 16:46:42 -0400

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