Hi Tom
Thanks for your reply.
From what I understand of the code, a native sequence is created if the
user has specified GenerationType.IDENTITY or GenerationType.SEQUENCE.
For the GenerationType.SEQUENCE case if the user has specified the
sequence name this would be the sequence name used. But if the user has
not specified any additional information the code creates a default
native sequence of the name
oracle.toplink.essentials.internal.ejb.cmp3.metadata.MetadataConstants.DEFAULT_SEQUENCE_GENERATOR
("SEQ_GEN_SEQUENCE"). Hence by the time we reach this code the sequence
name should be already known as either a user specified one or one a
default one. This is what I found by debugging the code. Am I missing a
scenario where the sequence name could be null ?
Additionally should this code be wrapped in a try-catch block? In the
catch block should we just log the exceptions and not try to set the
sequence name. If there are exceptions reported does it imply that there
is something wrong in the user's application. The reason for asking this
question is for issues like :
http://forums.java.net/jive/thread.jspa?forumID=56&threadID=18031
that have been reported after the previous changes were checked in.
After having sent out this code, I had a general comment/question in
this area. Is there a way to know from a ClassDescriptor, if the user
has specified a generation type of Identity or Sequence ? Since
internally we are treating both to as a native sequence, we have a
problem. The changed code that I have sent out would end up treating
both sequence and identity as the same. I think the correct way to deal
with Postgres would be to treat Identity and Sequence as 2 seperate
schemes. In case the user has defined an Id generation scheme of
Identity, create a SERIAL field and ensure that we setup the correct
internal sequence structure with name as
"<fullyQualifiedTableName>_<ColumnName>_SEQ". But if the user has
specified an Id generation scheme of Sequence, do not generate a SERIAL
field. Create a field with a user defined type and assign the sequence
name to be the user defined one. In the code that is currently checked
in, we end up creating a SERIAL field irrespective of whether the user
has defined identity or sequence as the id generation scheme.
Ideally we would just want to reset the name of the sequence if the user
has defined an Identity generation scheme only. The only way I could
think of doing this in the current code is by changing the line :
if (null != sequence && sequence instanceof NativeSequence) {
to
if (null != sequence && sequence instanceof NativeSequence &&
sequence.getName().equals(oracle.toplink.essentials.internal.ejb.cmp3.metadata.MetadataConstants.DEFAULT_SEQUENCE_GENERATOR)
{
Thanks
Pramod
Tom Ware wrote:
> Hi Pramod,
>
> In general this code looks good. I just have one question:
>
> In the old code:
>
> boolean usesDefaultSequence = sequence == null || sequence
> instanceof DefaultSequence;
>
> It looks like if the sequence is null, we treat it as a DefaultSequence.
>
> In the new code:
>
> if (null != sequence && sequence instanceof NativeSequence) {
>
> It looks like we do not do anything for null sequences.
>
> Am I missing something with how we deal with these?
>
> -Tom
>
>
> Pramod Gopinath wrote:
>
>> Hi Andrei
>> For Postgresql, issue 283 was reopened based on problems that
>> users were facing. Based on further testing found that there was an
>> issue with the current code. Hence I am resending the changed code
>> for your review.
>>
>> _/*Problem Description :*/_
>> Let me start by listing down the problems with the existing code :
>> 1. If there were 2 entities, both defined with
>> @Id
>> @GeneratedValue(strategy = GenerationType.IDENTITY)
>> public Long longKey;
>>
>> then for the first entity we properly associate the primary key
>> field to be a "SERIAL" and things work correctly.
>> But for the second entity, we treat it as as if GenerationType.AUTO
>> was specified i.e. we *do not* create the field as "SERIAL" and
>> create a table of the form
>> "SEQUENCE(SEQ_NAME, SEQ_COUNT)" and inserting a row with value of
>> "SEQ_GEN_SEQUENCE" from which we get the id field value at insert time.
>>
>> 2. If we have 2 entities defined as
>> @Entity
>> @Table(name = "Test1")
>> public class TestEntity1 {
>> @Id
>> @GeneratedValue(strategy = GenerationType.SEQUENCE, generator =
>> "TEST_SEQ_GENERATOR")
>> @SequenceGenerator(name = "TEST_SEQ_GENERATOR", sequenceName =
>> "TEST_ID_SEQ")
>> protected Long longKey;
>> }
>>
>>
>> @Entity
>> @Table(name = "Test2")
>> public class TestEntity2 {
>> @Id
>> @GeneratedValue(strategy = GenerationType.SEQUENCE, generator =
>> "TEST_SEQ_GENERATOR")
>> @SequenceGenerator(name = "TEST_SEQ_GENERATOR", sequenceName =
>> "TEST_ID_SEQ")
>> protected Long longKey;
>> }
>>
>> then for one of the entites we would get the SERIAL field correctly
>> created with the implicit sequence name. But for the other entity we
>> create a default sequence table with a row inserted for the user
>> defined sequence "TEST_ID_SEQ"
>>
>> _/*Solution :*/_
>> The 2 problem areas in the code are :
>> 1. the if loop to get the value of "shouldVerifySequenceName"
>> Cannot we just simplify this completely and just deal with one
>> case i.e. if sequence is found in the sequences hashmap and it is a
>> NativeSequence then continue with doing what we want to do.
>> 2. the part where we do :
>> removeSequence(currSequenceName);
>> By removing the default sequence "SEQ_GEN_SEQUENCE" for the first
>> entity, subsequent entities would not find them and hence we end up
>> creating the table.
>>
>> I have attached the latest code changes for your review.
>>
>> Thanks
>> Pramod
>
>
>