OpenJPA
  1. OpenJPA
  2. OPENJPA-1174

OpenJPA performs differently with orm.xml and annotations

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 1.1.1, 2.0.0-M3
    • Component/s: jpa
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      When configurations are provided from orm.xml file, some behaviors are different from behaviors on annotation configuration.
      The behavior difference occurs when fetch=LAZY is specified on many-to-one mapping.

      When there are two objects Country and Security and Security has many-to-one mapping field country,
      OpenJPA executes one more SQL query to get country field if orm.xml file is used.


      • annotation case

      // step 1) Load country in entity manager
      aUS_country = t.em.find(Country.class, aUS_sid);
      // SELECT t0.NAME FROM TEST16.COUNTRY t0 WHERE t0.COUNTRY_ID = ?

      // step 2) Load security in entity manager
      aI_security = t.em.find(Security.class, aI_sid);
      // SELECT t0.COUNTRY_ID, t0.SYMBOL FROM TEST19.SECURITY t0 WHERE t0.SECURITY_ID = ?

      // step 3) get country from security
      Country aUS_country2 = aI_security.getCountry();
      // no SQL was executed.
      .

      • orm.xml case
        .
        // step 1) Load country in entity manager
        aUS_country = t.em.find(Country.class, aUS_sid);
        // SELECT t0.NAME FROM TEST16.COUNTRY t0 WHERE t0.COUNTRY_ID = ?
        .
        // step 2) Load security in entity manager
        aI_security = t.em.find(Security.class, aI_sid);
        // SELECT t0.SYMBOL FROM TEST16.SECURITY t0 WHERE t0.SECURITY_ID = ?
        .
        // step 3) get country from security
        Country aUS_country2 = aI_security.getCountry();
        // SELECT t1.COUNTRY_ID, t1.NAME FROM TEST16.SECURITY t0, TEST16.COUNTRY t1 WHERE t0.SECURITY_ID = ? AND t0.COUNTRY_ID = t1.COUNTRY_ID

      The important difference is in step 2. When using orm.xml, many-to-one field "country" was not loaded if fetch=LAZY.
      Instead, it's loaded on annotation configuration.
      .
      Because many-to-one "country" field is not loaded, step 3 executes additional SQL to load "country" field on orm.xml.
      Instead, on annotation case, step 3 did not execute any SQLs.

      1. OPENJPA-1174-trunk.patch
        24 kB
        Ravi P Palacherla
      2. OPENJPA-1174-trunk.patch
        15 kB
        Ravi P Palacherla

        Activity

        Hide
        Ravi P Palacherla added a comment -

        Attached patch file contains the testcase and fix.

        Please comment after reviewing the changes.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Attached patch file contains the testcase and fix. Please comment after reviewing the changes. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Thanks for the patch Ravi. I noticed that you also updated how we handle embeddables when specified in a mapping file. They're now included in the default fetch group automatically.

        It matches the annotation handling and it's a great catch but we probably should add a testcase for it (unless I've missed where it's covered).

        The patch looks good though - running regression tests now.

        Show
        Michael Dick added a comment - Thanks for the patch Ravi. I noticed that you also updated how we handle embeddables when specified in a mapping file. They're now included in the default fetch group automatically. It matches the annotation handling and it's a great catch but we probably should add a testcase for it (unless I've missed where it's covered). The patch looks good though - running regression tests now.
        Hide
        Ravi P Palacherla added a comment -

        Hi Mike,

        Thanks for looking into it.

        The current testcase attached just shows how it is handled in case of mapping file.
        Are you looking for a testcase that shows the differences between annotation handling and mapping file?
        Please confirm and I will start working on it.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Mike, Thanks for looking into it. The current testcase attached just shows how it is handled in case of mapping file. Are you looking for a testcase that shows the differences between annotation handling and mapping file? Please confirm and I will start working on it. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Hi Ravi,

        Here's the portion of the patch that I'm talking about :

        @@ -1393,6 +1398,7 @@

        protected void parseEmbedded(FieldMetaData fmd, Attributes attrs)
        throws SAXException {
        assertPC(fmd, "Embedded");
        + fmd.setInDefaultFetchGroup(true);
        fmd.setEmbedded(true);
        fmd.setSerialized(false); // override any Lob annotation

        This is the only method in XMLPesistenceMetaDataParser that doesn't have a check like this one :
        String val = attrs.getValue("fetch");
        if (val != null)

        { fmd.setInDefaultFetchGroup("EAGER".equals(val)); }

        With this change we won't be checking the fetch value for Embedded fields. It matches the annotation parser, but we should add a testcase with an Embedded field to demonstrate the change in behavior.

        Actually it might be worth spending a few cycles on developing lower level unit test that validates that the annotations & xml are handled identically, but that's beyond the scope of this issue really.

        Show
        Michael Dick added a comment - Hi Ravi, Here's the portion of the patch that I'm talking about : @@ -1393,6 +1398,7 @@ protected void parseEmbedded(FieldMetaData fmd, Attributes attrs) throws SAXException { assertPC(fmd, "Embedded"); + fmd.setInDefaultFetchGroup(true); fmd.setEmbedded(true); fmd.setSerialized(false); // override any Lob annotation This is the only method in XMLPesistenceMetaDataParser that doesn't have a check like this one : String val = attrs.getValue("fetch"); if (val != null) { fmd.setInDefaultFetchGroup("EAGER".equals(val)); } With this change we won't be checking the fetch value for Embedded fields. It matches the annotation parser, but we should add a testcase with an Embedded field to demonstrate the change in behavior. Actually it might be worth spending a few cycles on developing lower level unit test that validates that the annotations & xml are handled identically, but that's beyond the scope of this issue really.
        Hide
        Michael Dick added a comment -

        Actually in thinking about this more I think the testcase needs to be reconsidered. What we're really interested in here is whether the metedata is generated identically when we use an xml mapping file or annotations. We're not particularly interested in whether we ignore FetchType=LAZY - so long as we do it identically either way the MetaData is obtained.

        The testcase relies on the LAZY hint being ignored (it's actually very interesting that it isn't ignored if no hint is specified). This behavior can't be relied on and wouldn't be expected in most cases. We could change this behavior at any point in the future and this testcase would no longer serve the intended purpose.

        A better functional test would be to have duplicates of the entities : one with annotations one with orm.xml and make sure the same SQL is issued for both of them. A better unit test could just look at the ClassMetaData / ClassMapping for the two entities and make sure they are equivalent.

        In addition we'll want to add an Embedded field in the entities since there's a specific change to parseEmbedded that needs to be asserted.

        While the code changes look correct I'd prefer to validate it with a more reliable unit test.

        Show
        Michael Dick added a comment - Actually in thinking about this more I think the testcase needs to be reconsidered. What we're really interested in here is whether the metedata is generated identically when we use an xml mapping file or annotations. We're not particularly interested in whether we ignore FetchType=LAZY - so long as we do it identically either way the MetaData is obtained. The testcase relies on the LAZY hint being ignored (it's actually very interesting that it isn't ignored if no hint is specified). This behavior can't be relied on and wouldn't be expected in most cases. We could change this behavior at any point in the future and this testcase would no longer serve the intended purpose. A better functional test would be to have duplicates of the entities : one with annotations one with orm.xml and make sure the same SQL is issued for both of them. A better unit test could just look at the ClassMetaData / ClassMapping for the two entities and make sure they are equivalent. In addition we'll want to add an Embedded field in the entities since there's a specific change to parseEmbedded that needs to be asserted. While the code changes look correct I'd prefer to validate it with a more reliable unit test.
        Hide
        Ravi P Palacherla added a comment -

        Hi Mike,

        Attached patch contains new testcase.
        This time it compared SQL generated from annotation processing with mapping file processing.

        Please review it and let me know of any changes.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Mike, Attached patch contains new testcase. This time it compared SQL generated from annotation processing with mapping file processing. Please review it and let me know of any changes. Regards, Ravi.
        Hide
        Michael Dick added a comment -

        Hi Ravi, the new patch looks good. I'm running the regression bucket now (not expecting anything just a sanity check). The only change I had to make was to reorder tags in orm.xml (m2m must come before embedded). Not sure how anyone would notice that but the IBM SDK happens to check it..

        Show
        Michael Dick added a comment - Hi Ravi, the new patch looks good. I'm running the regression bucket now (not expecting anything just a sanity check). The only change I had to make was to reorder tags in orm.xml (m2m must come before embedded). Not sure how anyone would notice that but the IBM SDK happens to check it..
        Hide
        Michael Dick added a comment -

        Thanks for the patch and all the work debugging Ravi.

        Committed in revision 799754

        Show
        Michael Dick added a comment - Thanks for the patch and all the work debugging Ravi. Committed in revision 799754
        Hide
        David Ezzio added a comment -

        Merged fix at trunk revision 799754 to 1.1.x branch at revision 802838

        Show
        David Ezzio added a comment - Merged fix at trunk revision 799754 to 1.1.x branch at revision 802838
        Hide
        Fay Wang added a comment -

        The default fetch type for ManyToOne and OneToOne is eager. That is, if the fetch type is not specified in the orm.xml, the fetch mode should be eager.

        Show
        Fay Wang added a comment - The default fetch type for ManyToOne and OneToOne is eager. That is, if the fetch type is not specified in the orm.xml, the fetch mode should be eager.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development