WIP: fix SET WITHOUT OIDS, add SET WITH OIDS
We have an open problem with CVS HEAD that ALTER TABLE SET WITHOUT OIDS
causes problems:
http://archives.postgresql.org/pgsql-hackers/2008-11/msg00332.php
I opined at the time that what we really have here is a table whose
tuples do not match its declared rowtype, and that the proper fix is
to make SET WITHOUT OIDS rewrite the table to physically get rid of
the OIDs. The attached patch (which lacks doc changes or regression
tests as yet) does that, and also adds the inverse SET WITH OIDS
operation to do what you'd expect, ie, add an OID column if it isn't
there already.
The major objection to this would probably be that SET WITHOUT OIDS has
historically been a "free" catalog-change operation, and now it will be
expensive on large tables. But given that we've deprecated OIDs in user
tables since 8.0, I think most people have been through that conversion
already, or have decided to keep their OIDs anyway. I don't think it's
worth taking a continuing risk of backend bugs in order to make life a
bit easier for any remaining laggards.
A different discussion is whether it's appropriate to put in SET WITH
OIDS now, when we're well past feature freeze. If we stripped that out
of this patch it'd save a few dozen lines of code, but I'm not really
seeing the point. The asymmetry of having SET WITHOUT and not SET WITH
has always been an implementation artifact anyway.
Comments?
regards, tom lane
Tom Lane wrote:
The attached patch (which lacks doc changes or regression
tests as yet) does that, and also adds the inverse SET WITH OIDS
operation to do what you'd expect, ie, add an OID column if it isn't
there already.
Why would we add an operation to implement a deprecated feature? That
seems very strange.
(I have no problem with making SET WITHOUT OIDS rewrite the table.)
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
The attached patch (which lacks doc changes or regression
tests as yet) does that, and also adds the inverse SET WITH OIDS
operation to do what you'd expect, ie, add an OID column if it isn't
there already.
Why would we add an operation to implement a deprecated feature?
Well, it was always intended to be that way:
http://archives.postgresql.org/pgsql-patches/2002-12/msg00071.php
The originally submitted patch didn't work very well
http://archives.postgresql.org/pgsql-patches/2003-01/msg00011.php
and what we ended up doing was applying just the SET WITHOUT OIDS half
of it, but my feeling always was that that was for lack of round tuits
rather than that it was a good place to be. Given the implementation
at the time it would've taken a lot of extra code to do SET WITH OIDS,
so nobody did get around to it. But subsequent changes in the ALTER
code have made it possible to piggyback on ALTER ADD COLUMN easily ---
which is what this patch is trying to demonstrate.
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether, I'm not sure I could dispute it. But if we have the ability
to do that ISTM we should offer the reverse too.
regards, tom lane
Tom Lane píše v ne 08. 02. 2009 v 11:51 -0500:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether, I'm not sure I could dispute it. But if we have the
ability to do that ISTM we should offer the reverse too.
By my opinion TABLES with OIDs is obsolete feature. It make sense to
have SET WITHOUT OIDS, because it is useful when people will migrate
form 7.4 to 8.4. But opposite way does not make me sense, because I
think we want to remove OID TABLES in the future. I personally prefer to
say that 8.4 is last version which supports CREATE TABLE ... WITH OIDS.
Zdenek
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Tom Lane píše v ne 08. 02. 2009 v 11:51 -0500:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether, I'm not sure I could dispute it. But if we have the
ability to do that ISTM we should offer the reverse too.
By my opinion TABLES with OIDs is obsolete feature. It make sense to
have SET WITHOUT OIDS, because it is useful when people will migrate
form 7.4 to 8.4. But opposite way does not make me sense, because I
think we want to remove OID TABLES in the future. I personally prefer to
say that 8.4 is last version which supports CREATE TABLE ... WITH OIDS.
If we're going to do that we should do it *now*, not later, because
right now is when we have a bug that we could actually save some effort
on. In practice, since we have not ever suggested that we were actually
going to remove the feature, I don't believe that we can do that. Not
in 8.4, and not in 8.5 or any other near-future release either.
The larger point though is that unless we restructure the system to the
point of not using OIDs in system catalogs ... which ain't happening
... the amount of code we could save by removing OIDs for users is
vanishingly small. Probably on the rough order of 100 lines, and about
the same in documentation. (We couldn't, for instances, stop
documenting that OIDs exist.) Doesn't really seem worth breaking
applications for, even deprecated ones.
regards, tom lane
On Sun, Feb 8, 2009 at 11:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
The attached patch (which lacks doc changes or regression
tests as yet) does that, and also adds the inverse SET WITH OIDS
operation to do what you'd expect, ie, add an OID column if it isn't
there already.Why would we add an operation to implement a deprecated feature?
Well, it was always intended to be that way:
http://archives.postgresql.org/pgsql-patches/2002-12/msg00071.phpThe originally submitted patch didn't work very well
http://archives.postgresql.org/pgsql-patches/2003-01/msg00011.php
and what we ended up doing was applying just the SET WITHOUT OIDS half
of it, but my feeling always was that that was for lack of round tuits
rather than that it was a good place to be. Given the implementation
at the time it would've taken a lot of extra code to do SET WITH OIDS,
so nobody did get around to it. But subsequent changes in the ALTER
code have made it possible to piggyback on ALTER ADD COLUMN easily ---
which is what this patch is trying to demonstrate.Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether, I'm not sure I could dispute it. But if we have the ability
to do that ISTM we should offer the reverse too.
+1. I really hate it (in any application, not just PostgreSQL) when
there's an option to add something but not delete it, delete it but
not put it back, etc. Personally, *I* would not have spent the time
to implement SET WITH OIDS, but since we now have the patch, I'm 100%
in favor of applying it. Most likely, very few people will use it,
but it doesn't cost us anything either, so I'm unclear why we would
tell Tom to go back and rip that functionality back out of his patch.
Sounds like bikeshedding to me.
...Robert
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether,
+1 for removing it altogether. Row OIDs are and ugly wart :P
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
I don't understand what's wrong with the existing setup where DROP
OIDS is a free operation. And the space is cleaned up later when the
tuple is next written.
It seems exactly equivalent to how we handle DROP COLUMN where the
natt field of the tuple disagrees with the tuple descriptor and any
additional columns are implicitly null.
--
Greg
On 8 Feb 2009, at 23:12, David Fetter <david@fetter.org> wrote:
Show quoted text
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether,+1 for removing it altogether. Row OIDs are and ugly wart :P
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.comRemember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
David Fetter wrote:
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether,+1 for removing it altogether. Row OIDs are and ugly wart :P
That might be true but I know of apps that use them. Having the ability
to migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).
cheers
andrew
Greg Stark <greg.stark@enterprisedb.com> writes:
I don't understand what's wrong with the existing setup where DROP
OIDS is a free operation.
It breaks things, in particular
http://archives.postgresql.org/pgsql-hackers/2008-11/msg00332.php
We could kluge around that particular problem, but the objection
I have to doing so is I'm 100% certain it won't be the last such bug.
It seems exactly equivalent to how we handle DROP COLUMN
It is just about exactly *unlike* DROP COLUMN, because in DROP COLUMN
we retain a memory that there used to be a column there. A close
emulation of DROP COLUMN would involve inventing some representation of
"oidisdropped", and going through every one of the multitudinous places
that special-case dropped columns in order to see if each one needs a
similar special case for dropped OIDs. The bug mentioned above stems
directly from not expecting a table to still contain OIDs after SET
WITHOUT OIDS, so I don't think this parallel is mistaken.
Note that I'm willing to lay a significant side bet that we still have
bugs of omission with dropped columns, too. But we'll fix those as we
come to them. I don't think it is worth making a similar open-ended
commitment of resources just to keep SET WITHOUT OIDS fast.
... where the
natt field of the tuple disagrees with the tuple descriptor and any
additional columns are implicitly null.
No, that's the mechanism that makes ADD COLUMN feasible (and indeed
pretty easy). DROP COLUMN is the far newer and uglier mess around
attisdropped.
regards, tom lane
On 2/9/09, Andrew Dunstan <andrew@dunslane.net> wrote:
David Fetter wrote:
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether,+1 for removing it altogether. Row OIDs are and ugly wart :P
That might be true but I know of apps that use them. Having the ability to
migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).
+1 for removal.
Also, whether the removal happens or not, I would suggest a setting that
makes Postgres accept, but ignore default_with_oids / WITH OIDS settings.
The problem is how to migrate apps that definitely do not use oids,
in a situation where you have hundred of databases.
Scanning all dbs and doing ALTER table would be option, if it would
work 100% and would not touch data. Otherwise is cannot be used.
Trying to manually manipulate dump files which are filled with
"SET default_with_oids" each time database is dumped/reloaded is also
not an option.
Currently the only sane path seems to patch Postgres to ignore the
settings, but that does not seem very user-friendly approach...
--
marko
On Mon, Feb 09, 2009 at 02:47:21PM +0200, Marko Kreen wrote:
That might be true but I know of apps that use them. Having the ability to
migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).+1 for removal.
Also, whether the removal happens or not, I would suggest a setting that
makes Postgres accept, but ignore default_with_oids / WITH OIDS settings.
Err, you mean a setting that makes Postgres throw an error on the use
of WITH OIDS. Just silently ignoring the option is a fantastic way to
break applications silently.
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.
Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
Show quoted text
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
Martijn van Oosterhout wrote:
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.
Or just run ALTER TABLE WITHOUT OIDS for all the tables before dumping.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On 2/9/09, Martijn van Oosterhout <kleptog@svana.org> wrote:
On Mon, Feb 09, 2009 at 02:47:21PM +0200, Marko Kreen wrote:
That might be true but I know of apps that use them. Having the ability to
migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).+1 for removal.
Also, whether the removal happens or not, I would suggest a setting that
makes Postgres accept, but ignore default_with_oids / WITH OIDS settings.Err, you mean a setting that makes Postgres throw an error on the use
of WITH OIDS. Just silently ignoring the option is a fantastic way to
break applications silently.
For me, ignoring is easier... But yeah, error would be better,
if it does not affect reloading the dump.
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.
I don't like it - it would require more work from users, but does
not seem to be any way safer. You usually do the check if db works
on restore time, not dump time...
From clarity standpoint, options that turns both default_with_oids
and WITH OIDS to errors seems the best.
--
marko
On 2/9/09, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
Martijn van Oosterhout wrote:
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.Or just run ALTER TABLE WITHOUT OIDS for all the tables before dumping.
This does not work on dbs that are actually in use...
--
marko
On 2/9/09, Martijn van Oosterhout <kleptog@svana.org> wrote:
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.
OTOH, the pg_dump already has option --oids. If the option is not given,
is there any point putting WITH OIDS / default_with_oids into dump?
--
marko
On Mon, Feb 09, 2009 at 03:19:55PM +0200, Marko Kreen wrote:
Making pg_dump not output the WITH OIDS option on tables may be an
easier option.I don't like it - it would require more work from users, but does
not seem to be any way safer. You usually do the check if db works
on restore time, not dump time...
Another idea, have WITH OIDS just append a column to the table called
OID with SERIAL type. People see them, go "whoops" and drop them.
Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
Show quoted text
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
On Feb 9, 2009, at 7:47 AM, Marko Kreen <markokr@gmail.com> wrote:
On 2/9/09, Andrew Dunstan <andrew@dunslane.net> wrote:
David Fetter wrote:
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT
OIDS
altogether,+1 for removing it altogether. Row OIDs are and ugly wart :P
That might be true but I know of apps that use them. Having the
ability to
migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).+1 for removal.
Why? What benefit do we get out of denying users this option?
Also, whether the removal happens or not, I would suggest a setting
that
makes Postgres accept, but ignore default_with_oids / WITH OIDS
settings.The problem is how to migrate apps that definitely do not use oids,
in a situation where you have hundred of databases.Scanning all dbs and doing ALTER table would be option, if it would
work 100% and would not touch data. Otherwise is cannot be used.
That might be true in your environment, but is certainly not true in
general. We have many DDL commands that require full-table rewrites,
and they are FAR from useless.
Trying to manually manipulate dump files which are filled with
"SET default_with_oids" each time database is dumped/reloaded is also
not an option.Currently the only sane path seems to patch Postgres to ignore the
settings, but that does not seem very
...Robert
On 2/9/09, Robert Haas <robertmhaas@gmail.com> wrote:
On Feb 9, 2009, at 7:47 AM, Marko Kreen <markokr@gmail.com> wrote:
On 2/9/09, Andrew Dunstan <andrew@dunslane.net> wrote:
David Fetter wrote:
On Sun, Feb 08, 2009 at 11:51:22AM -0500, Tom Lane wrote:
Now, if you want to argue that we should get rid of SET WITHOUT OIDS
altogether,+1 for removing it altogether. Row OIDs are and ugly wart :P
That might be true but I know of apps that use them. Having the ability
to
migrate those slowly by using SET WITHOUT OIDS is a Good Thing (tm).
+1 for removal.
Why? What benefit do we get out of denying users this option?
Why should we continue to support historical special case? It is not
a feature that adds anything to user experience with Postgres.
Anyway, that was my vote only. If there are developers interested
in supporting oids feel free to do so.
But now that I learned that ALTER TABLE WITHOUT OIDS either causes bugs
or requires table rewrite, it turned from minor annoyance to big annoyance.
So I'd like have a reasonable path for getting rid of them, which we don't
have currently. Removing them completely is simplest path, but adding
extra features to support it is another.
If we are talking about adding a feature, then I like retargeting
pg_dump --oids from data-only flag to apply to both data and schema.
Yes, this is incompatible change, but the change affects feature we
are discouraging anyway.
If this does not work, then we need another postgresql.conf option.
Also, whether the removal happens or not, I would suggest a setting that
makes Postgres accept, but ignore default_with_oids / WITH OIDS settings.The problem is how to migrate apps that definitely do not use oids,
in a situation where you have hundred of databases.Scanning all dbs and doing ALTER table would be option, if it would
work 100% and would not touch data. Otherwise is cannot be used.That might be true in your environment, but is certainly not true in
general. We have many DDL commands that require full-table rewrites, and
they are FAR from useless.
Compared to not having the DDL commands or having DDL commands that
do not rewrite the tables? ;)
--
marko
Marko Kreen wrote:
But now that I learned that ALTER TABLE WITHOUT OIDS either causes bugs
or requires table rewrite, it turned from minor annoyance to big annoyance.
So I'd like have a reasonable path for getting rid of them, which we don't
have currently. Removing them completely is simplest path, but adding
extra features to support it is another.If we are talking about adding a feature, then I like retargeting
pg_dump --oids from data-only flag to apply to both data and schema.
Yes, this is incompatible change, but the change affects feature we
are discouraging anyway.
How about a pg_dump flag that simply suppresses OIDs from the data and
schema?
cheers
andrew
