Thread: Re: [BUGS] BUG #1883: Renaming a schema leaves inconsistent sequence
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > This item has been added to the 8.1 bugs list: > > http://momjian.postgresql.org/cgi-bin/pgbugs > > This isn't going to be fixed for 8.1. I think it's really a variant of > the TODO item > o %Have ALTER TABLE RENAME rename SERIAL sequence names Well, it might be a variant, but its failure is much worse. For a table rename, you just get a strange \d display:test=> CREATE TABLE test (x SERIAL);NOTICE: CREATE TABLE will create implicitsequence "test_x_seq" for serial column "test.x"CREATE TABLEtest=> ALTER TABLE test RENAME TO test2;ALTER TABLEtest=>INSERT INTO test2 VALUES (DEFAULT);INSERT 0 1test=> \d test2 Table "public.test2" Column| Type | Modifiers--------+---------+-----------------------------------------------------x | integer | not null default nextval('public.test_x_seq'::text) The insert into the table still works. For the schema rename, the insert into the table doesn't work anymore. The odds that a schema rename is going to have _no_ sequence dependencies in the same schema seems pretty unlikely, meaning rename schema is almost guarantted to create some broken table defaults. With this behavior, if we can't fix it in 8.1, I am wonderingf we should just disable the feature: test=> CREATE SCHEMA aa;CREATE SCHEMAtest=> CREATE TABLE aa.test (x SERIAL);NOTICE: CREATE TABLE will create implicit sequence"test_x_seq" for serial column "test.x"CREATE TABLEtest=> ALTER SCHEMA aa RENAME TO bb;ALTER SCHEMAtest=> INSERTINTO bb.test VALUES (DEFAULT);ERROR: SCHEMA "aa" does NOT existtest=> \d bb.test Table "bb.test"Column | Type | Modifiers--------+---------+-------------------------------------------------x | integer | not null default nextval('aa.test_x_seq'::text) -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
With the following errors caused by ALTER SCHEMA RENAME, I recommend we remove this feature for 8.1 and revisit it for 8.2. I would just remove the grammar construct for it and the documentation. To fix this, we would need to redesign the way we store DEFAULT sequence assignments, and I don't think that is a good thing to do during beta. I see people wanting bitmapped scans ASAP, not renaming of schemas. Our beta time is better spent on other things than getting this to work now. --------------------------------------------------------------------------- Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > This item has been added to the 8.1 bugs list: > > > http://momjian.postgresql.org/cgi-bin/pgbugs > > > > This isn't going to be fixed for 8.1. I think it's really a variant of > > the TODO item > > o %Have ALTER TABLE RENAME rename SERIAL sequence names > > Well, it might be a variant, but its failure is much worse. For a table > rename, you just get a strange \d display: > > test=> CREATE TABLE test (x SERIAL); > NOTICE: CREATE TABLE will create implicit sequence "test_x_seq" for serial column "test.x" > CREATE TABLE > test=> ALTER TABLE test RENAME TO test2; > ALTER TABLE > test=> INSERT INTO test2 VALUES (DEFAULT); > INSERT 0 1 > test=> \d test2 > Table "public.test2" > Column | Type | Modifiers > --------+---------+----------------------------------------------------- > x | integer | not null default nextval('public.test_x_seq'::text) > > The insert into the table still works. For the schema rename, the > insert into the table doesn't work anymore. The odds that a schema > rename is going to have _no_ sequence dependencies in the same schema > seems pretty unlikely, meaning rename schema is almost guarantted to > create some broken table defaults. With this behavior, if we can't fix > it in 8.1, I am wonderingf we should just disable the feature: > > test=> CREATE SCHEMA aa; > CREATE SCHEMA > test=> CREATE TABLE aa.test (x SERIAL); > NOTICE: CREATE TABLE will create implicit sequence "test_x_seq" for serial column "test.x" > CREATE TABLE > test=> ALTER SCHEMA aa RENAME TO bb; > ALTER SCHEMA > test=> INSERT INTO bb.test VALUES (DEFAULT); > ERROR: SCHEMA "aa" does NOT exist > test=> \d bb.test > Table "bb.test" > Column | Type | Modifiers > --------+---------+------------------------------------------------- > x | integer | not null default nextval('aa.test_x_seq'::text) > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, Pennsylvania 19073 > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> With the following errors caused by ALTER SCHEMA RENAME, I recommend we
> remove this feature for 8.1 and revisit it for 8.2.
It's a mistake to blame ALTER SCHEMA RENAME for this problem, as you can
cause it equally well by renaming the sequence itself, or by moving it
into another schema with ALTER TABLE SET SCHEMA. Will you also insist
on disabling the latter new feature?
I experimented a little bit with defining nextval() and friends as
taking "regclass" instead of text, and it seems like that works pretty
nicely for these problems, once you've got the literal in the form of
regclass (ie, internally an OID). For actual SERIAL columns that
doesn't seem like a big deal, because the default expression doesn't
appear literally in dumps (at least not dumps made with a recent version
of pg_dump). All we'd have to do is tweak the parser to generate a call
to nextval(regclass) instead of nextval(text) when expanding SERIAL.
For dumps that contain explicit calls, likekeycol int default nextval('foo'::text)
I really don't think there is anything much we can do :-( except to
recommend that people update the default expressions. You'd need to
change it tokeycol int default nextval('foo'::regclass)
to be safe against renamings of 'foo', and I don't see any very good
way to force that to happen automatically.
I think that a reasonable answer for 8.1 would be to add
nextval(regclass) (and I guess parallel versions of currval and setval,
too), leaving the existing text-based functions available, and modifying
the parser to use nextval(regclass) instead of nextval(text) in SERIAL
defaults.
In the long run it would be nice to deprecate and eventually remove
the text-based functions, but I don't see how to do that in the short
run without introducing an implicit text-to-regclass cast for
compatibility purposes. That seems a bit risky.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > With the following errors caused by ALTER SCHEMA RENAME, I recommend we
> > remove this feature for 8.1 and revisit it for 8.2.
>
> It's a mistake to blame ALTER SCHEMA RENAME for this problem, as you can
> cause it equally well by renaming the sequence itself, or by moving it
> into another schema with ALTER TABLE SET SCHEMA. Will you also insist
> on disabling the latter new feature?
Sure. (Silly question, silly answer.)
Seriously, you have to consider the likelihood of breakage, and the
surprise factor. If someone moves a sequence to another schema or
renames it, they would assume breakage, but moving all tables/sequences
together would not suggest sequence breakage.
So, below, you are saying that once 8.0.X dumps are loaded into 8.1,
that the renaming of those schemas would succeed, at least for SERIAL,
but not for manual sequence defaults. That seems OK, I guess, in the
hope that people who are creating defaults manually based on sequences
are going to know how to fix things.
---------------------------------------------------------------------------
> I experimented a little bit with defining nextval() and friends as
> taking "regclass" instead of text, and it seems like that works pretty
> nicely for these problems, once you've got the literal in the form of
> regclass (ie, internally an OID). For actual SERIAL columns that
> doesn't seem like a big deal, because the default expression doesn't
> appear literally in dumps (at least not dumps made with a recent version
> of pg_dump). All we'd have to do is tweak the parser to generate a call
> to nextval(regclass) instead of nextval(text) when expanding SERIAL.
>
> For dumps that contain explicit calls, like
> keycol int default nextval('foo'::text)
> I really don't think there is anything much we can do :-( except to
> recommend that people update the default expressions. You'd need to
> change it to
> keycol int default nextval('foo'::regclass)
> to be safe against renamings of 'foo', and I don't see any very good
> way to force that to happen automatically.
>
> I think that a reasonable answer for 8.1 would be to add
> nextval(regclass) (and I guess parallel versions of currval and setval,
> too), leaving the existing text-based functions available, and modifying
> the parser to use nextval(regclass) instead of nextval(text) in SERIAL
> defaults.
>
> In the long run it would be nice to deprecate and eventually remove
> the text-based functions, but I don't see how to do that in the short
> run without introducing an implicit text-to-regclass cast for
> compatibility purposes. That seems a bit risky.
>
> regards, tom lane
>
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073