Postgres Pro
Downloads
Home   >   mailing lists

Thread: creating a table with a serial column sets currval

creating a table with a serial column sets currval

From
Kris Jurka
Date:
jurka=# create table t (c serial);
NOTICE:  CREATE TABLE will create implicit sequence "t_c_seq" for serial
column "t.c"
CREATE TABLE
jurka=# select currval('t_c_seq');
  currval
---------
        1
(1 row)

I would expect it to say that currval wasn't set like so:

jurka=# create sequence myseq;
CREATE SEQUENCE
jurka=# select currval('myseq');
ERROR:  currval of sequence "myseq" is not yet defined in this session

Kris Jurka

Re: creating a table with a serial column sets currval

From
Kris Jurka
Date:
On Thu, 18 Oct 2007, Kris Jurka wrote:

>
> jurka=# create table t (c serial);
> NOTICE:  CREATE TABLE will create implicit sequence "t_c_seq" for serial
> column "t.c"
> CREATE TABLE
> jurka=# select currval('t_c_seq');
> currval
> ---------
>       1
> (1 row)
>
> I would expect it to say that currval wasn't set like so:
>

Looks like any alter sequence command will do this.  The serial case uses
alter sequence owned by under the hood which exposes this.  The problem is
that altering the sequence puts it into the SeqTable cache list when it
really shouldn't be.

Kris Jurka

Re: creating a table with a serial column sets currval

From
Tom Lane
Date:
Kris Jurka <books@ejurka.com> writes:
>> jurka=# create table t (c serial);
>> NOTICE:  CREATE TABLE will create implicit sequence "t_c_seq" for serial
>> column "t.c"
>> CREATE TABLE
>> jurka=# select currval('t_c_seq');
>> currval
>> ---------
>> 1
>> (1 row)
>>
>> I would expect it to say that currval wasn't set like so:

... as indeed it did say, up till 8.2, so I concur this is a bug.

> Looks like any alter sequence command will do this.  The serial case uses
> alter sequence owned by under the hood which exposes this.  The problem is
> that altering the sequence puts it into the SeqTable cache list when it
> really shouldn't be.

It's not that it gets put in the cache, it's that read_info gets called
(setting elm->increment).  I think we probably should clean this up by
creating a separate flag in that struct that explicitly says "currval is
valid", which would be set by nextval(), setval() (because historically
it's acted that way), and I guess ALTER SEQUENCE RESTART WITH (for
consistency with setval()).

Should any of the ALTER SEQUENCE options *reset* such a flag?
Offhand I don't see any...

            regards, tom lane

Re: creating a table with a serial column sets currval

From
Kris Jurka
Date:
On Thu, 18 Oct 2007, Tom Lane wrote:

>> Looks like any alter sequence command will do this.  The serial case uses
>> alter sequence owned by under the hood which exposes this.  The problem is
>> that altering the sequence puts it into the SeqTable cache list when it
>> really shouldn't be.
>
> It's not that it gets put in the cache, it's that read_info gets called
> (setting elm->increment).  I think we probably should clean this up by
> creating a separate flag in that struct that explicitly says "currval is
> valid", which would be set by nextval(), setval() (because historically
> it's acted that way), and I guess ALTER SEQUENCE RESTART WITH (for
> consistency with setval()).

Personally I think setval should only set validCurrval and the last_value
if iscalled = true.  If is_called = false I think it should retain the
previous last_value if any until the next nextval call.

jurka=# create sequence s;
CREATE SEQUENCE
jurka=# select nextval('s');
  nextval
---------
        1
(1 row)

jurka=# select setval('s',5, false);
  setval
--------
       5
(1 row)

jurka=# select currval('s');
  currval
---------
        5
(1 row)

Should return 1 instead of 5.

Kris Jurka

Re: creating a table with a serial column sets currval

From
Tom Lane
Date:
Kris Jurka <books@ejurka.com> writes:
> On Thu, 18 Oct 2007, Tom Lane wrote:
>> It's not that it gets put in the cache, it's that read_info gets called
>> (setting elm->increment).  I think we probably should clean this up by
>> creating a separate flag in that struct that explicitly says "currval is
>> valid", which would be set by nextval(), setval() (because historically
>> it's acted that way), and I guess ALTER SEQUENCE RESTART WITH (for
>> consistency with setval()).

> Personally I think setval should only set validCurrval and the last_value
> if iscalled = true.  If is_called = false I think it should retain the
> previous last_value if any until the next nextval call.

Hmm.  If we did that, then ALTER RESTART WITH (which always sets
iscalled false) shouldn't affect the flag either, which would likely
simplify matters a bit.  However, it would be a change in behavior,
so I'm not convinced we should back-patch 8.2 like that.

Any other opinions out there?

            regards, tom lane