ADD/DROP INHERITS

Greg Stark wrote:

I can send the actual patch to psql-patches, it includes some other changes to
refactor StoreCatalogInheritance and add the syntax to gram.y. But it's still
not quite finished because of default values.

You can send what you've got, and note that it's not for application
yet. Post early and post often ;-) There are a surprising number of
things to be done when you play with the syntax, as I found out not too
long ago.

cheers

andrew

Greg Stark <gsstark@mit.edu> writes:

I've implemented most of ADD/DROP INHERITS but it's my first significant piece
of code at this level. I would appreciate any feedback about it.

I thought we had agreed that the semantics of ADD INHERITS would be to
reject the command if the child wasn't already suitable to be a child
of the parent. Not to modify it by adding columns or constraints or
whatever. For the proposed uses of ADD INHERITS (in particular,
linking and unlinking partition tables) an incompatibility in schema
almost certainly means you made a mistake, and you don't really want
the system helpfully "fixing" your table to match the parent.

regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:

I thought we had agreed that the semantics of ADD INHERITS would be to
reject the command if the child wasn't already suitable to be a child
of the parent. Not to modify it by adding columns or constraints or
whatever. For the proposed uses of ADD INHERITS (in particular,
linking and unlinking partition tables) an incompatibility in schema
almost certainly means you made a mistake, and you don't really want
the system helpfully "fixing" your table to match the parent.

I didn't see any discussion like that and I find it pretty surprising.

Personally I would have agreed. For partitioned tables you certainly don't
want it to create new columns without warning you.

But that's entirely inconsistent with the way inherited tables work in
general. It seems to go against the grain of Postgres's general style to
implement just the use case that's useful for a particular application rather
than keep the features logically consistent with each other.

Perhaps there should be an option when issuing the ADD INHERITS to indicate
whether you want it to create new columns or only match existing columns. That
would also give me a convenient excuse to skip all those NOTICEs about merging
column definitions.

Actually I think in the long term for partitioned tables Postgres will have to
implement a special syntax just like Oracle and other databases. The user
doesn't really want to have to manually manage all the partitions as tables.
That imposes a lot of extra work to have to define the tables with the right
syntax, maintain the constraints properly, etc.

For the user it would be better to have a single property of the partitioned
table that specified the partition key. Then when adding a partition you would
only have to specify the key range it covers, not write an arbitrary
constraint from scratch. Nor would you have to create an empty table with the
proper definition first then add it in.

--
greg

Greg Stark <gsstark@mit.edu> writes:

Tom Lane <tgl@sss.pgh.pa.us> writes:

I thought we had agreed that the semantics of ADD INHERITS would be to
reject the command if the child wasn't already suitable to be a child
of the parent.

I didn't see any discussion like that and I find it pretty surprising.

I'm pretty sure it was mentioned somewhere along the line.

But that's entirely inconsistent with the way inherited tables work in
general.

I don't see any basis for that conclusion. The properties of a table
are set when it's created and you need to do pretty explicit ALTERs to
change them. We do not for example automatically make a unique index
for a table when someone tries to reference a foreign key to a column
set that doesn't already have such an index.

In this situation, I think it's entirely reasonable to expect the user
to do any needed ALTER ADD COLUMN, CONSTRAINT, etc commands before
trying to attach a child table to a parent. Having the system do it
for you offers no functionality gain, just a way to shoot yourself in
the foot.

regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:

Greg Stark <gsstark@mit.edu> writes:

Tom Lane <tgl@sss.pgh.pa.us> writes:

I thought we had agreed that the semantics of ADD INHERITS would be to
reject the command if the child wasn't already suitable to be a child
of the parent.

I didn't see any discussion like that and I find it pretty surprising.

I'm pretty sure it was mentioned somewhere along the line.

But that's entirely inconsistent with the way inherited tables work in
general.

I don't see any basis for that conclusion. The properties of a table
are set when it's created and you need to do pretty explicit ALTERs to
change them.

It just seems weird for:

CREATE TABLE foo (x,y,z) INHERITS (bar)

to not be the equivalent to:

CREATE TABLE foo (x,y,z)
ALTER TABLE foo ADD INHERITS bar

We do not for example automatically make a unique index for a table when
someone tries to reference a foreign key to a column set that doesn't
already have such an index.

But that's not really the same thing. Whether you add the foreign key later or
when you initially create the table it never creates that index.

On the other hand if you add a column to the parent it doesn't complain if not
all the children already have that column -- it goes and adds it recursively.

In this situation, I think it's entirely reasonable to expect the user
to do any needed ALTER ADD COLUMN, CONSTRAINT, etc commands before
trying to attach a child table to a parent. Having the system do it
for you offers no functionality gain, just a way to shoot yourself in
the foot.

