OpenJPA
  1. OpenJPA
  2. OPENJPA-1979

Regression for non-standard joins with constant column values

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-M1, 2.0.0
    • Fix Version/s: 2.3.0
    • Component/s: jdbc, kernel
    • Labels:
      None

      Description

      The non-standard join can use constant column values by enclosing them in single-quote character. This behavior is regressed. The likely cause of this regression is new delimiting support for schema elements introduced in JPA 2.0. The constant column value used to be detected during schema definition based on the assumption of the name being enclosed in single-quote. Due to introduction of delimiting capability, the constant value is now enclosed in double-quote followed by a single-quote.

      The visible effect is failure to define schema for non-standard join with constant values.

      1. Test-1979.zip
        10 kB
        Pinaki Poddar
      2. OPENJPA-1979.patch.1.txt
        1 kB
        Pinaki Poddar

        Issue Links

          Activity

          Hide
          brian yoder added a comment -

          I think it may still not be working for constant numeric values. Maybe you fixed for constant string values, but I am still getting errors in 2.3.0 using hardcoded numbers:

          <openjpa-2.3.0-r422266:1540826 fatal user error> org.apache.openjpa.persistence.ArgumentException: "com.sscims.im.entity.IMSObject.refObject" defines a target of ""14"" for column "entity_type_id", but that target does not exist in table "dbo.im_ims_obj".

          @ManyToOne(fetch=FetchType.LAZY)
          @JoinTable(
          name="im_ref",
          joinColumns=

          { @JoinColumn(name="entity_type_id", referencedColumnName="14"), @JoinColumn(name="entity_id", referencedColumnName="obj_type_id") }

          )

          Show
          brian yoder added a comment - I think it may still not be working for constant numeric values. Maybe you fixed for constant string values, but I am still getting errors in 2.3.0 using hardcoded numbers: <openjpa-2.3.0-r422266:1540826 fatal user error> org.apache.openjpa.persistence.ArgumentException: "com.sscims.im.entity.IMSObject.refObject" defines a target of ""14"" for column "entity_type_id", but that target does not exist in table "dbo.im_ims_obj". @ManyToOne(fetch=FetchType.LAZY) @JoinTable( name="im_ref", joinColumns= { @JoinColumn(name="entity_type_id", referencedColumnName="14"), @JoinColumn(name="entity_id", referencedColumnName="obj_type_id") } )
          Hide
          ASF subversion and git services added a comment -

          Commit 1484326 from hthomann
          [ https://svn.apache.org/r1484326 ]

          OPENJPA-1979: Regression for non-standard joins with constant column values - back ported to 2.2.x Pinaki Poddar's trunk changes.

          Show
          ASF subversion and git services added a comment - Commit 1484326 from hthomann [ https://svn.apache.org/r1484326 ] OPENJPA-1979 : Regression for non-standard joins with constant column values - back ported to 2.2.x Pinaki Poddar's trunk changes.
          Hide
          ASF subversion and git services added a comment -

          Commit 1484322 from hthomann
          [ https://svn.apache.org/r1484322 ]

          OPENJPA-1979: Regression for non-standard joins with constant column values - back ported to 2.2.1.x Pinaki Poddar's trunk changes.

          Show
          ASF subversion and git services added a comment - Commit 1484322 from hthomann [ https://svn.apache.org/r1484322 ] OPENJPA-1979 : Regression for non-standard joins with constant column values - back ported to 2.2.1.x Pinaki Poddar's trunk changes.
          Hide
          ASF subversion and git services added a comment -

          Commit 1484313 from hthomann
          [ https://svn.apache.org/r1484313 ]

          OPENJPA-1979: Regression for non-standard joins with constant column values - back ported to 2.1.x Pinaki Poddar's trunk changes.

          Show
          ASF subversion and git services added a comment - Commit 1484313 from hthomann [ https://svn.apache.org/r1484313 ] OPENJPA-1979 : Regression for non-standard joins with constant column values - back ported to 2.1.x Pinaki Poddar's trunk changes.
          Hide
          ASF subversion and git services added a comment -

          Commit 1461833 from Pinaki Poddar
          [ https://svn.apache.org/r1461833 ]

          OPENJPA-1979: reference column name starting with single-quote not delimited as they have special semantics for non-standard constant join

          Show
          ASF subversion and git services added a comment - Commit 1461833 from Pinaki Poddar [ https://svn.apache.org/r1461833 ] OPENJPA-1979 : reference column name starting with single-quote not delimited as they have special semantics for non-standard constant join
          Hide
          Pat McCabe added a comment -

          Hello. Is there any update on if/when this fix will be available ?

          Show
          Pat McCabe added a comment - Hello. Is there any update on if/when this fix will be available ?
          Hide
          David Minor added a comment -

          A similar fix is also needed for 'private static Column newColumn(ElementJoinColumn join, boolean delimit)', and possibly the other join annotations if they used to support constant joins.

          Show
          David Minor added a comment - A similar fix is also needed for 'private static Column newColumn(ElementJoinColumn join, boolean delimit)', and possibly the other join annotations if they used to support constant joins.
          Hide
          Francois added a comment -

          Hi.

          I checked out version 2.2.0 and found that this patch wasn't applied yet.
          Based on Micael's question, I modified the method as follows and I can confirm that it worked with constant values as numbers and text:

          private Column newColumn(JoinColumn join) {
          Column col = new Column();
          if (!StringUtils.isEmpty(join.name()))
          col.setIdentifier(DBIdentifier.newColumn(join.name(), delimit()));
          if (!StringUtils.isEmpty(join.columnDefinition()))
          col.setTypeIdentifier(DBIdentifier.newColumnDefinition(join.columnDefinition()));
          // if (!StringUtils.isEmpty(join.referencedColumnName())) //f fix from openjpa-1979
          // col.setTargetIdentifier(DBIdentifier.newColumn(join.referencedColumnName(), delimit()));
          String joinColumnName = join.referencedColumnName();
          if (!StringUtils.isEmpty(joinColumnName)) {
          boolean isConstant = joinColumnName.charAt(0) == '\'';
          if(!isConstant){
          if (joinColumnName.charAt(0) == '-'

          joinColumnName.charAt(0) == '.'
          Character.isDigit(joinColumnName.charAt(0))) {
          isConstant = true;
          try
          Unknown macro: { if (joinColumnName.indexOf('.') == -1){ new Integer(joinColumnName); } else{ new Double(joinColumnName); } }

          catch (NumberFormatException nfe)

          { throw new RuntimeException("Constant Join Column is not a Valid Number: " + joinColumnName + ", Join: " + join); }

          } else if ("null".equalsIgnoreCase(joinColumnName))

          { isConstant = true; }

          }

          if (isConstant)

          { col.setTargetIdentifier(DBIdentifier.newConstant(joinColumnName)); }

          else

          { col.setTargetIdentifier(DBIdentifier.newColumn(joinColumnName, delimit())); }

          }
          col.setNotNull(!join.nullable());
          col.setFlag(Column.FLAG_UNINSERTABLE, !join.insertable());
          col.setFlag(Column.FLAG_UNUPDATABLE, !join.updatable());
          return col;
          }

          Show
          Francois added a comment - Hi. I checked out version 2.2.0 and found that this patch wasn't applied yet. Based on Micael's question, I modified the method as follows and I can confirm that it worked with constant values as numbers and text: private Column newColumn(JoinColumn join) { Column col = new Column(); if (!StringUtils.isEmpty(join.name())) col.setIdentifier(DBIdentifier.newColumn(join.name(), delimit())); if (!StringUtils.isEmpty(join.columnDefinition())) col.setTypeIdentifier(DBIdentifier.newColumnDefinition(join.columnDefinition())); // if (!StringUtils.isEmpty(join.referencedColumnName())) //f fix from openjpa-1979 // col.setTargetIdentifier(DBIdentifier.newColumn(join.referencedColumnName(), delimit())); String joinColumnName = join.referencedColumnName(); if (!StringUtils.isEmpty(joinColumnName)) { boolean isConstant = joinColumnName.charAt(0) == '\''; if(!isConstant){ if (joinColumnName.charAt(0) == '-' joinColumnName.charAt(0) == '.' Character.isDigit(joinColumnName.charAt(0))) { isConstant = true; try Unknown macro: { if (joinColumnName.indexOf('.') == -1){ new Integer(joinColumnName); } else{ new Double(joinColumnName); } } catch (NumberFormatException nfe) { throw new RuntimeException("Constant Join Column is not a Valid Number: " + joinColumnName + ", Join: " + join); } } else if ("null".equalsIgnoreCase(joinColumnName)) { isConstant = true; } } if (isConstant) { col.setTargetIdentifier(DBIdentifier.newConstant(joinColumnName)); } else { col.setTargetIdentifier(DBIdentifier.newColumn(joinColumnName, delimit())); } } col.setNotNull(!join.nullable()); col.setFlag(Column.FLAG_UNINSERTABLE, !join.insertable()); col.setFlag(Column.FLAG_UNUPDATABLE, !join.updatable()); return col; }
          Hide
          Michael Spiro added a comment -

          I opened a duplicate issue OPENJPA-2054.

          Pinaki, your patch handles only the case with a string constant but doesn`t handle numeric constants (beginning with a digit, a dot or a minus sign). "Null" should also be considered a constant. Shouldn't this be added? I think the parsing logic should be the same as in MappingInfo.mergeJoinColumn().

          Show
          Michael Spiro added a comment - I opened a duplicate issue OPENJPA-2054 . Pinaki, your patch handles only the case with a string constant but doesn`t handle numeric constants (beginning with a digit, a dot or a minus sign). "Null" should also be considered a constant. Shouldn't this be added? I think the parsing logic should be the same as in MappingInfo.mergeJoinColumn().
          Hide
          Pinaki Poddar added a comment -

          > XML mapping parser as well.
          Yes, of course.

          > lacking a jUnit f
          The test case patch has ASF license granted. One can consider including it. The test case does not conform setUp() routine though.

          Another couple of points:
          a) There are few more rules to determine whether a specification denotes constant-valued column (ref: MappingInfo.meregColumn(...) and assignment of the local boolean variable named 'constant'). The quick patch does not work out those rules.

          b) The downstream effect of changing DBIdentifierType to CONSTANT instead of usual COLUMN not investigated. It was just done to stop the columnName being delimited.

          Show
          Pinaki Poddar added a comment - > XML mapping parser as well. Yes, of course. > lacking a jUnit f The test case patch has ASF license granted. One can consider including it. The test case does not conform setUp() routine though. Another couple of points: a) There are few more rules to determine whether a specification denotes constant-valued column (ref: MappingInfo.meregColumn(...) and assignment of the local boolean variable named 'constant'). The quick patch does not work out those rules. b) The downstream effect of changing DBIdentifierType to CONSTANT instead of usual COLUMN not investigated. It was just done to stop the columnName being delimited.
          Hide
          Jeremy Bauer added a comment -

          Hey Pinaki. Your patch looks good to me. We may need to look at the XML mapping parser as well. It looks like we were sorely lacking a jUnit for this scenario...

          Show
          Jeremy Bauer added a comment - Hey Pinaki. Your patch looks good to me. We may need to look at the XML mapping parser as well. It looks like we were sorely lacking a jUnit for this scenario...
          Hide
          Pinaki Poddar added a comment -

          This is a patch to fix the problem.
          WARNING: Only partially tested.

          Show
          Pinaki Poddar added a comment - This is a patch to fix the problem. WARNING: Only partially tested.
          Hide
          Pinaki Poddar added a comment -

          The test case uses non-standard join for schema definition.
          The domain classes will fail to define database schema

          Show
          Pinaki Poddar added a comment - The test case uses non-standard join for schema definition. The domain classes will fail to define database schema

            People

            • Assignee:
              Pinaki Poddar
              Reporter:
              Pinaki Poddar
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development