OpenJPA
  1. OpenJPA
  2. OPENJPA-1051

[patch] Mappingtool doesn't check name conflicts if MappingDefaultsImpl is called with multiple columns.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0, 2.0.0-M2
    • Fix Version/s: 1.1.1, 2.0.0-M2
    • Component/s: jdbc
    • Labels:
      None
    • Environment:
      OpenJPA Trunk.

      Description

      In OpenJPA implementation, it looks that MappingDefaultsImpl.populateColumns can accept multiple columns
      because it has Column[] signature.
      If column name is longer than DBDictionary restriction (it's very short in some databases.
      For example, oracle max name length is 32), names are truncated.

      Because name conflict is detected based on actual Table info,given Column[] data does not get checked.
      So, if given Column[] have very long name and truncated name of these Column[] is conflicted,
      it could not be detected.

      1. OPENJPA-1051_Trunk.patch
        7 kB
        Ravi P Palacherla
      2. OPENJPA-1051_Simple_Trunk.patch
        5 kB
        Ravi P Palacherla

        Activity

        Hide
        Ravi P Palacherla added a comment -

        I added temporary name set to detect such name conflicts to Table and NameSet classes.
        Then, MappingDefaultsImpl uses this mechanism.
        name collision is detected in NameSet class and resolved truncated name is added on MappingDefaultsImpl. After resolving all names, temporary name cache is removed on
        MappingDefaultsImpl.

        The fix can be verified by new unittest class TestMappingDefaultsImpl.java.

        Testing Done:
        OpenJPA head

        Show
        Ravi P Palacherla added a comment - I added temporary name set to detect such name conflicts to Table and NameSet classes. Then, MappingDefaultsImpl uses this mechanism. name collision is detected in NameSet class and resolved truncated name is added on MappingDefaultsImpl. After resolving all names, temporary name cache is removed on MappingDefaultsImpl. The fix can be verified by new unittest class TestMappingDefaultsImpl.java. Testing Done: OpenJPA head
        Hide
        Ravi P Palacherla added a comment -

        Can someone please commit the patch.

        Show
        Ravi P Palacherla added a comment - Can someone please commit the patch.
        Hide
        David Ezzio added a comment -

        Applied Ravi's patch. Revision 774580.

        Show
        David Ezzio added a comment - Applied Ravi's patch. Revision 774580.
        Hide
        David Ezzio added a comment -

        From trunk, merged fix to 1.1.x branch at rev 802208

        Show
        David Ezzio added a comment - From trunk, merged fix to 1.1.x branch at rev 802208
        Hide
        Pinaki Poddar added a comment -

        A more parsimonious solution (perhaps) would be to simply add the valid name to NameSet.addName(...),
        after the name is validated by DBDictionary.makeNameValid() which also ensures uniqueness against the name set,

        The issue of naming is getting trickier with several aspects
        a) The logic/protocol of naming of database elements (Table/Column/Schema/Sequence) is somewhat well-spread at different parts
        b) few assumptions are implicit (like full name of a table is a concatenation of schema.table – but MySQL, for example, will not like that)
        c) other 'container' things often cache these elements by their names but these names may get shortened/modified because of database length restrictions/keyword clash
        – all these make naming a complex issue. One can see when methods like MappingDefaults.correctName() appears – as if we know we have done mistakes

        d) the proverbial straw on the camel's back is the new JPA 2.0 requirement of these names be optionally quoted/delimited with default/platform specific quote characters.

        In light of all the above, having a NameSet._subNames and adding/resetting it everywhere may add to the complexities.

        Comments/Thoughts?

        Show
        Pinaki Poddar added a comment - A more parsimonious solution (perhaps) would be to simply add the valid name to NameSet.addName(...), after the name is validated by DBDictionary.makeNameValid() which also ensures uniqueness against the name set, The issue of naming is getting trickier with several aspects a) The logic/protocol of naming of database elements (Table/Column/Schema/Sequence) is somewhat well-spread at different parts b) few assumptions are implicit (like full name of a table is a concatenation of schema.table – but MySQL, for example, will not like that) c) other 'container' things often cache these elements by their names but these names may get shortened/modified because of database length restrictions/keyword clash – all these make naming a complex issue. One can see when methods like MappingDefaults.correctName() appears – as if we know we have done mistakes d) the proverbial straw on the camel's back is the new JPA 2.0 requirement of these names be optionally quoted/delimited with default/platform specific quote characters. In light of all the above, having a NameSet._subNames and adding/resetting it everywhere may add to the complexities. Comments/Thoughts?
        Hide
        Ravi P Palacherla added a comment -

        Hi Pinaki,

        Thanks for your suggestions.
        After reviewing your comments, I agree that the current solution is a bit complex and will try to work on the solution you suggested.

        Regards,
        Ravi.

        Show
        Ravi P Palacherla added a comment - Hi Pinaki, Thanks for your suggestions. After reviewing your comments, I agree that the current solution is a bit complex and will try to work on the solution you suggested. Regards, Ravi.
        Hide
        Ravi P Palacherla added a comment -

        Attached is the simplified fix suggested by Pinaki.
        Thanks for the suggestion.

        Ravi.

        Show
        Ravi P Palacherla added a comment - Attached is the simplified fix suggested by Pinaki. Thanks for the suggestion. Ravi.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development