Well if that's the consensus feeling then it certainly makes my life easier.

--
greg

Greg Stark <gsstark@mit.edu> writes:

Tom Lane <tgl@sss.pgh.pa.us> writes:

In this situation, I think it's entirely reasonable to expect the user
to do any needed ALTER ADD COLUMN, CONSTRAINT, etc commands before
trying to attach a child table to a parent. Having the system do it
for you offers no functionality gain, just a way to shoot yourself in
the foot.

Well if that's the consensus feeling then it certainly makes my life easier.

Well, one reason for my position is exactly to make your life easier.
I think that making ADD INHERITS do all these other things automagically
is lily-gilding, or at least implementing features not shown to be
needed. Let's make it do the minimum needed for the use-cases cited so
far --- we can always add more functionality later, *after* it's proven
needed.

regards, tom lane

On Wed, Jun 07, 2006 at 03:33:54PM -0400, Greg Stark wrote:

Perhaps there should be an option when issuing the ADD INHERITS to indicate
whether you want it to create new columns or only match existing columns. That
would also give me a convenient excuse to skip all those NOTICEs about merging
column definitions.

+1, but I also agree with Tom that this doesn't need to be in the first
pass.

Actually I think in the long term for partitioned tables Postgres will have to
implement a special syntax just like Oracle and other databases. The user
doesn't really want to have to manually manage all the partitions as tables.
That imposes a lot of extra work to have to define the tables with the right
syntax, maintain the constraints properly, etc.

For the user it would be better to have a single property of the partitioned
table that specified the partition key. Then when adding a partition you would
only have to specify the key range it covers, not write an arbitrary
constraint from scratch. Nor would you have to create an empty table with the
proper definition first then add it in.

I think this is on the TODO list; it's just a matter of actually doing
it. A good first step would be creating an easy means to create an
inherited table that contained everything the parent did; constraints,
indexes, etc. After that's in place, it's easier to create a new
partition (constraints and all) with a single command.

Note that there's no reason this *has* to be in the backend; someone
could do it as a pgFoundry project. Of course long-term it would be best
if it was included, but that's probably more involved, especially for a
newer coder.
--
Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com
Pervasive Software http://pervasive.com work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461

Ühel kenal päeval, K, 2006-06-07 kell 15:33, kirjutas Greg Stark:

Tom Lane <tgl@sss.pgh.pa.us> writes:

I thought we had agreed that the semantics of ADD INHERITS would be to
reject the command if the child wasn't already suitable to be a child
of the parent. Not to modify it by adding columns or constraints or
whatever. For the proposed uses of ADD INHERITS (in particular,
linking and unlinking partition tables) an incompatibility in schema
almost certainly means you made a mistake, and you don't really want
the system helpfully "fixing" your table to match the parent.

I didn't see any discussion like that and I find it pretty surprising.

I'm pretty sure that what was discussed was just attaching/detaching
child tables into inheritance chains with no table alterations.

Maybe it was never mentioned explicitly, but that was how I understood
the discussion.

Personally I would have agreed. For partitioned tables you certainly don't
want it to create new columns without warning you.

Exactly!

But that's entirely inconsistent with the way inherited tables work in
general. It seems to go against the grain of Postgres's general style to
implement just the use case that's useful for a particular application rather
than keep the features logically consistent with each other.

There are too many conflicting definitions of "logically consistent", so
doing the bare minimum is the best way to avoid the whole problem.

Perhaps there should be an option when issuing the ADD INHERITS to indicate
whether you want it to create new columns or only match existing columns. That
would also give me a convenient excuse to skip all those NOTICEs about merging
column definitions.

nonono! the whole pg inheritance/partitioning thing is still quite
low-level and ADD/DEL INHERITS is the wrong place to start fixing it.

Actually I think in the long term for partitioned tables Postgres will have to
implement a special syntax just like Oracle and other databases. The user
doesn't really want to have to manually manage all the partitions as tables.
That imposes a lot of extra work to have to define the tables with the right
syntax, maintain the constraints properly, etc.

Yes. Maybe. But this is something that requires much more thought and
planning than adding the simplest possible ADD/DELETE INHERITS.

For the user it would be better to have a single property of the partitioned
table that specified the partition key. Then when adding a partition you would
only have to specify the key range it covers, not write an arbitrary
constraint from scratch. Nor would you have to create an empty table with the
proper definition first then add it in.

Don't try to solve too many problems at once. Starting with just a
possibility to move suitable ready-made partitions in and out of
inheritance chain solves a really big problem. No need to try to
obfuscate it with extra functionality, at least not initially.

