Обсуждение: Dropping columns with inheritance not working as documented
While investigating an issue someone reported in pg_partman ( https://github.com/keithf4/pg_partman/issues/61) came across something that seems very inconsistent with Inheritance. According to the documentation, dropping a column should be propagated down to all children. This only seems to happen for columns that are added AFTER a child table is inherited. There's no way to tell when a column was added to an inheritance set, so this could be very confusing for someone down the line that wasn't there when a table was set up. Here is a gist showing this using the current version of pg_partman (1.8.6) on PostgreSQL 9.4.2 https://gist.github.com/keithf4/a762144e46d0c4211d25 -- Keith Fiske Database Administrator OmniTI Computer Consulting, Inc. http://www.keithf4.com
Keith Fiske <keith@omniti.com> writes:
> According to the documentation, dropping a column should be propagated down
> to all children. This only seems to happen for columns that are added AFTER
> a child table is inherited. There's no way to tell when a column was added
> to an inheritance set, so this could be very confusing for someone down the
> line that wasn't there when a table was set up.
I think you might be misreading the docs. DROP COLUMN only propagates to
columns that have no independent reason to exist in the child. For
instance, given
create table p (f1 int);
these two commands have different results:
create table c () inherits (p);
create table c (f1 int) inherits (p);
In the second case, c.f1 has two reasons to exist: it was declared locally
to c, and it was inherited from p. Dropping p's f1 would remove only one
of those reasons to exist, so c.f1 would still be there.
If you do something like
create table c (f1 int);
alter table c inherit p;
that's treated as the second case. This is a debatable call but that's
the way we decided it should work back when these commands were
implemented.
FYI, the pg_attribute columns attislocal and attinhcount track the state
necessary to implement this behavior.
regards, tom lane
On Tue, Jun 2, 2015 at 6:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Keith Fiske <keith@omniti.com> writes: > > According to the documentation, dropping a column should be propagated > down > > to all children. This only seems to happen for columns that are added > AFTER > > a child table is inherited. There's no way to tell when a column was > added > > to an inheritance set, so this could be very confusing for someone down > the > > line that wasn't there when a table was set up. > > I think you might be misreading the docs. DROP COLUMN only propagates to > columns that have no independent reason to exist in the child. For > instance, given > > create table p (f1 int); > > these two commands have different results: > > create table c () inherits (p); > > create table c (f1 int) inherits (p); > > In the second case, c.f1 has two reasons to exist: it was declared locally > to c, and it was inherited from p. Dropping p's f1 would remove only one > of those reasons to exist, so c.f1 would still be there. > > If you do something like > > create table c (f1 int); > alter table c inherit p; > > that's treated as the second case. This is a debatable call but that's > the way we decided it should work back when these commands were > implemented. > > FYI, the pg_attribute columns attislocal and attinhcount track the state > necessary to implement this behavior. > > regards, tom lane > Honestly, this is incredibly unclear in the documentation. I have no idea how to even edit the documentation to explain that clearly to someone coming into this. Just doing the scenario I laid out in my example makes it clear that this is really not intuitive at all. You can see the same confusion from the person that reported it in the pg_partman ticket. Just imagine you're someone that inherited (great choice of words) some long running system and you decide to drop some columns from a parent table that are no longer needed. Some where there when inheritance was set up, some were added later. Without deep internals knowledge, you'd have no idea which ones are which. All you'd see is some column drops propagating down and others not. Any chance on some documentation clarity? Or honestly, some further justification for why it needs to stay like this? -- Keith Fiske Database Administrator OmniTI Computer Consulting, Inc. http://www.keithf4.com
Keith Fiske <keith@omniti.com> writes:
> On Tue, Jun 2, 2015 at 6:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think you might be misreading the docs. DROP COLUMN only propagates to
>> columns that have no independent reason to exist in the child.
> Honestly, this is incredibly unclear in the documentation. I have no idea
> how to even edit the documentation to explain that clearly to someone
> coming into this. Just doing the scenario I laid out in my example makes it
> clear that this is really not intuitive at all. You can see the same
> confusion from the person that reported it in the pg_partman ticket.
It is documented, for example in the ALTER TABLE reference page:
<para>
A recursive <literal>DROP COLUMN</literal> operation will remove a
descendant table's column only if the descendant does not inherit
that column from any other parents and never had an independent
definition of the column. A nonrecursive <literal>DROP
COLUMN</literal> (i.e., <command>ALTER TABLE ONLY ... DROP
COLUMN</command>) never removes any descendant columns, but
instead marks them as independently defined rather than inherited.
</para>
I'm not sure whether it's worth going into such details in section 5.9,
but certainly we could try to provide some explanation there, or wherever
it was that you were looking.
> Any chance on some documentation clarity? Or honestly, some further
> justification for why it needs to stay like this?
The killer argument, as I recall, was that we should take pains not
to irretrievably drop data if there was any doubt as to whether it
was still wanted. But even if you didn't believe that, there's now
a dozen years worth of backwards compatibility to worry about --- it's
been like this since 2002 or so.
regards, tom lane
On Tue, Jun 2, 2015 at 10:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Keith Fiske <keith@omniti.com> writes: > > On Tue, Jun 2, 2015 at 6:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> I think you might be misreading the docs. DROP COLUMN only propagates > to > >> columns that have no independent reason to exist in the child. > > > Honestly, this is incredibly unclear in the documentation. I have no idea > > how to even edit the documentation to explain that clearly to someone > > coming into this. Just doing the scenario I laid out in my example makes > it > > clear that this is really not intuitive at all. You can see the same > > confusion from the person that reported it in the pg_partman ticket. > > It is documented, for example in the ALTER TABLE reference page: > > <para> > A recursive <literal>DROP COLUMN</literal> operation will remove a > descendant table's column only if the descendant does not inherit > that column from any other parents and never had an independent > definition of the column. A nonrecursive <literal>DROP > COLUMN</literal> (i.e., <command>ALTER TABLE ONLY ... DROP > COLUMN</command>) never removes any descendant columns, but > instead marks them as independently defined rather than inherited. > </para> > > I'm not sure whether it's worth going into such details in section 5.9, > but certainly we could try to provide some explanation there, or wherever > it was that you were looking. > > > Any chance on some documentation clarity? Or honestly, some further > > justification for why it needs to stay like this? > > The killer argument, as I recall, was that we should take pains not > to irretrievably drop data if there was any doubt as to whether it > was still wanted. But even if you didn't believe that, there's now > a dozen years worth of backwards compatibility to worry about --- it's > been like this since 2002 or so. > > regards, tom lane > Agreed that if it's been there that long like that it would be hard to change now. But it seems a thought for data safety (which I'm not against) wasn't weighed carefully against how this behavior would appear to the average user. If it can't be changed, I would definitely make note of that in the Inheritance documentation somehow with a working example like you gave. I hadn't even considered looking down in the notes in the ALTER TABLE command for that. I just looked up in the DROP COLUMN section at the top. Keith
Tom Lane wrote:
> > Any chance on some documentation clarity? Or honestly, some further
> > justification for why it needs to stay like this?
>
> The killer argument, as I recall, was that we should take pains not
> to irretrievably drop data if there was any doubt as to whether it
> was still wanted. But even if you didn't believe that, there's now
> a dozen years worth of backwards compatibility to worry about --- it's
> been like this since 2002 or so.
Wow, it's been a long time ...
commit c328b6dd8bd85d91a0fd465c30b0bb352ea51e2b
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun Sep 22 19:42:52 2002 +0000
Replace pg_attribute.attisinherited with attislocal and attinhcount
columns, to allow more correct behavior in multiple-inheritance cases.
Patch by Alvaro Herrera, review by Tom Lane.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services