Thread: pg_dump bug in 7.3.9 with sequences
Hello,
Ran into this little gem with a customer today:
This works:
create table foo (foo int not null, bar text);
create sequence foo_seq;
alter table foo alter column foo set default nextval('foo_seq');
pg_dump will correctly dump the table:
CREATE TABLE foo (
foo integer DEFAULT nextval('foo_seq'::text) NOT NULL,
bar text
);
--
-- TOC entry 3 (OID 107565218)
-- Name: foo_seq; Type: SEQUENCE; Schema: public; Owner: postgres
--
CREATE SEQUENCE foo_seq
START 1
INCREMENT 1
MAXVALUE 9223372036854775807
MINVALUE 1
CACHE 1;
However if you do the following:
create table foo (foo serial not null, bar text);
create sequence foo_seq;
alter table foo alter column foo set default nextval('foo_seq');
pg_dump will give you the following:
CREATE TABLE foo (
foo serial NOT NULL,
bar text
);
--
-- TOC entry 3 (OID 107566148)
-- Name: foo_seq; Type: SEQUENCE; Schema: public; Owner: postgres
--
CREATE SEQUENCE foo_seq
START 1
INCREMENT 1
MAXVALUE 9223372036854775807
MINVALUE 1
CACHE 1;
Which is wrong because we want the column foo to use a default of foo_seq
not foo_foo_seq.
Sincerely,
Joshua D. Drake
--
Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC
Postgresql support, programming shared hosting and dedicated hosting.
+1-503-667-4564 - jd@commandprompt.com - http://www.commandprompt.com
PostgreSQL Replicator -- production quality replication for PostgreSQL
Attachment
"Joshua D. Drake" <jd@commandprompt.com> writes:
> However if you do the following:
> create table foo (foo serial not null, bar text);
> create sequence foo_seq;
> alter table foo alter column foo set default nextval('foo_seq');
This is flat out pilot error: you do not get to mess with the default
expression of a SERIAL column, because it's part of the internal
implementation of the SERIAL pseudo-type. If I were going to do
anything about it, I'd patch ALTER TABLE to refuse the above command.
regards, tom lane
>>create table foo (foo serial not null, bar text);
>>create sequence foo_seq;
>>alter table foo alter column foo set default nextval('foo_seq');
>>
>>
>
>This is flat out pilot error: you do not get to mess with the default
>expression of a SERIAL column, because it's part of the internal
>implementation of the SERIAL pseudo-type. If I were going to do
>anything about it, I'd patch ALTER TABLE to refuse the above command.
>
>
It is not pilot error if PostgreSQL allows it. There is
nothing "illegal" about the above commands in their execution.
The pg_dump application should recognize that the object has
changed and react accordingly.
Let me elaborate. Look at the following table (I didn't design it):
rp_nuke_old=# \d nuke_bbtopics
Table "public.nuke_bbtopics"
Column | Type | Modifiers
---------------------+----------------+---------------------------------------------------------------
topic_id | integer | not null default
nextval('public.nuke_bbtopics_id_seq'::text)
forum_id | smallint | not null default '0'
topic_title | character(255) | not null default ''
topic_poster | integer | not null default '0'
topic_time | integer | not null default '0'
topic_views | integer | not null default '0'
topic_replies | integer | not null default '0'
topic_status | smallint | not null default '0'
topic_vote | smallint | not null default '0'
topic_type | smallint | not null default '0'
topic_last_post_id | integer | not null default '0'
topic_first_post_id | integer | not null default '0'
topic_moved_id | integer | not null default '0'
news_id | integer | not null default '0'
Indexes: nuke_bbtopics_pkey primary key btree (topic_id),
forum_id_nuke_bbtopics btree (forum_id),
nuke_bbtopics_news_id btree (news_id),
topic_last_post_id_nuke_bbtopics btree (topic_last_post_id),
topic_type_nuke_bbtopics btree (topic_type),
topic_vote_nuke_bbtopics btree (topic_vote)
Check constraints: "$1" (forum_id >= 0)
"$2" (topic_views >= 0)
"$3" (topic_replies >= 0)
"$4" (topic_last_post_id >= 0)
"$5" (topic_first_post_id >= 0)
"$6" (topic_moved_id >= 0)
Notice that topic_id is an integer with a default value of:
nextval('public.nuke_bbtopics_id_seq'::text) .
Now lets look at what pg_dump does to this table:
CREATE TABLE nuke_bbtopics (
topic_id serial NOT NULL,
forum_id smallint DEFAULT '0' NOT NULL,
topic_title character(255) DEFAULT '' NOT NULL,
topic_poster integer DEFAULT '0' NOT NULL,
topic_time integer DEFAULT '0' NOT NULL,
topic_views integer DEFAULT '0' NOT NULL,
topic_replies integer DEFAULT '0' NOT NULL,
topic_status smallint DEFAULT '0' NOT NULL,
topic_vote smallint DEFAULT '0' NOT NULL,
topic_type smallint DEFAULT '0' NOT NULL,
topic_last_post_id integer DEFAULT '0' NOT NULL,
topic_first_post_id integer DEFAULT '0' NOT NULL,
topic_moved_id integer DEFAULT '0' NOT NULL,
news_id integer DEFAULT '0' NOT NULL,
CONSTRAINT "$1" CHECK ((forum_id >= 0)),
CONSTRAINT "$2" CHECK ((topic_views >= 0)),
CONSTRAINT "$3" CHECK ((topic_replies >= 0)),
CONSTRAINT "$4" CHECK ((topic_last_post_id >= 0)),
CONSTRAINT "$5" CHECK ((topic_first_post_id >= 0)),
CONSTRAINT "$6" CHECK ((topic_moved_id >= 0))
);
Notice that pg_dump has changed the topic_id integer to the serial
psuedotype. Which when restored will create:
Table "public.nuke_bbtopics"
Column | Type |
Modifiers
---------------------+----------------+---------------------------------------------------------------------
topic_id | integer | not null default
nextval('public.nuke_bbtopics_topic_id_seq'::text)
forum_id | smallint | not null default '0'
topic_title | character(255) | not null default ''
topic_poster | integer | not null default '0'
topic_time | integer | not null default '0'
topic_views | integer | not null default '0'
topic_replies | integer | not null default '0'
topic_status | smallint | not null default '0'
topic_vote | smallint | not null default '0'
topic_type | smallint | not null default '0'
topic_last_post_id | integer | not null default '0'
topic_first_post_id | integer | not null default '0'
topic_moved_id | integer | not null default '0'
news_id | integer | not null default '0'
Check constraints: "$1" (forum_id >= 0)
"$2" (topic_views >= 0)
"$3" (topic_replies >= 0)
"$4" (topic_last_post_id >= 0)
"$5" (topic_first_post_id >= 0)
"$6" (topic_moved_id >= 0)
So in the end I have a table with a column topic_id that is an integer
that points to the WRONG sequence.
Sincerely,
Joshua D. Drake
> regards, tom lane
>
>
--
Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC
Postgresql support, programming shared hosting and dedicated hosting.
+1-503-667-4564 - jd@commandprompt.com - http://www.commandprompt.com
PostgreSQL Replicator -- production quality replication for PostgreSQL
Attachment
On Wed, Feb 02, 2005 at 01:54:48PM -0800, Joshua D. Drake wrote: > It is not pilot error if PostgreSQL allows it. There is > nothing "illegal" about the above commands in their execution. > The pg_dump application should recognize that the object has > changed and react accordingly. ISTM this is a bug, but it's not clear to me what is the solution. I can think of two: 1. Changing the default is forbidden 2. When the default is changed, the dependency on the sequence is dropped, and the sequence itself is dropped. Which one do you think is best? Why? (I'd say "less bad" instead of "best", but I'm not sure if that's a correct choice of words.) -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Someone said that it is at least an order of magnitude more work to do production software than a prototype. I think he is wrong by at least an order of magnitude." (Brian Kernighan)
Alvaro Herrera wrote: >On Wed, Feb 02, 2005 at 01:54:48PM -0800, Joshua D. Drake wrote: > > > >>It is not pilot error if PostgreSQL allows it. There is >>nothing "illegal" about the above commands in their execution. >>The pg_dump application should recognize that the object has >>changed and react accordingly. >> >> > >ISTM this is a bug, but it's not clear to me what is the solution. >I can think of two: > >1. Changing the default is forbidden >2. When the default is changed, the dependency on the sequence is >dropped, and the sequence itself is dropped. > > 3. When the default is changed, the dependency is updated to reflect the new sequence. The old sequence is left intact as an independent object. Sincerely, Joshua D. Drake >Which one do you think is best? Why? (I'd say "less bad" instead of >"best", but I'm not sure if that's a correct choice of words.) > > > -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-667-4564 - jd@commandprompt.com - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL
Attachment
On Wed, Feb 02, 2005 at 03:49:59PM -0800, Joshua D. Drake wrote: > Alvaro Herrera wrote: > > >On Wed, Feb 02, 2005 at 01:54:48PM -0800, Joshua D. Drake wrote: > > > >>It is not pilot error if PostgreSQL allows it. There is > >>nothing "illegal" about the above commands in their execution. > >>The pg_dump application should recognize that the object has > >>changed and react accordingly. > > > >ISTM this is a bug, but it's not clear to me what is the solution. > > 3. When the default is changed, the dependency is updated > to reflect the new sequence. The old sequence is left intact > as an independent object. It seems reasonable to update the dependency. But it isn't reasonable to leave the old sequence intact, because it is an internal implementation detail that should not be left around. It would be a bug, because later when the table is dropped then you have a dangling object; this behavior would be equivalent to leaving the original sequence around when the table is dropped, which is exactly the scenario dependencies were written for. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) You liked Linux a lot when he was just the gawky kid from down the block mowing your lawn or shoveling the snow. But now that he wants to date your daughter, you're not so sure he measures up. (Larry Greenemeier)
"Joshua D. Drake" <jd@commandprompt.com> writes:
> Alvaro Herrera wrote:
>> ISTM this is a bug, but it's not clear to me what is the solution.
>> I can think of two:
>>
>> 1. Changing the default is forbidden
>> 2. When the default is changed, the dependency on the sequence is
>> dropped, and the sequence itself is dropped.
> 3. When the default is changed, the dependency is updated
> to reflect the new sequence. The old sequence is left intact
> as an independent object.
What exactly is the use-case of that (or any other manipulation of a
serial column's default)? There is no point that I can see in just
rolling one sequence object into a serial in place of another. Whatever
parameter change you might need to accomplish can be done with ALTER
SEQUENCE on the original sequence, without replacing the object per se.
(Except for renaming it; but given the way pg_dump handles this stuff,
you do not actually have the option to control the sequence name anyway.)
I also think that altering the default expression is useless --- it's
not a serial column anymore if you do that. It might be worth trying to
teach ALTER COLUMN TYPE to handle the cases of switching a serial column
to a non-serial type or vice versa, but I don't think users should be
allowed to reach in and mess with the default directly.
In short I vote for #1. If you want to support #2 then teach ALTER
COLUMN TYPE to handle it. #3 is simply pointless.
BTW, experimenting with this reveals a different pg_dump issue, which is
that it will not replicate a nondefault set of sequence parameters for a
serial sequence. For instance
dtest=# create table t1 (f1 serial);
NOTICE: CREATE TABLE will create implicit sequence "t1_f1_seq" for serial column "t1.f1"
CREATE TABLE
dtest=# alter sequence t1_f1_seq cycle;
ALTER SEQUENCE
pg_dump will just emit "create table t1 (f1 serial)" with no hint that
the sequence ought to be set to CYCLE mode. I'm not sure about an
appropriate fix offhand --- we can't very well use ALTER SEQUENCE in
just this way in the dump, because of the risk of the sequence name
being possibly different at reload. (Come to think of it, we are not
very good about propagating GRANTs on the sequence either, because of
the same risk.)
regards, tom lane
>>3. When the default is changed, the dependency is updated >>to reflect the new sequence. The old sequence is left intact >>as an independent object. >> >> > >What exactly is the use-case of that (or any other manipulation of a >serial column's default)? There is no point that I can see in just >rolling one sequence object into a serial in place of another. Whatever >parameter change you might need to accomplish can be done with ALTER >SEQUENCE on the original sequence, without replacing the object per se. >(Except for renaming it; but given the way pg_dump handles this stuff, >you do not actually have the option to control the sequence name anyway.) > > O.k. I will buy that. So I say: #3 rev2: When the default is changed, the dependency is updated to reflect the new sequence and the old sequence is dropped. >I also think that altering the default expression is useless --- it's >not a serial column anymore if you do that. It might be worth trying to >teach ALTER COLUMN TYPE to handle the cases of switching a serial column >to a non-serial type or vice versa, but I don't think users should be >allowed to reach in and mess with the default directly. > > Well that would be fine if pg_dump actually handled the scenario I presented in my previous email correctly. The problem is you have situations where colummns became serial columns after the fact or they are columns that were created in a dataset before there was a serial data type (such as 7.2). Sincerely, Joshua D. Drake >In short I vote for #1. If you want to support #2 then teach ALTER >COLUMN TYPE to handle it. #3 is simply pointless. > >BTW, experimenting with this reveals a different pg_dump issue, which is >that it will not replicate a nondefault set of sequence parameters for a >serial sequence. For instance > >dtest=# create table t1 (f1 serial); >NOTICE: CREATE TABLE will create implicit sequence "t1_f1_seq" for serial column "t1.f1" >CREATE TABLE >dtest=# alter sequence t1_f1_seq cycle; >ALTER SEQUENCE > >pg_dump will just emit "create table t1 (f1 serial)" with no hint that >the sequence ought to be set to CYCLE mode. I'm not sure about an >appropriate fix offhand --- we can't very well use ALTER SEQUENCE in >just this way in the dump, because of the risk of the sequence name >being possibly different at reload. (Come to think of it, we are not >very good about propagating GRANTs on the sequence either, because of >the same risk.) > > regards, tom lane > >---------------------------(end of broadcast)--------------------------- >TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > > -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-667-4564 - jd@commandprompt.com - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL
Attachment
> pg_dump will just emit "create table t1 (f1 serial)" with no hint that > the sequence ought to be set to CYCLE mode. I'm not sure about an > appropriate fix offhand --- we can't very well use ALTER SEQUENCE in > just this way in the dump, because of the risk of the sequence name > being possibly different at reload. (Come to think of it, we are not > very good about propagating GRANTs on the sequence either, because of > the same risk.) I suggest some time ago an alternative syntax for ALTER SEQUENCE. So, you'd have: ALTER SEQUENCE seqname ... or ALTER SEQUENCE ON table.column ... or something. Then it would be similar to have pg_get_serial_sequence() works. Chris
On Wed, 2 Feb 2005 19:04:04 -0300, Alvaro Herrera
<alvherre@dcc.uchile.cl> wrote:
> > It is not pilot error if PostgreSQL allows it. There is
> > nothing "illegal" about the above commands in their execution.
> > The pg_dump application should recognize that the object has
> > changed and react accordingly.
>
> ISTM this is a bug, but it's not clear to me what is the solution.
> I can think of two:
>
> 1. Changing the default is forbidden
> 2. When the default is changed, the dependency on the sequence is
> dropped, and the sequence itself is dropped.
In my humble opinion, the command:
ALTER TABLE ALTER COLUMN fooser SET DEFAULT nextval('aaa_seq');
should be equivalent to:
ALTER TABLE ALTER COLUMN fooser TYPE integer;
ALTER TABLE ALTER COLUMN fooser SET DEFAULT nextval('aaa_seq');
...ie, if we mess with 'default' it is no longer a serial, but a plain
integer with default sequence.
Regards, Dawid
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> pg_dump will just emit "create table t1 (f1 serial)" with no hint that
>> the sequence ought to be set to CYCLE mode. I'm not sure about an
>> appropriate fix offhand --- we can't very well use ALTER SEQUENCE in
>> just this way in the dump, because of the risk of the sequence name
>> being possibly different at reload. (Come to think of it, we are not
>> very good about propagating GRANTs on the sequence either, because of
>> the same risk.)
> I suggest some time ago an alternative syntax for ALTER SEQUENCE.
Could work, but we'd have to support it for GRANT/REVOKE too, which is
starting to get ugly. (Now, if we went down the recently suggested path
of allowing sub-selects to compute the target object names for all
utility statements, this would fall out nicely. But having a special
case in ALTER SEQUENCE and GRANT seems pretty wart-ish.)
Another alternative I was thinking about in the shower this morning is
to have pg_dump treat the sequence as an independent object. So the
dump script would CREATE it, set parameters and GRANTs, just the same as
for a plain sequence, and then do a magic ALTER TABLE command that
attaches it to the serial column, which the script would initially
declare as a plain integer or bigint. This seems relatively clean to me
because it is in the same spirit as the way we handle index constraints
now: they aren't in the initial table definition but get added by ALTER
at a suitable time. However the "magic command" is a big wart of its
own, no doubt. It's not just an ALTER COLUMN TYPE command because you'd
need to be able to specify the name of the sequence to attach. It's not
just an ALTER SET DEFAULT, either, because it would have special
side-effects on pg_depend.
regards, tom lane
Tom Lane wrote:
> Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>
>>>pg_dump will just emit "create table t1 (f1 serial)" with no hint that
>>>the sequence ought to be set to CYCLE mode. I'm not sure about an
>>>appropriate fix offhand --- we can't very well use ALTER SEQUENCE in
>>>just this way in the dump, because of the risk of the sequence name
>>>being possibly different at reload. (Come to think of it, we are not
>>>very good about propagating GRANTs on the sequence either, because of
>>>the same risk.)
>
>
>>I suggest some time ago an alternative syntax for ALTER SEQUENCE.
>
>
> Could work, but we'd have to support it for GRANT/REVOKE too, which is
> starting to get ugly. (Now, if we went down the recently suggested path
> of allowing sub-selects to compute the target object names for all
> utility statements, this would fall out nicely. But having a special
> case in ALTER SEQUENCE and GRANT seems pretty wart-ish.)
>
> Another alternative I was thinking about in the shower this morning is
> to have pg_dump treat the sequence as an independent object. So the
> dump script would CREATE it, set parameters and GRANTs, just the same as
> for a plain sequence, and then do a magic ALTER TABLE command that
> attaches it to the serial column, which the script would initially
> declare as a plain integer or bigint. This seems relatively clean to me
> because it is in the same spirit as the way we handle index constraints
> now: they aren't in the initial table definition but get added by ALTER
> at a suitable time. However the "magic command" is a big wart of its
> own, no doubt. It's not just an ALTER COLUMN TYPE command because you'd
> need to be able to specify the name of the sequence to attach. It's not
> just an ALTER SET DEFAULT, either, because it would have special
> side-effects on pg_depend.
Wouldn't a lot of this be solved (I know I am over simplyfing) by making
serial a real type? E.g; if you have type serial it is type serial not
type integer with a default of nextval('sequence'). Thus if I have an
integer with a default of (anything really) that is how it is restore.
If I have a serial, it is a serial and is restored in that manner.
Using this idea, you would get the can't alter default of a serial but
also the ability to alter the default if it is NOT a serial but will
still auto-increment.
Sincerely,
Joshua D. Drake
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
--
Command Prompt, Inc., your source for PostgreSQL replication,
professional support, programming, managed services, shared
and dedicated hosting. Home of the Open Source Projects plPHP,
plPerlNG, pgManage, and pgPHPtoolkit.
Contact us now at: +1-503-667-4564 - http://www.commandprompt.com
Attachment
"Joshua D. Drake" <jd@commandprompt.com> writes:
> Wouldn't a lot of this be solved (I know I am over simplyfing) by making
> serial a real type?
Not that I can see. You still have to get the sequence object from
someplace, and having it be a distinct datatype is zero help for finding
a column-specific sequence.
regards, tom lane
> Not that I can see. You still have to get the sequence object from > someplace, and having it be a distinct datatype is zero help for finding > a column-specific sequence. What about allowing sequence qualifiers in the SERIAL definition? Chris