--
----------------
Hannu Krosing
Database Architect
Skype Technologies OÜ
Akadeemia tee 21 F, Tallinn, 12618, Estonia

Skype me: callto:hkrosing
Get Skype for free: http://www.skype.com

Greg Stark said:

Greg Stark <gsstark@MIT.EDU> writes:

How does

ALTER TABLE table INHERITS ADD parent
ALTER TABLE table INHERITS DROP parent

sound?

I'll admit it doesn't read very well but it doesn't necessitate
complicating other rules in gram.y

Or alternatively if people want to keep English-like SQL style grammar:

ALTER TABLE table INHERIT parent
ALTER TABLE table NO INHERIT parent

That could work ... or maybe UNINHERIT would read better than NO INHERIT.

cheers

andrew

How does

ALTER TABLE table INHERITS ADD parent
ALTER TABLE table INHERITS DROP parent

sound?

I'll admit it doesn't read very well but it doesn't necessitate complicating
other rules in gram.y

--
greg

Greg Stark <gsstark@MIT.EDU> writes:

How does

ALTER TABLE table INHERITS ADD parent
ALTER TABLE table INHERITS DROP parent

sound?

I'll admit it doesn't read very well but it doesn't necessitate complicating
other rules in gram.y

Or alternatively if people want to keep English-like SQL style grammar:

ALTER TABLE table INHERIT parent
ALTER TABLE table NO INHERIT parent

--
greg

On Jun 8, 2006, at 9:13 , Greg Stark wrote:

Greg Stark <gsstark@MIT.EDU> writes:

How does

ALTER TABLE table INHERITS ADD parent
ALTER TABLE table INHERITS DROP parent

sound?

I'll admit it doesn't read very well but it doesn't necessitate
complicating
other rules in gram.y

Or alternatively if people want to keep English-like SQL style
grammar:

ALTER TABLE table INHERIT parent
ALTER TABLE table NO INHERIT parent

ALTER TABLE table DISOWN parent?

Michael Glaesemann
grzm seespotcode net

"Andrew Dunstan" <andrew@dunslane.net> writes:

Greg Stark said:

Or alternatively if people want to keep English-like SQL style grammar:

ALTER TABLE table INHERIT parent
ALTER TABLE table NO INHERIT parent

That could work ... or maybe UNINHERIT would read better than NO INHERIT.

DISINHERIT maybe?

While creating unreserved keywords isn't the end of the world it seems better
to stick to the vocabulary already there if possible. It makes it easier for
the user to remember how to spell commands. That's why I didn't suggest fixing
the DROP INHERITS ambiguity by inventing something like REMOVE INHERITS.

--
greg

grzm,

ALTER TABLE table DISOWN parent?

You can't disown your parents. ;-)

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Greg Stark <gsstark@MIT.EDU> writes:

While creating unreserved keywords isn't the end of the world it seems better
to stick to the vocabulary already there if possible. It makes it easier for
the user to remember how to spell commands.

+1. Don't invent new keywords (even if unreserved) when there's no
strong reason to do so.

regards, tom lane

But that's entirely inconsistent with the way inherited tables

work

in general.

I don't see any basis for that conclusion. The properties of a

table

are set when it's created and you need to do pretty explicit ALTERs

to

change them.

It just seems weird for:

CREATE TABLE foo (x,y,z) INHERITS (bar)

to not be the equivalent to:

CREATE TABLE foo (x,y,z)
ALTER TABLE foo ADD INHERITS bar

Imho the op should only choose that path if he wants to fill the table
before
adding the inheritance. It makes no sense to add columns with default
values
to existing rows of the child table, especially when you inherit the
defaults
from the parent.

So I agree with Tom, that ADD INHERITS should not add columns.

Andreas

"Zeugswetter Andreas DCP SD" <ZeugswetterA@spardat.at> writes:

Imho the op should only choose that path if he wants to fill the table
before adding the inheritance. It makes no sense to add columns with default
values to existing rows of the child table, especially when you inherit the
defaults from the parent.

We already have ALTER TABLE ADD COLUMN working for columns with defaults, so I
think that horse has left the barn. It was awfully annoying for users when
that feature was missing. Any non-linearities in the user interface like this
end up being surprises and annoyances for users.

In any case there's a separate problem with defaults. We want to guarantee
that you can DROP a partition and then re-ADD it and the result should be a
noop at least from the user's perspective. We can't do that unless I
compromise on my idea that adding a child after the fact should be equivalent
to creating it with the parent in the definition.

When creating a table with the parent in the definition CREATE TABLE will copy
the parent's default if the default in the child is NULL:

postgres=# create table b (i integer default null) inherits (a);
NOTICE: merging column "i" with inherited definition
CREATE TABLE
postgres=# \d b
Table "public.b"
Column | Type | Modifiers
--------+---------+-----------
i | integer | default 2
Inherits: a

