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
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com