OpenJPA
  1. OpenJPA
  2. OPENJPA-904 Testcase failures when using the PostgreSQL, Oracle, and DB2 databases
  3. OPENJPA-1083

org.apache.openjpa.persistence.kernel.TestEJBState fails with two exceptions ORA-00904 and ORA-02275 against oracleDB.

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-M2
    • Fix Version/s: 1.3.0, 2.0.0-M3
    • Component/s: None
    • Labels:
      None
    • Patch Info:
      Patch Available
    1. OPENJPA-1083_option1.patch
      6 kB
      Ravi P Palacherla
    2. OPENJPA-1083_option2.patch
      3 kB
      Ravi P Palacherla
    3. OPENJPA-1083_trunk.patch
      14 kB
      Ravi P Palacherla
    4. OPENJPA-1083.patch
      0.9 kB
      Ravi P Palacherla

      Issue Links

        Activity

        Hide
        Ravi P Palacherla added a comment -

        TestEJBState fails with ORA-00904 .

        After applying the patch on OPENJPA-946 , ORA-00904 is fixed and now I get the following exception:

        "Caused by: org.apache.openjpa.lib.jdbc.ReportingSQLException: ORA-02275: such a referential constrai
        nt already exists in the table

        {stmnt 25798515 ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC (ID) DEFERR ABLE}

        [code=2275, state=42000]"

        Show
        Ravi P Palacherla added a comment - TestEJBState fails with ORA-00904 . After applying the patch on OPENJPA-946 , ORA-00904 is fixed and now I get the following exception: "Caused by: org.apache.openjpa.lib.jdbc.ReportingSQLException: ORA-02275: such a referential constrai nt already exists in the table {stmnt 25798515 ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC (ID) DEFERR ABLE} [code=2275, state=42000] "
        Hide
        Ravi P Palacherla added a comment -

        Attached is a proposed fix.
        Please review and commit if it is fine.

        Fix for OPENJPA-946 should be applied before applying this fix.

        Show
        Ravi P Palacherla added a comment - Attached is a proposed fix. Please review and commit if it is fine. Fix for OPENJPA-946 should be applied before applying this fix.
        Hide
        Michael Dick added a comment -

        Ravi, is this something that only happens with Oracle? Or is Oracle is the only database that throws an exception?

        Either way this patch looks like it will work, but I'd like to know why we're getting a duplicate FK relationship. I believe some of the unit tests use similar entities (same table name) - if that's the case then qualifying the table name is a better approach.

        Show
        Michael Dick added a comment - Ravi, is this something that only happens with Oracle? Or is Oracle is the only database that throws an exception? Either way this patch looks like it will work, but I'd like to know why we're getting a duplicate FK relationship. I believe some of the unit tests use similar entities (same table name) - if that's the case then qualifying the table name is a better approach.
        Hide
        Ravi P Palacherla added a comment -

        Michael,

        Thanks a lot for reviewing it.

        I think the above exception will occur in any database that will not allow executing the same SQL statement to add same foreign key more than once.
        I tested it against Derby and oracle only.

        E.g: If you execute both the following statements (same statement twice)
        ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC
        ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC

        Execution of above two statement will be successful in Derby ( hence test case passes in Derby) .
        The second statement will fail in oracle with the ORA-02275 exception.

        >>Why we are getting duplicate FK ?

        In getDeleteTableContentsSQL() of DBDictionary.java,
        while trying to delete the contents of table , openJPA tries to
        a. remove FK
        b. delete rows and then
        c. add FK back

        When there is no FK name then we are not removing it but still trying to add it.
        The attached patch will not add FK unless we remove it.

        Please let me know of any suggestions.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Michael, Thanks a lot for reviewing it. I think the above exception will occur in any database that will not allow executing the same SQL statement to add same foreign key more than once. I tested it against Derby and oracle only. E.g: If you execute both the following statements (same statement twice) ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC ALTER TABLE DEPFIELDPC ADD FOREIGN KEY (OWNER_ID) REFERENCES DEPFIELDPC Execution of above two statement will be successful in Derby ( hence test case passes in Derby) . The second statement will fail in oracle with the ORA-02275 exception. >>Why we are getting duplicate FK ? In getDeleteTableContentsSQL() of DBDictionary.java, while trying to delete the contents of table , openJPA tries to a. remove FK b. delete rows and then c. add FK back When there is no FK name then we are not removing it but still trying to add it. The attached patch will not add FK unless we remove it. Please let me know of any suggestions. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Hi Ravi,

        I think there's a larger problem here. We're able to create a FK without a name but unable to remove one. The relevant methods are getDropForeignKeySQL and getForeignKeyConstraintSQL()..

        Either it should be impossible to have an unnamed FK constraint when we call getDropFKSQL or we need to make those two methods consistent..

        As far as the patch goes the approach will work for the getDeleteTableContentsSQL method, but we may still have a (potentially) larger problem to solve.

        Show
        Michael Dick added a comment - Hi Ravi, I think there's a larger problem here. We're able to create a FK without a name but unable to remove one. The relevant methods are getDropForeignKeySQL and getForeignKeyConstraintSQL().. Either it should be impossible to have an unnamed FK constraint when we call getDropFKSQL or we need to make those two methods consistent.. As far as the patch goes the approach will work for the getDeleteTableContentsSQL method, but we may still have a (potentially) larger problem to solve.
        Hide
        Ravi P Palacherla added a comment -

        Hi Michael,

        Thanks a lot for looking into it.

        Yes I agree with you, the actual issue is "We're able to create a FK without a name but unable to remove one. "

        When trying to add FK with out a name then the DB will use a system generated name.

        There are couple of options that I can think of in fixing the issue:
        1)
        While trying to remove the FK ( getDropForeignKeySQL ) if there is no foreign key name then I will try to get the FK name from DB and use it in the drop FK statement.

        2)
        While adding a constraint, openJPA should give a default name to constraint, unless already specified by app or testcase.
        In this case we may have to handle the duplicate constraint name on a table just like column names.

        I will start working on it, unless you have objection with any one of them (or both).

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Michael, Thanks a lot for looking into it. Yes I agree with you, the actual issue is "We're able to create a FK without a name but unable to remove one. " When trying to add FK with out a name then the DB will use a system generated name. There are couple of options that I can think of in fixing the issue: 1) While trying to remove the FK ( getDropForeignKeySQL ) if there is no foreign key name then I will try to get the FK name from DB and use it in the drop FK statement. 2) While adding a constraint, openJPA should give a default name to constraint, unless already specified by app or testcase. In this case we may have to handle the duplicate constraint name on a table just like column names. I will start working on it, unless you have objection with any one of them (or both). Regards, Ravi.
        Hide
        Ravi P Palacherla added a comment - - edited

        Hi Michael,

        I spent last couple of days on this issue and came up with a solution as described in #1 in my previous comment.
        That is to retrieve the FK name from DB while deleting the FK constraint.
        Even though this solution works fine it could be costly as it involves going to DB to fetch the FK name.
        So I am still researching on a less expensive solution.

        Can you commit my attached changes on this JIRA as it fixed the testcase and I will open a new JIRA to handle the actual foreign key issue.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - - edited Hi Michael, I spent last couple of days on this issue and came up with a solution as described in #1 in my previous comment. That is to retrieve the FK name from DB while deleting the FK constraint. Even though this solution works fine it could be costly as it involves going to DB to fetch the FK name. So I am still researching on a less expensive solution. Can you commit my attached changes on this JIRA as it fixed the testcase and I will open a new JIRA to handle the actual foreign key issue. Regards, Ravi.
        Hide
        Ravi P Palacherla added a comment -

        Hi Michael,

        option#1 and option#2 patches are uploaded.
        You do not need both patches, either one of them should work.

        TestEJBState will work only with patch OPENJPA-946 and the current patch.
        My current patch handles the foreign key problem.

        option#1 :
        Every time I try to delete a FK, I will check the FKname.
        If it does not exist then I go to DB and fetch the FKName.

        If option#1 is not preferable for any reason then you can try option2.

        option#2 :
        Every time I try to add a FK.
        I will check in the Mappings if FKName exists.
        If it does not exist then a default name will be given.
        The default name is local table name + column name of FK.

        The only issue with option#2 is that it will work on a new clean database.
        So running "mvn clean test -DfailIfNoTests=false" will run fine with out any issues.

        If the tables are already created then it will not work because openJPA will think that the default name FK
        is new FK and will try to add the existing FK with the new default name.

        I think it is an existing issue, same behavior for columns , tables etc...

        Please let me know if option#2 is preferable, but my changes are not acceptable because it does not work with existing database. I can add the logic of making it work on existing database also.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Michael, option#1 and option#2 patches are uploaded. You do not need both patches, either one of them should work. TestEJBState will work only with patch OPENJPA-946 and the current patch. My current patch handles the foreign key problem. option#1 : Every time I try to delete a FK, I will check the FKname. If it does not exist then I go to DB and fetch the FKName. If option#1 is not preferable for any reason then you can try option2. option#2 : Every time I try to add a FK. I will check in the Mappings if FKName exists. If it does not exist then a default name will be given. The default name is local table name + column name of FK. The only issue with option#2 is that it will work on a new clean database. So running "mvn clean test -DfailIfNoTests=false" will run fine with out any issues. If the tables are already created then it will not work because openJPA will think that the default name FK is new FK and will try to add the existing FK with the new default name. I think it is an existing issue, same behavior for columns , tables etc... Please let me know if option#2 is preferable, but my changes are not acceptable because it does not work with existing database. I can add the logic of making it work on existing database also. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Hi Ravi,

        I prefer option 1. It shouldn't affect existing tables (say if you migrate from 1.2 to 2.0) and it should work with your testcase.

        Show
        Michael Dick added a comment - Hi Ravi, I prefer option 1. It shouldn't affect existing tables (say if you migrate from 1.2 to 2.0) and it should work with your testcase.
        Hide
        Michael Dick added a comment -

        Some comments on the patch for option 1.

        It seems like the logic to obtain the foreign key name from the database belongs in the ForeignKey or Constraint class. Adding a method called loadName() or something similar seems cleaner.

        That being said it's not obvious where we read the name from the database.

        I also noticed that the patch always removes the FK from localTable (DBDictionary.getDropForeignKeySQL) but only adds it if it wasn't found.

        The last suggestion I'd make is to add some sort of unit test just for this change. I know it shows up in other testcases on Oracle, but it'd be nice to just validate this part of the change.

        Show
        Michael Dick added a comment - Some comments on the patch for option 1. It seems like the logic to obtain the foreign key name from the database belongs in the ForeignKey or Constraint class. Adding a method called loadName() or something similar seems cleaner. That being said it's not obvious where we read the name from the database. I also noticed that the patch always removes the FK from localTable (DBDictionary.getDropForeignKeySQL) but only adds it if it wasn't found. The last suggestion I'd make is to add some sort of unit test just for this change. I know it shows up in other testcases on Oracle, but it'd be nice to just validate this part of the change.
        Hide
        Michael Dick added a comment -

        Forgot to finish my earlier comment. On the whole this is a good patch and thanks for putting it all together Ravi!

        Show
        Michael Dick added a comment - Forgot to finish my earlier comment. On the whole this is a good patch and thanks for putting it all together Ravi!
        Hide
        Milosz Tylenda added a comment -

        Hi Ravi & Mike,

        Just wondering whether we need to modify the regex in TestParentChild to account for an optional "DEFFERABLE" clause? The syntax can be seen in the first comment to this issue. I remember failures of this tests, OracleDictionary has supportsDeferredConstraints = true. Is this test passing for you?

        Show
        Milosz Tylenda added a comment - Hi Ravi & Mike, Just wondering whether we need to modify the regex in TestParentChild to account for an optional "DEFFERABLE" clause? The syntax can be seen in the first comment to this issue. I remember failures of this tests, OracleDictionary has supportsDeferredConstraints = true. Is this test passing for you?
        Hide
        Ravi P Palacherla added a comment -

        Hi,

        Thanks for your comments.

        It is supposed to be handled as part of
        http://issues.apache.org/jira/browse/OPENJPA-907

        There is a patch attached to it (I was a bit liberal with the RegEx).
        I will correct it and ask for commiting the patch, once I am done with this.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi, Thanks for your comments. It is supposed to be handled as part of http://issues.apache.org/jira/browse/OPENJPA-907 There is a patch attached to it (I was a bit liberal with the RegEx). I will correct it and ask for commiting the patch, once I am done with this. Regards, Ravi.
        Hide
        Ravi P Palacherla added a comment -

        Hi Mike,

        Attached patch includes all the suggestions in your previous comment.

        Major changes:
        a. Moved the logic of getting FK name from DB to ForeignKey.java
        b. Added a test case to validate loadNameFromDB() of ForeignKey.java
        c. My patch( ForeignKey. loadNameFromDB() ) no more removes all the FKs.
        It will only remove the newly added FK.

        Please review it and let me know of any further changes.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Mike, Attached patch includes all the suggestions in your previous comment. Major changes: a. Moved the logic of getting FK name from DB to ForeignKey.java b. Added a test case to validate loadNameFromDB() of ForeignKey.java c. My patch( ForeignKey. loadNameFromDB() ) no more removes all the FKs. It will only remove the newly added FK. Please review it and let me know of any further changes. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Thanks for the patch Ravi. I did a little cleanup for compiler warnings and formatting - nothing major.

        Show
        Michael Dick added a comment - Thanks for the patch Ravi. I did a little cleanup for compiler warnings and formatting - nothing major.
        Hide
        Milosz Tylenda added a comment -

        I am afraid the change causes a crash with MySQL:

        <openjpa-2.0.0-SNAPSHOT-r422266:782132 nonfatal general error> org.apache.openjpa.persistence.PersistenceException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CONSTRAINT DEPFIELDPC_ibfk_1' at line 1

        {stmnt 15864952 ALTER TABLE DEPFIELDPC DROP CONSTRAINT DEPFIELDPC_ibfk_1}

        [code=1064, state=42000]
        at org.apache.openjpa.jdbc.meta.MappingTool.record(MappingTool.java:553)
        [...]

        Probably MySQL requires DROP FOREIGN KEY instead of DROP CONSTRAINT.

        Other databases should also be checked.

        Show
        Milosz Tylenda added a comment - I am afraid the change causes a crash with MySQL: <openjpa-2.0.0-SNAPSHOT-r422266:782132 nonfatal general error> org.apache.openjpa.persistence.PersistenceException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CONSTRAINT DEPFIELDPC_ibfk_1' at line 1 {stmnt 15864952 ALTER TABLE DEPFIELDPC DROP CONSTRAINT DEPFIELDPC_ibfk_1} [code=1064, state=42000] at org.apache.openjpa.jdbc.meta.MappingTool.record(MappingTool.java:553) [...] Probably MySQL requires DROP FOREIGN KEY instead of DROP CONSTRAINT. Other databases should also be checked.
        Hide
        Milosz Tylenda added a comment -

        I have checked the latest docs of DB2, Derby, Firebird, MySQL, Oracle, PostgreSQL, SQL Server and Sybase 11and it seems that only MySQL does not like DROP CONSTRAINT and needs DROP FOREIGN KEY key_name.

        Maybe this issue could be closed and a new one created for updating MySQL dictionary.

        Since MySQL does not understand DROP CONSTRAINT at all, we might also need to override getDropPrimaryKeySQL. On the other hand, getDropPrimaryKeySQL seems not to be used with PK name != null. Is there a way for an application to specify PK constraint name?

        Show
        Milosz Tylenda added a comment - I have checked the latest docs of DB2, Derby, Firebird, MySQL, Oracle, PostgreSQL, SQL Server and Sybase 11and it seems that only MySQL does not like DROP CONSTRAINT and needs DROP FOREIGN KEY key_name. Maybe this issue could be closed and a new one created for updating MySQL dictionary. Since MySQL does not understand DROP CONSTRAINT at all, we might also need to override getDropPrimaryKeySQL. On the other hand, getDropPrimaryKeySQL seems not to be used with PK name != null. Is there a way for an application to specify PK constraint name?
        Hide
        Ravi P Palacherla added a comment - - edited

        Hi Milosz,

        The bug you mentioned exists even before the fix for this JIRA went in.

        The cause of the issue is inside DBDictionary.getDropForeignKeySQL() where it tries to use DROP constraint instead of DROP FK.

        The reason for why this issue appears now is because before the fix for this JIRA , getDropForeignKeySQL() was not dropping the foreignkey if FK name is null.

        The fix in this JIRA makes sure that the FKs are dropped properly before clearing table contents and hence you are seeing this issue now.

        So, I guess a new JIRA would be appropriate to fix the issue. Please let me know if you think otherwise.

        Also we may have to make getDropPrimaryKeySQL() consistent with getDropForeignKeySQL().

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - - edited Hi Milosz, The bug you mentioned exists even before the fix for this JIRA went in. The cause of the issue is inside DBDictionary.getDropForeignKeySQL() where it tries to use DROP constraint instead of DROP FK. The reason for why this issue appears now is because before the fix for this JIRA , getDropForeignKeySQL() was not dropping the foreignkey if FK name is null. The fix in this JIRA makes sure that the FKs are dropped properly before clearing table contents and hence you are seeing this issue now. So, I guess a new JIRA would be appropriate to fix the issue. Please let me know if you think otherwise. Also we may have to make getDropPrimaryKeySQL() consistent with getDropForeignKeySQL(). Regards, Ravi.
        Hide
        Milosz Tylenda added a comment -

        I will open a new issue to deal with MySQL requirements.

        Show
        Milosz Tylenda added a comment - I will open a new issue to deal with MySQL requirements.
        Hide
        Jeremy Bauer added a comment -

        I've been working on an Oracle test failure(TestRelationFieldAsPrimaryKeyAndForeignKey.testQuery) in trunk that is identical to this issue. I thought this fix would have corrected the problem, but I discovered that the fix works great for single column foreign keys, but does not work with multi-column keys. I have an addendum to the original fix that I'll be committing to trunk shortly.

        Show
        Jeremy Bauer added a comment - I've been working on an Oracle test failure(TestRelationFieldAsPrimaryKeyAndForeignKey.testQuery) in trunk that is identical to this issue. I thought this fix would have corrected the problem, but I discovered that the fix works great for single column foreign keys, but does not work with multi-column keys. I have an addendum to the original fix that I'll be committing to trunk shortly.

          People

          • Assignee:
            Michael Dick
            Reporter:
            Ravi P Palacherla
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development