The problem is that it's possible to fiddle with the defaults after the table
is created, including dropping a default. If you drop the default and then
DROP-ADD the partition it would be a problem if the default magically
reappeared.

The only way to allow DROP then ADD to be a noop would be to accept whatever
the DEFAULT is on the child table without complaint. And I'm not just saying
that because it's the easiest for me to implement :)

This is already a factor for NOT NULL constraints too. When adding a parent
after the fact your NULLable column can magically become NOT NULL if the
parent is NOT NULL. But for adding a partition after the fact we can't just
change the column to NOT NULL because there may already be NULL rows in the
table.

We could do a pass-3 check for the NOT NULL constraint but if we're not doing
other schema changes then it makes more sense to just refuse to add such a
table.

--
greg

I can't find any standard api to remove a single specific dependency. It seems
normally dependencies are only removed when dropping objects via
performDeletion.

Should I just put a scan of pg_depend in ATExecDropInherits or should I add a
new function to pg_depend or somewhere else to handle deleting a specific
dependency?

The only way I can see to implement it is kind of gross. It would have to do
the same scan deleteDependencyRecordsFor does and add an explicit check on
each loop to see if it matches the referenced class/object.

--
greg

Ühel kenal päeval, N, 2006-06-08 kell 09:32, kirjutas Greg Stark:

"Zeugswetter Andreas DCP SD" <ZeugswetterA@spardat.at> writes:

Imho the op should only choose that path if he wants to fill the table
before adding the inheritance. It makes no sense to add columns with default
values to existing rows of the child table, especially when you inherit the
defaults from the parent.

We already have ALTER TABLE ADD COLUMN working for columns with defaults, so I
think that horse has left the barn.

Do you mean that in newer versions ALTER TABLE ADD COLUMN will change
existing data without asking me ?

That would be evil!

Even worse if ALTER TABLE ALTER COLUMN SET DEFAULT would do the same.

Soon we will be as bad as MS Word !

It was awfully annoying for users when that feature was missing.
Any non-linearities in the user interface like this
end up being surprises and annoyances for users.

I would be *really*, *really*, *really* annoyed if an op that I expected
to take less than 1 sec takes 5 hours and then forces me to spend
another 10 hours on VACUUM FULL+REINDEX or CLUSTER to get performance
back.

And making such radical changes even between major versions should be
stricty forbidden.

In any case there's a separate problem with defaults. We want to guarantee
that you can DROP a partition and then re-ADD it and the result should be a
noop at least from the user's perspective.

If DROP partition keeps defaults, and ADD does not change them then DROP
+ADD is a NOOP.

We can't do that unless I
compromise on my idea that adding a child after the fact should be equivalent
to creating it with the parent in the definition.

When creating a table with the parent in the definition CREATE TABLE will copy
the parent's default if the default in the child is NULL:

postgres=# create table b (i integer default null) inherits (a);
NOTICE: merging column "i" with inherited definition
CREATE TABLE
postgres=# \d b
Table "public.b"
Column | Type | Modifiers
--------+---------+-----------
i | integer | default 2
Inherits: a

The problem is that it's possible to fiddle with the defaults after the table
is created, including dropping a default. If you drop the default and then
DROP-ADD the partition it would be a problem if the default magically
reappeared.

sure. it should not magically appear.

The only way to allow DROP then ADD to be a noop would be to accept whatever
the DEFAULT is on the child table without complaint. And I'm not just saying
that because it's the easiest for me to implement :)

exactly. that would be the correct behaviour. even for NULL default.

This is already a factor for NOT NULL constraints too. When adding a parent
after the fact your NULLable column can magically become NOT NULL if the
parent is NOT NULL. But for adding a partition after the fact we can't just
change the column to NOT NULL because there may already be NULL rows in the
table.

constraints should match, that is a child table should already have all
the constraints of parent, but may have more.

We could do a pass-3 check for the NOT NULL constraint but if we're not doing
other schema changes then it makes more sense to just refuse to add such a
table.

nono. the ADD/DROP INHERITS should not do any data checking, just
comparison of metainfo. the partitions could be huge and having to check
data inside them would negate most of the usefullness for ADD/DROP
INHERITS.

--
----------------
Hannu Krosing
Database Architect
Skype Techshould benologies OÜ
Akadeemia tee 21 F, Tallinn, 12618, Estonia

Skype me: callto:hkrosing
Get Skype for free: http://www.skype.com

NOTICE: This communication contains privileged or other confidential
information. If you have received it in error, please advise the sender
by reply email and immediately delete the message and any attachments
without copying or disclosing the contents.