Hi Pramod,
Some comments.
1. Is there any way this change can be isolated so that there is not a
method in DatabaseSessionImpl that only applies to PostGreSQL platform?
The DatabasePlatform subclasses are designed to be the mechanism for
dbms-specific functionality. A line like the following one is an
indication that the functionality should be designed in such a way that
it appears only DataasePlatform related code.
if (! getPlatform().isPostgreSQL())
return;
2. DatabaseSessionImpl should not rely on classes in the annotations
package. (import
oracle.toplink.essentials.internal.annotations.AnnotationsSequencingProcessor;)
Core TopLink classes must work for users that do not have Java 5. (Most
packages except oracle.toplink.essentials.ejb.cmp3,
oracle.toplink.essentials.internal.ejb.cmp3, and
oracle.toplink.essentials.internal.annotations are core packages. )
Since non-core classes do not have this restriction, a dependancy on a
non-core class from a core class should be avoided. I realize that
glassfish is compiled in Java 5, but the core classes will be made
available for JDK 1.4 users by Oracle.
-Tom
Pramod Gopinath wrote:
> Hi Tom
> These code changes deal with SERIAL field of Postgresql database.
> The related issue is
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=283.
>
> *Problem Description :*
> For an entity if the user has specified
> @Id
> @GeneratedValue(strategy=GenerationType.SEQUENCE)
>
> we generate a SERIAL field. For this type of field, postgresql
> database creates an implicit sequence in the database. Each time a row
> is inserted into this table, the next id field value would be picked
> from this implicit sequence. Currently we are forcing the user to also
> define the
> @SequenceGenerator annotation with the correct sequence name to ensure
> that our code works correctly.
>
> *Proposed Changes :*
> The code changes that I have attached deal with automatically setting
> this sequence name correctly for Postgresql platform. The files
> affected are :
>
> src/java/oracle/toplink/essentials/internal/databaseaccess/DatabasePlatform.java
> Added a new method : getDBSequenceName(String tableName, String
> pkFieldName) which returns null
>
> src/java/oracle/toplink/essentials/platform/database/PostgreSQLPlatform.java
> This platform ensures that we return back a String that would be of
> the form tableName_pkFieldName_seq
>
> src/java/oracle/toplink/essentials/internal/sessions/DatabaseSessionImpl.java
> This is where the actual logic is currently placed.
> I have added a new method resetSequenceName() and the logic for
> this method can be summed as :
> if it is PostgreSQLPlatform
> get a list of all the descriptors and iterate over them
> if the descriptor.hasSimplePrimaryKey()
> if the sequenceName is set on the descriptor
> and is equal to the default sequence ("SEQ_GEN_SEQUENCE")
> then
> get the tableName, primaryKeyFieldName
> from the descriptor
> call into the
> platform.getDBSequenceName(tableName, primaryKeyFieldName)
> create a NativeSequence using this
> name and add this native sequence into the model
> set the sequence NAme into the descriptor
> set a boolean flag to true.
> endif
> endif
> end loop
> if the flag is set to true it implies that we have
> atleast one descriptor whose sequenceName was changed
> recreate the sequence objects so that the native
> sequences that we created are setup correctly
> end if
>
>
> Currently this method resetSequnce() has been placed in the
> DatabaseSessionImpl() and is called after the call to
> initializeDescriptors() in postConnectDatasource().
> Did think of having this code in PostgreSQLPlatform but was not sure
> if that was the correct thing to do.
> Anyway it would be great to hear your comments on the changes.
>
> Thanks
> Pramod
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com