OpenJPA
  1. OpenJPA
  2. OPENJPA-1376

@SequenceGenerator allocationSize incorrect implementation

    Details

      Description

      OpenJpa use allocationSize parameter on NativeJDBCSeq to set the sequence cache value.

      But one "JSR 220: Enterprise JavaBeansTM,Version 3.0" , the allocationSize is : "The amount to increment by when allocating sequence
      numbers from the sequence."

      Allocating is used to limit the number of access to the sequence or table not to set the sequence cache value !

      If allocationSize=50, the sequence need to be call one time on each 50 call on AbstractJDBCSeq.next().

      1 call on AbstractJDBCSeq.next(). call the sequence or table
      2 call : return previous value + 1;
      3 call : return previous value + 2;
      ...
      51 call :re-call the sequence
      52 return previous value + 1
      ...

        Issue Links

          Activity

          Hide
          Palmer Cox added a comment -

          The attached patch modifies the NativeJDBCSeq class to make use of the increment parameter. The modified class will only get the next value of the database sequence object once every increment number of invocations.

          The patch also modifies the meta-data parsers to use the allocationSize parameter to the SequenceGenerator's to set the increment size.

          Show
          Palmer Cox added a comment - The attached patch modifies the NativeJDBCSeq class to make use of the increment parameter. The modified class will only get the next value of the database sequence object once every increment number of invocations. The patch also modifies the meta-data parsers to use the allocationSize parameter to the SequenceGenerator's to set the increment size.
          Hide
          Milosz Tylenda added a comment -

          Palmer, thanks for the patch. I have modified it, e.g. allocate property is used instead of increment to be more consistent with table-based sequence, but the idea is the same. Committed.

          I also got rid of the auto-configuration support which was marked deprecated from the beginning of OpenJPA (2006) which was interferring with this change.

          Since this change poses some burden during migration, I will update the manual soon.

          Show
          Milosz Tylenda added a comment - Palmer, thanks for the patch. I have modified it, e.g. allocate property is used instead of increment to be more consistent with table-based sequence, but the idea is the same. Committed. I also got rid of the auto-configuration support which was marked deprecated from the beginning of OpenJPA (2006) which was interferring with this change. Since this change poses some burden during migration, I will update the manual soon.
          Hide
          Milosz Tylenda added a comment -

          Added other Fix Versions since code has been checked in.

          On branches earlier than 2.2, the behaviour is gated by the useNativeSequenceCache property of DBDictionary. Set it to true to enable this new functionality.

          Show
          Milosz Tylenda added a comment - Added other Fix Versions since code has been checked in. On branches earlier than 2.2, the behaviour is gated by the useNativeSequenceCache property of DBDictionary. Set it to true to enable this new functionality.
          Hide
          Milosz Tylenda added a comment -

          Should be: Set it to FALSE to enable this new functionality.

          Show
          Milosz Tylenda added a comment - Should be: Set it to FALSE to enable this new functionality.
          Hide
          Milosz Tylenda added a comment -

          Heath, I think the following code, found in a branch, needs correction. Won't we get too many "INCREMENT BY"s ?

          if (seq.getIncrement() > 1 && useNativeSequenceCache)

          { buf.append(" INCREMENT BY ").append(seq.getIncrement()); }

          else if ((seq.getIncrement() > 1) || (seq.getAllocate() > 1))

          { buf.append(" INCREMENT BY ").append(seq.getIncrement() * seq.getAllocate()); }

          if (seq.getIncrement() > 1)

          buf.append(" INCREMENT BY ").append(seq.getIncrement());

          Show
          Milosz Tylenda added a comment - Heath, I think the following code, found in a branch, needs correction. Won't we get too many "INCREMENT BY"s ? if (seq.getIncrement() > 1 && useNativeSequenceCache) { buf.append(" INCREMENT BY ").append(seq.getIncrement()); } else if ((seq.getIncrement() > 1) || (seq.getAllocate() > 1)) { buf.append(" INCREMENT BY ").append(seq.getIncrement() * seq.getAllocate()); } if (seq.getIncrement() > 1) buf.append(" INCREMENT BY ").append(seq.getIncrement());
          Hide
          Heath Thomann added a comment -

          Hello Milosz!! Excellent catch!! Thank you! If you would please, could you double check my work here to make sure we are on the same page? That is, in 1.2.x and 1.3.x, it looks like my changes to DBDictionary are correct. However, looking at the DBDictionary changes in 2.0.x and 2.1.x, I forgot to remove this 'if' block:

          if (seq.getIncrement() > 1)

          buf.append(" INCREMENT BY ").append(seq.getIncrement());

          Sound correct or am I still missing something?

          Show
          Heath Thomann added a comment - Hello Milosz!! Excellent catch!! Thank you! If you would please, could you double check my work here to make sure we are on the same page? That is, in 1.2.x and 1.3.x, it looks like my changes to DBDictionary are correct. However, looking at the DBDictionary changes in 2.0.x and 2.1.x, I forgot to remove this 'if' block: if (seq.getIncrement() > 1) buf.append(" INCREMENT BY ").append(seq.getIncrement()); Sound correct or am I still missing something?
          Hide
          Milosz Tylenda added a comment -

          Hi Heath! It sounds correct now. I will have a look at the other files and will let you know if I find anything.

          Show
          Milosz Tylenda added a comment - Hi Heath! It sounds correct now. I will have a look at the other files and will let you know if I find anything.
          Hide
          Albert Lee added a comment -

          Close issue in preparation for 2.2.0 release.

          Show
          Albert Lee added a comment - Close issue in preparation for 2.2.0 release.

            People

            • Assignee:
              Milosz Tylenda
              Reporter:
              Yves
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 8h
                8h
                Remaining:
                Remaining Estimate - 8h
                8h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development