Thread: DROP COLUMN misbehaviour with multiple inheritance
I've come upon a misbehaviour of drop column, where drop column unconditionally drops inherited column from child tables. What it should do is to check if the same column is not inherited from other parents and drop it only when it is not Here is the test case: hannu=# create table p1(id int, name text); CREATE TABLE hannu=# create table p2(id2 int, name text); CREATE TABLE hannu=# create table c1(age int) inherits(p1,p2); NOTICE: CREATE TABLE: merging multiple inherited definitions of attribute "name" CREATE TABLE hannu=# \d c1 Table "public.c1"Column | Type | Modifiers --------+---------+-----------id | integer | name | text | id2 | integer | age | integer | hannu=# alter table p1 drop column name; ALTER TABLE hannu=# \d c1 Table "public.c1"Column | Type | Modifiers --------+---------+-----------id | integer | id2 | integer | age | integer | The column "c1.name" should survive the drop from p1, as it is also inherited from p2. -------------------- Hannu
Hannu Krosing <hannu@tm.ee> writes:
> I've come upon a misbehaviour of drop column, where drop column
> unconditionally drops inherited column from child tables.
> What it should do is to check if the same column is not inherited from
> other parents and drop it only when it is not
Hm. Seems like attisinherited should have been a count, not a boolean.
Is anyone sufficiently excited about this issue to force an initdb to
fix it?
regards, tom lane
On Thu, 2002-09-12 at 16:14, Tom Lane wrote: > Hannu Krosing <hannu@tm.ee> writes: > > I've come upon a misbehaviour of drop column, where drop column > > unconditionally drops inherited column from child tables. > > What it should do is to check if the same column is not inherited from > > other parents and drop it only when it is not > > Hm. Seems like attisinherited should have been a count, not a boolean. either that, or some check at drop column time. > Is anyone sufficiently excited about this issue to force an initdb to > fix it? The count approach seems definitely the right way, but a check (possibly a slow one) can be probably done without initdb. The other sad thing about the current behaviour is that in addition to being wrong it also breaks dump/reload - after dump/reload the initially dropped column is back in c1. ------------- Hannu
Tom Lane dijo: > Hannu Krosing <hannu@tm.ee> writes: > > I've come upon a misbehaviour of drop column, where drop column > > unconditionally drops inherited column from child tables. > > What it should do is to check if the same column is not inherited from > > other parents and drop it only when it is not > > Hm. Seems like attisinherited should have been a count, not a boolean. I'll try to make a fix and submit. > Is anyone sufficiently excited about this issue to force an initdb to > fix it? If people thinks it's important, the fix can be integrated. If not, it can wait until 7.4. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Aprende a avergonzarte mas ante ti que ante los demas" (Democrito)
Hannu Krosing <hannu@tm.ee> writes:
>> Hm. Seems like attisinherited should have been a count, not a boolean.
>> Is anyone sufficiently excited about this issue to force an initdb to
>> fix it?
> The count approach seems definitely the right way, but a check (possibly
> a slow one) can be probably done without initdb.
Slow, complicated to code, and deadlock-prone (since you'd have to
acquire locks on the other parent tables). My feeling is we fix this
with a counted attisinherited field, or don't fix at all.
We can certainly do the proper fix in 7.4; do we consider this bug
important enough to do an initdb for 7.3beta2? I don't have a strong
feeling either way about that.
regards, tom lane
> > The count approach seems definitely the right way, but a check (possibly > > a slow one) can be probably done without initdb. > > We can certainly do the proper fix in 7.4; do we consider this bug > important enough to do an initdb for 7.3beta2? I don't have a strong > feeling either way about that. I think we are too scared of doing initdb during beta... Initdb during beta should not be evaultated on a per bug basis, but keep a list of all things that could be fixed and judge if the total of all the fixes is worth one initdb. Right now off the top of my head I can think of the split function and this inherited change, are there more? my two cents...
On Thu, 12 Sep 2002, Matthew T. OConnor wrote: > > > The count approach seems definitely the right way, but a check (possibly > > > a slow one) can be probably done without initdb. > > > > We can certainly do the proper fix in 7.4; do we consider this bug > > important enough to do an initdb for 7.3beta2? I don't have a strong > > feeling either way about that. > > I think we are too scared of doing initdb during beta... > > Initdb during beta should not be evaultated on a per bug basis, but keep a > list of all things that could be fixed and judge if the total of all the > fixes is worth one initdb. Right now off the top of my head I can think of > the split function and this inherited change, are there more? > > my two cents... Agreed. Actually, an argument could likely be made that changes that require initdb should be done as early as possible since the later the change the more people there will be to test the change, and there will be fewer people who actually have to initdb since a lot of folks don't test beta releases until the 3rd or 4th beta.
On Thu, 12 Sep 2002, scott.marlowe wrote: > Agreed. > > Actually, an argument could likely be made that changes that require > initdb should be done as early as possible since the later the change the > more people there will be to test the change, and there will be fewer > people who actually have to initdb since a lot of folks don't test beta > releases until the 3rd or 4th beta. My mental dyslexia strikes again, that should be: ... since the EARLIER the change the more people there will be to test the change, ... sheesh. Sorry...
Tom Lane dijo: > Hannu Krosing <hannu@tm.ee> writes: > > > The count approach seems definitely the right way, but a check (possibly > > a slow one) can be probably done without initdb. > > Slow, complicated to code, and deadlock-prone (since you'd have to > acquire locks on the other parent tables). My feeling is we fix this > with a counted attisinherited field, or don't fix at all. All right, I now have all the catalog changes on place; this is the easy part (is an int2 count enough?). But when actually dropping a column, the recursion cannot be done the way it's done now, fetching the whole inheritor tree in one pass, because there's no way to distinguish the direct ones that have the attisinherited count greater than 1 from deeper ones; it has to be done step by step. If this is not clear, imagine the following situation: create table p1(id int, name text); create table p2(id2 int, name text); create table c1(age int) inherits(p1,p2); create table gc1() inherits (c1); p1 and p2 have name->attisinherited=0, while c1 has name->attisinherited=2. But gc1->name->attisinherited=1. If I just recurse the tree the way it's done now, I will happily drop "name" from gc1 while keeping it on c1. So I have to switch from find_all_inheritors() to find_inheritance_children() and keep recursing until there are no more inheritors (I still have to check if there are other gotchas with this approach, or optimizations to be done). I am already midway with this, but wanted to let you know in case the patch is rejected. Is this Ok? I see this is getting away from the "trivial fix" camp. -- Alvaro Herrera (<alvherre[a]atentus.com>) "The Gord often wonders why people threaten never to come back after they've been told never to return" (www.actsofgord.com)
En 12 Sep 2002 17:23:41 +0200 Hannu Krosing <hannu@tm.ee> escribió: > The other sad thing about the current behaviour is that in addition to > being wrong it also breaks dump/reload - after dump/reload the initially > dropped column is back in c1. I hadn't read this paragraph before. But I don't understand what you're saying. If I drop the column from p1 but not from p2, how is it expected that the column doesn't show in c1, that inherits both? Truth is that the column shouldn't have disappeared in the first place, so it isn't a mistake that shows up in the dump. Sure, databases before and after the dump are different, but the one before dump is broken. I don't have the original pgsql version (without the patch) compiled right now, but I think that if you were to select from p2, the backend would crash (or at least elog(ERROR)). Anyway, the patch I just submitted should fix this bug. Please test it and thanks for the report. -- Alvaro Herrera (<alvherre[a]atentus.com>) "La conclusion que podemos sacar de esos estudios es que no podemos sacar ninguna conclusion de ellos" (Tanenbaum)
Alvaro Herrera <alvherre@atentus.com> writes:
> If this is not clear, imagine the following situation:
> create table p1(id int, name text);
> create table p2(id2 int, name text);
> create table c1(age int) inherits(p1,p2);
> create table gc1() inherits (c1);
> p1 and p2 have name->attisinherited=0, while c1 has
> name->attisinherited=2. But gc1->name->attisinherited=1.
Ick. I hadn't thought that far ahead.
We could probably cause gc1->name->attisinherited to be 2 in this
scenario; does that help?
Actually, there might not be a problem. c1.name can't be deleted until
both p1.name and p2.name go away, and at that point we want both c1.name
and gc1.name to go away. So as long as we don't *recursively* decrement
the inherits count when c1.name.attisinherited hasn't reached 0, this
might be okay. But it needs thought.
> I see this is getting away from the "trivial fix" camp.
Yup. Let's step back and think carefully before we plunge into the
coding. What goes away when, and how do we define the inherits-count
to make it work right?
regards, tom lane
En Thu, 12 Sep 2002 23:40:21 -0400 Tom Lane <tgl@sss.pgh.pa.us> escribió: > Alvaro Herrera <alvherre@atentus.com> writes: > > If this is not clear, imagine the following situation: > > > create table p1(id int, name text); > > create table p2(id2 int, name text); > > create table c1(age int) inherits(p1,p2); > > create table gc1() inherits (c1); > > > p1 and p2 have name->attisinherited=0, while c1 has > > name->attisinherited=2. But gc1->name->attisinherited=1. > > We could probably cause gc1->name->attisinherited to be 2 in this > scenario; does that help? I'm trying to imagine a case where this is harmful, but cannot find any. It would have to be proven that there is none; IMHO this is a little deviating from the "reality". > Actually, there might not be a problem. c1.name can't be deleted until > both p1.name and p2.name go away, and at that point we want both c1.name > and gc1.name to go away. So as long as we don't *recursively* decrement > the inherits count when c1.name.attisinherited hasn't reached 0, this > might be okay. But it needs thought. This is what I implemented on the patch I posted, I think. The idea is that attisinherited is decremented non-recursively, i.e. only in direct inheritors; and when it reaches zero the column is dropped, and its inheritors have it decremented also. In the cases I've tried this works, and it seems to me that it is correct; however, I haven't proven it is. Multiple inheritance and multiple generations is weird. It just ocurred to me that maybe I overlooked the ALTER TABLE ONLY ... DROP COLUMN case, but I'm now going to bed. I'll think about this case tomorrow. > > I see this is getting away from the "trivial fix" camp. > > Yup. Let's step back and think carefully before we plunge into the > coding. What goes away when, and how do we define the inherits-count > to make it work right? Huh, I already did. Please think about my solution. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Para tener mas hay que desear menos"
Alvaro Herrera <alvherre@atentus.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> escribi�:
>> Actually, there might not be a problem. c1.name can't be deleted until
>> both p1.name and p2.name go away, and at that point we want both c1.name
>> and gc1.name to go away. So as long as we don't *recursively* decrement
>> the inherits count when c1.name.attisinherited hasn't reached 0, this
>> might be okay. But it needs thought.
> This is what I implemented on the patch I posted, I think. The idea is
> that attisinherited is decremented non-recursively, i.e. only in direct
> inheritors; and when it reaches zero the column is dropped, and its
> inheritors have it decremented also.
Yeah; after marginally more thought, I'm thinking that the correct
definition of attisinherited (need new name BTW) is "number of *direct*
ancestors this table inherits this column from". I think you are
describing the same idea.
Given the obvious algorithms for updating and using such a value,
does anyone see a flaw in the behavior?
One corner case is that I think we currently allow
create table p (f1 int);create table c (f1 int) inherits(p);
which is useless in the given example but is not useless if c
provides a default or constraints for column f1. ISTM f1 should
not go away in c if we drop it in p, in this case. Maybe we want
not an "inherits count" but a "total sources of definitions count",
which would include 1 for each ancestral table plus 1 if declared
locally. When it drops to 0, okay to delete the column.
> however, I haven't proven it is. Multiple inheritance and
> multiple generations is weird.
What he said... I'm way too tired to think this through tonight...
regards, tom lane
Tom Lane dijo: > One corner case is that I think we currently allow > > create table p (f1 int); > create table c (f1 int) inherits(p); In this case, c.f1.attisinherited count is 2; thus when I drop f1 from p, it is not dropped from c. Do you have some suggestion on what the name should be? Clearly attisinherited is not appropiate. attinhcount maybe? The patch submitted does what you describe. I'm leaving tomorrow and won't be back until next weekend, so please do the name change yourself if the patch is to be applied. -- Alvaro Herrera (<alvherre[a]atentus.com>) Voy a acabar con todos los humanos / con los humanos yo acabaré voy a acabar con todos / con todos los humanos acabaré (Bender)
[ back to thinking about this patch ]
Alvaro Herrera <alvherre@atentus.com> writes:
> Tom Lane dijo:
>> One corner case is that I think we currently allow
>>
>> create table p (f1 int);
>> create table c (f1 int) inherits(p);
> In this case, c.f1.attisinherited count is 2; thus when I drop f1 from
> p, it is not dropped from c.
That seems right, but the problem I have with it is that the resulting
state of c.f1 is attisinherited = 1. This means that you cannot drop
c.f1. It seems to me that we should have this behavior:
create table p (f1 int);
create table c (f1 int not null) inherits(p);
drop column c.f1;
-- should be rejected since c.f1 is inherited
drop column p.f1;
-- c.f1 is still there, but no longer inherited
drop column c.f1;
-- should succeed; but will fail with patch as given
as compared to
create table p (f1 int);
create table c () inherits(p);
drop column c.f1;
-- should be rejected since c.f1 is inherited
drop column p.f1;
-- c.f1 is dropped now, since there is no local definition for it
And if you aren't confused yet, what about non-recursive drops of p.f1
(ie, alter table ONLY p drop column f1)? This case seems clear:
create table p (f1 int);
create table c () inherits(p);
drop column c.f1;
-- should be rejected since c.f1 is inherited
drop ONLY column p.f1;
-- c.f1 is NOT dropped, but must now be considered non-inherited
drop column c.f1;
-- should succeed
And then I think we should say
create table p (f1 int);
create table c (f1 int not null) inherits(p);
drop column c.f1;
-- should be rejected since c.f1 is inherited
drop ONLY column p.f1;
-- c.f1 is still there, but no longer inherited
drop column c.f1;
-- should succeed
I am not sure how to make all four of these cases work. We might need
two fields :-( ... a "locally defined" boolean and a "number of times
inherited" counter. This seems like overkill though.
If we don't have the "locally defined" boolean then I think we have to
make the first case work like so:
create table p (f1 int);
create table c (f1 int not null) inherits(p);
drop column p.f1;
-- c.f1 GOES AWAY, because its inherit count went to zero
Is this reasonable behavior? I'm not sure. You could probably argue
it either way.
Another interesting case is multiple inheritance.
create table p1 (f1 int);
create table p2 (f1 int);
create table c () inherits(p1, p2);
drop ONLY column p1.f1;
drop column p2.f1;
After this sequence, what is the state of c.f1? Is it still there?
Should it be? If it is still there, will it be possible to get rid of
it with "drop column c.f1"? What if we did DROP ONLY on *both*
ancestors?
regards, tom lane
> That seems right, but the problem I have with it is that the resulting > state of c.f1 is attisinherited = 1. This means that you cannot drop > c.f1. It seems to me that we should have this behavior: Has anyone given much thought as to perhaps we could just drop multiple inheritance from Postgres? There are people using single inheritance - but how many actually use multiple inheritance? If we dumped it we could use the proposed all-child-tables-in-one-relation idea, and everything would become very easy... Chris
Christopher Kings-Lynne wrote: > > That seems right, but the problem I have with it is that the resulting > > state of c.f1 is attisinherited = 1. This means that you cannot drop > > c.f1. It seems to me that we should have this behavior: > > Has anyone given much thought as to perhaps we could just drop multiple > inheritance from Postgres? There are people using single inheritance - but > how many actually use multiple inheritance? If we dumped it we could use > the proposed all-child-tables-in-one-relation idea, and everything would > become very easy... I am for it. Multiple inheritance is more of a mess than a help. Just look at C++. Everyone is moving away from multiple inheritance for that reason. -- 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:
> Christopher Kings-Lynne wrote:
>> Has anyone given much thought as to perhaps we could just drop multiple
>> inheritance from Postgres?
> I am for it. Multiple inheritance is more of a mess than a help.
I'm not agin it ... but if that's the lay of the land then we have
no need to apply a last-minute catalog reformatting to fix a
multiple-inheritance bug. This patch is off the "must fix for 7.3"
list, no?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Christopher Kings-Lynne wrote: > >> Has anyone given much thought as to perhaps we could just drop multiple > >> inheritance from Postgres? > > > I am for it. Multiple inheritance is more of a mess than a help. > > I'm not agin it ... but if that's the lay of the land then we have > no need to apply a last-minute catalog reformatting to fix a > multiple-inheritance bug. This patch is off the "must fix for 7.3" > list, no? I don't think a few days before beta2 is the time to be making such decisions. I think we have to keep the course and open the discussion in 7.4. Sorry. -- 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
> > > I am for it. Multiple inheritance is more of a mess than a help. > > > > I'm not agin it ... but if that's the lay of the land then we have > > no need to apply a last-minute catalog reformatting to fix a > > multiple-inheritance bug. This patch is off the "must fix for 7.3" > > list, no? Multiple inheritance patches should go in for 7.3, since we support multiple inheritance in 7.3. However, I think thought should be put into removing multiple inheritance in 7.4 - after a user survey perhaps. If removing multiple inheritance means we can have perfece, indexable single inheritance then I think it's worth it. Unless the spec calls for multiple inheritance of course. Chris
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> I'm not agin it ... but if that's the lay of the land then we have
>> no need to apply a last-minute catalog reformatting to fix a
>> multiple-inheritance bug. This patch is off the "must fix for 7.3"
>> list, no?
> I don't think a few days before beta2 is the time to be making such
> decisions.
The decision at hand is whether to apply a patch. You cannot say "we're
not deciding now", because that is a decision...
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> I'm not agin it ... but if that's the lay of the land then we have > >> no need to apply a last-minute catalog reformatting to fix a > >> multiple-inheritance bug. This patch is off the "must fix for 7.3" > >> list, no? > > > I don't think a few days before beta2 is the time to be making such > > decisions. > > The decision at hand is whether to apply a patch. You cannot say "we're > not deciding now", because that is a decision... Yes. I am saying we should not assume we are going to remove multiple inheritance. We should apply the patch and make things a good as they can be for 7.3. -- 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
> > The decision at hand is whether to apply a patch. You cannot say "we're > > not deciding now", because that is a decision... > > Yes. I am saying we should not assume we are going to remove multiple > inheritance. We should apply the patch and make things a good as they > can be for 7.3. I think the patch should be applied. That way people who are using multiple inheritance (if there are any) can know that they have a vaguely bug free implementation in 7.3 until they redo their stuff for 7.4. Chris
Tom Lane <tgl@sss.pgh.pa.us> writes: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Christopher Kings-Lynne wrote: > >> Has anyone given much thought as to perhaps we could just drop > >> multiple inheritance from Postgres? > > > I am for it. Multiple inheritance is more of a mess than a help. > > I'm not agin it I'm abstaining. > but if that's the lay of the land then we have no need to apply a > last-minute catalog reformatting to fix a multiple-inheritance bug. The catalog format has changed since beta1 anyway due to the casting changes, right? (not to mention the split -> split_part change). If that's the case, I don't see a good reason not to include the fix, provided it's reasonably low-risk. Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Tom Lane kirjutas R, 20.09.2002 kell 04:49: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Christopher Kings-Lynne wrote: > >> Has anyone given much thought as to perhaps we could just drop multiple > >> inheritance from Postgres? > > > I am for it. Multiple inheritance is more of a mess than a help. > > I'm not agin it ... but if that's the lay of the land then we have > no need to apply a last-minute catalog reformatting to fix a > multiple-inheritance bug. What I'm actually envisioning for PostgreSQL inheritance is model similar to (my understanding of) SQL99 : 1. Single "data" inheritance for SELECT/INSERT/UPDATE/DELETE , meaning that the set of inherited tables any such command operates on comes from a single-inheritance hierarchy the SQL99 syntax for defining tables is CREATE TABLE child ( ... ) UNDER parent; 2. This single inheritance also applies both to "tuple-level" constraints (not null, check) and to "relation-level" constraints - unique, primary and foreign keys. 3. Multiple "interface-only" inheritance the SQL99 syntax for defining tables is CREATE TABLE child ( ..., LIKE othertable, LIKE yetanothertable, ); which would behave like our current multiple inheritance, only without affecting SELECT/INSERT/UPDATE/DELETE and "relation-level" constraints. 4. "tuple-level" constraints should still be inherited 5. function selection for functions defined on row typoes should be able to select both from among functions defined over "data" inheritance parents and "interface-only" inheritance parents. 5 such selection should be dynamic (scan-time) for queries running over inheritance trees. (SELECT without ONLY , formely SELECT*) > This patch is off the "must fix for 7.3" list, no? I still think that this should be fixed in 7.3, but the inhcount attribute should show all tables where the column is defined, not just inherited. The default, no-inheritance case should set the column to 1. ------------- Hannu
Hannu Krosing <hannu@tm.ee> writes:
> I still think that this should be fixed in 7.3, but the inhcount
> attribute should show all tables where the column is defined, not just
> inherited. The default, no-inheritance case should set the column to 1.
Well, no, because then a locally defined column is indistinguishable
from a singly-inherited column, breaking the cases that the original
attisinherited patch was supposed to fix.
It doesn't fix the ONLY problem, either. Consider
create table p1 (f1 int);
create table p2 (f1 int);
create table c () inherits(p1, p2);
--c.f1 now has definition-count 2
drop ONLY column p1.f1;
--c.f1 now has count 1?
drop column p2.f1;
--c.f1 removed because count went to 0?
It might look like we could fix this by defining DROP ONLY as not
touching the child-table definition-counts at all; then a DROP ONLY
effectively makes a child column look like it's locally defined
instead of inherited. But that trick only works once. Consider:
create table p1 (f1 int);
create table p2 (f1 int);
create table c () inherits(p1, p2);
--c.f1 now has definition-count 2
drop ONLY column p1.f1;
--c.f1 still has count 2?
drop ONLY column p2.f1;
--c.f1 still has count 2?
drop column c.f1
--fails because count>1, so there is now no way to delete c.f1
I think we could make all these cases work if we replaced attisinherited
with *two* columns, a boolean attislocal(ly defined) and a count of
(direct) inheritances. DROP ONLY would have the effect of decrementing
the count and setting attislocal to true in each direct child; recursive
DROP would decrement the count and then drop if count is 0 *and*
attislocal is not set. At the start of a recursion, we'd allow DROP
only if count is 0 (and, presumably, attislocal is true, else the column
would not be there...).
Question is, is fixing these cases worth this much trouble? I think the
two-column solution is actually free in terms of storage space in
pg_attribute, because of alignment considerations. But it's still a
large reworking of the existing patch, and we have other fish to fry by
Sunday.
In any case I am inclined to reject the patch as-it-stands, because it
fixes one problem at the cost of introducing new ones.
regards, tom lane
Tom Lane dijo: > I think we could make all these cases work if we replaced attisinherited > with *two* columns, a boolean attislocal(ly defined) and a count of > (direct) inheritances. DROP ONLY would have the effect of decrementing > the count and setting attislocal to true in each direct child; recursive > DROP would decrement the count and then drop if count is 0 *and* > attislocal is not set. At the start of a recursion, we'd allow DROP > only if count is 0 (and, presumably, attislocal is true, else the column > would not be there...). The cases you presented are really tricky. I'll work today on the attislocal and attinhcount patch; I hope to have it ready later today for review and inclusion before beta2. -- Alvaro Herrera (<alvherre[a]atentus.com>) Officer Krupke, what are we to do? Gee, officer Krupke, Krup you! (West Side Story, "Gee, Officer Krupke")
Alvaro Herrera <alvherre@atentus.com> writes:
>> Another interesting case is multiple inheritance.
>>
>> create table p1 (f1 int);
>> create table p2 (f1 int);
>> create table c () inherits(p1, p2);
>>
>> drop ONLY column p1.f1;
>> drop column p2.f1;
>>
>> After this sequence, what is the state of c.f1? Is it still there?
>> Should it be?
> Well, in this case the column is dropped. If the last drop is ONLY, the
> column will stay (regardless of what the first drop did).
It seems to me that DROP ONLY should set attislocal true on each child
for which it decrements the inherit count, whether the count reaches
zero or not. This would cause the behavior in the above case to be that
c.f1 stays around after the second drop (but can be dropped with a third
drop of c.f1 itself). I think this is correct, since the implication of
DROP ONLY is that child columns are being cut loose from their parent's
apron strings and now have independent existence.
This is a minor tweak to your patch, and I'll make it work that way
unless I hear squawks...
regards, tom lane
Tom Lane dijo: > It seems to me that DROP ONLY should set attislocal true on each child > for which it decrements the inherit count, whether the count reaches > zero or not. This would cause the behavior in the above case to be that > c.f1 stays around after the second drop (but can be dropped with a third > drop of c.f1 itself). I think this is correct, since the implication of > DROP ONLY is that child columns are being cut loose from their parent's > apron strings and now have independent existence. Yes, I think it's more consistent the way you are proposing. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Acepta los honores y aplausos y perderas tu libertad"
Tom Lane kirjutas P, 22.09.2002 kell 18:56: > Alvaro Herrera <alvherre@atentus.com> writes: > >> Another interesting case is multiple inheritance. > >> > >> create table p1 (f1 int); > >> create table p2 (f1 int); > >> create table c () inherits(p1, p2); > >> > >> drop ONLY column p1.f1; > >> drop column p2.f1; > >> > >> After this sequence, what is the state of c.f1? Is it still there? > >> Should it be? > > > Well, in this case the column is dropped. If the last drop is ONLY, the > > column will stay (regardless of what the first drop did). > > It seems to me that DROP ONLY should set attislocal true on each child > for which it decrements the inherit count, whether the count reaches > zero or not. This would not be what I e'd expect - if c inherited f1 twice and then one of the parents disinherits it, then it would still be inherited from the other parent > This would cause the behavior in the above case to be that > c.f1 stays around after the second drop (but can be dropped with a third > drop of c.f1 itself). I'd vote for the way Alvaro describes it - keep the attislocal=false while there exist parents from which the column was inherited. > I think this is correct, since the implication of > DROP ONLY is that child columns are being cut loose from their parent's > apron strings and now have independent existence. For me the implication is that ONLY this parent cuts loose the strings from its side, but should not mess with anything the child inherits from other parties. > This is a minor tweak to your patch, and I'll make it work that way > unless I hear squawks... I was disconnected for the weekend, I hope this is not too late to squawk ;) ----------------- Hannu
Tom Lane kirjutas P, 22.09.2002 kell 18:56: > Alvaro Herrera <alvherre@atentus.com> writes: > >> Another interesting case is multiple inheritance. > >> > >> create table p1 (f1 int); > >> create table p2 (f1 int); > >> create table c () inherits(p1, p2); > >> > >> drop ONLY column p1.f1; > >> drop column p2.f1; > >> > >> After this sequence, what is the state of c.f1? Is it still there? > >> Should it be? > > > Well, in this case the column is dropped. If the last drop is ONLY, the > > column will stay (regardless of what the first drop did). > > It seems to me that DROP ONLY should set attislocal true on each child > for which it decrements the inherit count, whether the count reaches > zero or not. Would it then not produce a situation, which can't be reproduced using just CREATEs ? i.e. same column in bot parent (p2.f1) and child (c.f1) but _not_ inherited ?? Then there would be no way to move a field from one parent table to another and still have it as an inherited column in child. It also seems bogus considering when doing SELECT * FROM p2 -- How should the select behave regarding c.f1 - there is a field with the same name and type but not inherited . > This would cause the behavior in the above case to be that > c.f1 stays around after the second drop (but can be dropped with a third > drop of c.f1 itself). What if you have a deeper hierarchy under c - will this make you traverse them all to drop f1 ? > I think this is correct, since the implication of > DROP ONLY is that child columns are being cut loose from their parent's > apron strings and now have independent existence. From (this) parent's but not from (other) parents' ;) Like In real world one should only be allowed to disinherit what _he_ owns. -------------- Hannu
Hannu Krosing dijo: > Tom Lane kirjutas P, 22.09.2002 kell 18:56: > > It seems to me that DROP ONLY should set attislocal true on each child > > for which it decrements the inherit count, whether the count reaches > > zero or not. > > Would it then not produce a situation, which can't be reproduced using > just CREATEs ? i.e. same column in bot parent (p2.f1) and child (c.f1) > but _not_ inherited ?? No, you cannot do that. For example, create table p1 (f1 int, f2 int); create table p2 (f1 int, f3 int); create table c () inherits (p1, p2); alter table only p1 drop column f1; alter table only p2 drop column f1; In this case, f1 is kept on c, and this situation can be recreated as: create table p1 (f2 int); create table p2 (f3 int); create table c (f1 int) inherits (p2, p3); If you drop it on only one parent it is exactly the same. The next question is whether pg_dump knows how to do such things. The answer is that it doesn't know that it must locally define f1 on c if you drop the column on only one parent. Oddly enough, the following create table p (f1 int); create table c (f1 int not null); produces the right behavior in pg_dump, but create table p (f1 int); create table c () inherits (p); alter table c alter f1 set not null; produces exactly the same as the former. I don't know if it's right. > Then there would be no way to move a field from one parent table to > another and still have it as an inherited column in child. You cannot add a column to a table that is inherited by another table that has a column with the same name: inhtest=# alter table p1 add column f1 int; ERROR: ALTER TABLE: column name "f1" already exists in table "c" inhtest=# alter table only p1 add column f1 int; ERROR: Attribute must be added to child tables too inhtest=# IOW: there's no way to "move" a column, unless you drop it in the whole inheritance tree first. Maybe this is a bug, and adding a column that exists in all childs (with the same name and type) should be allowed. > It also seems bogus considering when doing SELECT * FROM p2 -- How > should the select behave regarding c.f1 - there is a field with the same > name and type but not inherited . I don't understand. Suppose table c has column f1. If I select from p2 and it has f1 also, f1 will show up. If p2 doesn't have f1, it won't: the inheritance status of the attribute doesn't matter. > > This would cause the behavior in the above case to be that > > c.f1 stays around after the second drop (but can be dropped with a third > > drop of c.f1 itself). > > What if you have a deeper hierarchy under c - will this make you > traverse them all to drop f1 ? The recursion is always done in steps one level deep. If the column is inherited from somewhere else in the grandchild, it will stay. If not, it will disappear. If you want to drop in more than one level, but not all of them, you will have to drop it locally on each. This seems just natural, doesn't it? -- Alvaro Herrera (<alvherre[a]atentus.com>) "Granting software the freedom to evolve guarantees only different results, not better ones." (Zygo Blaxell)
En 23 Sep 2002 10:23:06 +0200 Hannu Krosing <hannu@tm.ee> escribió: > Tom Lane kirjutas P, 22.09.2002 kell 18:56: > > It seems to me that DROP ONLY should set attislocal true on each child > > for which it decrements the inherit count, whether the count reaches > > zero or not. > > This would not be what I e'd expect - if c inherited f1 twice and then > one of the parents disinherits it, then it would still be inherited from > the other parent The problem with this is that two sequences of commands only differing in the ordering of two clauses give different result: create table p1 (f1 int, f2 int); create table p2 (f1 int, f2 int); create table c () inherits (p1, p2); alter table only p1 drop column f1; alter table p2 drop column f1; create table p1 (f1 int, f2 int); create table p2 (f1 int, f2 int); create table c () inherits (p1, p2); alter table p2 drop column f1; alter table only p1 drop column f1; The former drops f1 from c, while the latter does not. It's inconsistent. -- Alvaro Herrera (<alvherre[a]atentus.com>) "La Primavera ha venido. Nadie sabe como ha sido" (A. Machado)
Alvaro Herrera kirjutas E, 23.09.2002 kell 10:06: > Hannu Krosing dijo: > > > Tom Lane kirjutas P, 22.09.2002 kell 18:56: > > > > It seems to me that DROP ONLY should set attislocal true on each child > > > for which it decrements the inherit count, whether the count reaches > > > zero or not. > > > > Would it then not produce a situation, which can't be reproduced using > > just CREATEs ? i.e. same column in bot parent (p2.f1) and child (c.f1) > > but _not_ inherited ?? > > No, you cannot do that. For example, > create table p1 (f1 int, f2 int); > create table p2 (f1 int, f3 int); > create table c () inherits (p1, p2); > > alter table only p1 drop column f1; > alter table only p2 drop column f1; > > In this case, f1 is kept on c, and this situation can be recreated as: > create table p1 (f2 int); > create table p2 (f3 int); > create table c (f1 int) inherits (p2, p3); > > If you drop it on only one parent it is exactly the same. I meant create table p1 (f1 int, f2 int); create table p2 (f1 int, f3 int); create table c () inherits (p1, p2); alter table only p1 drop column f1; If you now set c.f1.attislocal = 1 as suggested by Tom , it seems like you have a local p1.f1 _and_ local c.f1 , for which there is no way to create without DROP's. If I understand the meaning of attislocal correctly, the after the above, I could do ALTER TABLE c DROP COLUMN f1, which would break SELECT * FROM p2. > The next question is whether pg_dump knows how to do such things. The > answer is that it doesn't know that it must locally define f1 on c if > you drop the column on only one parent. Oddly enough, the following > > create table p (f1 int); > create table c (f1 int not null); Did you mean create table c (f1 int not null) inherits (p); ? > produces the right behavior in pg_dump, but > > create table p (f1 int); > create table c () inherits (p); > alter table c alter f1 set not null; > > produces exactly the same as the former. I don't know if it's right. > > > Then there would be no way to move a field from one parent table to > > another and still have it as an inherited column in child. > > You cannot add a column to a table that is inherited by another table > that has a column with the same name: > > inhtest=# alter table p1 add column f1 int; > ERROR: ALTER TABLE: column name "f1" already exists in table "c" > inhtest=# alter table only p1 add column f1 int; > ERROR: Attribute must be added to child tables too > inhtest=# > > IOW: there's no way to "move" a column, unless you drop it in the whole > inheritance tree first. Maybe this is a bug, and adding a column that > exists in all childs (with the same name and type) should be allowed. It should be symmetric to DROP behaviour. So we should first check, if there are no childs with columns with the same name but different type, then add it to all children where it is missing and just make it inherited, where it is already present. ----------- Hannu
Alvaro Herrera kirjutas E, 23.09.2002 kell 10:30: > En 23 Sep 2002 10:23:06 +0200 > Hannu Krosing <hannu@tm.ee> escribió: > > > Tom Lane kirjutas P, 22.09.2002 kell 18:56: > > > > It seems to me that DROP ONLY should set attislocal true on each child > > > for which it decrements the inherit count, whether the count reaches > > > zero or not. > > > > This would not be what I e'd expect - if c inherited f1 twice and then > > one of the parents disinherits it, then it would still be inherited from > > the other parent > > The problem with this is that two sequences of commands only differing > in the ordering of two clauses give different result: IMHO this is the correct behaviour > create table p1 (f1 int, f2 int); > create table p2 (f1 int, f2 int); > create table c () inherits (p1, p2); > alter table only p1 drop column f1; Here you get rid of f1 in p1 _only_, i.e you keep it in children. > alter table p2 drop column f1; At this point c.f1 is inherited from only p2 and should be dropped > create table p1 (f1 int, f2 int); > create table p2 (f1 int, f2 int); > create table c () inherits (p1, p2); > alter table p2 drop column f1; Here c.f1 is still inherited from p1 and thus will not be dropped > alter table only p1 drop column f1; If you say ONLY you _do_ mean "don't drop from child tables". > The former drops f1 from c, while the latter does not. It's > inconsistent. But this is what _should_ happen. It is quite unreasonable to expect that order of commands makes no difference. ------------ Hannu
Hannu Krosing <hannu@tm.ee> writes:
> Alvaro Herrera kirjutas E, 23.09.2002 kell 10:30:
>> The former drops f1 from c, while the latter does not. It's
>> inconsistent.
> But this is what _should_ happen.
On what grounds do you claim that? I agree with Alvaro: it's
inconsistent to have ONLY produce different effects depending on
the order in which you issue the commands.
> It is quite unreasonable to expect that order of commands makes no
> difference.
Why?
I'll agree that it's not an overriding argument, but it is something
to shoot for if we can. And I'm not seeing the argument on the other
side.
regards, tom lane
Hannu Krosing <hannu@tm.ee> writes:
> I meant
> create table p1 (f1 int, f2 int);
> create table p2 (f1 int, f3 int);
> create table c () inherits (p1, p2);
> alter table only p1 drop column f1;
> If you now set c.f1.attislocal = 1 as suggested by Tom , it seems like
> you have a local p1.f1 _and_ local c.f1 , for which there is no way to
> create without DROP's.
Uh, no, you don't have a p1.f1 at all.
> If I understand the meaning of attislocal correctly, the after the
> above, I could do ALTER TABLE c DROP COLUMN f1, which would break
> SELECT * FROM p2.
No you could not, because c.f1 still has attinhcount = 1 due to the
inheritance from p2. As long as c.f1.attinhcount > 0, you won't be
allowed to drop c.f1. attislocal does not override that.
>> The next question is whether pg_dump knows how to do such things. The
>> answer is that it doesn't know that it must locally define f1 on c if
>> you drop the column on only one parent.
That's a good point. It could be fixed easily though (pg_dump would
just have to take attislocal into consideration when deciding whether
to emit a column definition in the child table).
>> ... produces the right behavior in pg_dump, but
>>
>> create table p (f1 int);
>> create table c () inherits (p);
>> alter table c alter f1 set not null;
>>
>> produces exactly the same as the former. I don't know if it's right.
I think this is fine. Having done something to the field in c (and not
recursively from p) means that you are attaching special new meaning
to c.f1; I'm okay with equating this action to "c is now locally defined".
Maybe the backend should make that equation too, and actively set
attislocal in the top level when doing an ALTER COLUMN.
BTW, do we prohibit ALTER DROP NOT NULL on inherited columns? We
probably should.
>> You cannot add a column to a table that is inherited by another table
>> that has a column with the same name:
>>
>> inhtest=# alter table p1 add column f1 int;
>> ERROR: ALTER TABLE: column name "f1" already exists in table "c"
>> inhtest=# alter table only p1 add column f1 int;
>> ERROR: Attribute must be added to child tables too
>> inhtest=#
>>
>> IOW: there's no way to "move" a column, unless you drop it in the whole
>> inheritance tree first. Maybe this is a bug, and adding a column that
>> exists in all childs (with the same name and type) should be allowed.
Yeah, this is an implementation shortcoming in ALTER ADD COLUMN: if it
finds an existing column of the same name in a child table, it should
test whether it's okay to "merge" the columns (same types, no conflict
in constraints/defaults, cf CREATE's behavior); if so, it should
increment the child column's attinhcount instead of failing.
I had noticed that yesterday, and meant to ask Bruce to put it on TODO,
but got distracted with other stuff.
regards, tom lane
Hannu Krosing <hannu@tm.ee> writes:
>> It seems to me that DROP ONLY should set attislocal true on each child
>> for which it decrements the inherit count, whether the count reaches
>> zero or not.
> Would it then not produce a situation, which can't be reproduced using
> just CREATEs ? i.e. same column in bot parent (p2.f1) and child (c.f1)
> but _not_ inherited ??
No, because the child will still have attinhcount > 0 until you drop the
last matching parent column. attislocal is independent of the value of
attinhcount (that's why we need two fields).
regards, tom lane
On Mon, 2002-09-23 at 18:41, Tom Lane wrote: > Hannu Krosing <hannu@tm.ee> writes: > > Alvaro Herrera kirjutas E, 23.09.2002 kell 10:30: > >> The former drops f1 from c, while the latter does not. It's > >> inconsistent. > > > But this is what _should_ happen. > > On what grounds do you claim that? I agree with Alvaro: it's > inconsistent to have ONLY produce different effects depending on > the order in which you issue the commands. Sorry it took some time thin down my thoughts ;) As the three following sets of commands ( should ) yield exactly the same database schema (as visible to user): 1) -------------------------------- create table p1 (f1 int, g1 int); create table p2 (f1 int, h1 int); create table c () inherits(p1, p2); drop column p2.f1; -- this DROP is in fact implicitly ONLY 2) -------------------------------- create table p1 (f1 int, g1 int); create table p2 (f1 int, h1 int); create table c () inherits(p1, p2); drop only column p2.f1; 3) -------------------------------- create table p1 (f1 int, g1 int); create table p2 (h1 int); create table c () inherits(p1, p2); ----------------------------------- For this schema, no matter how we arrived at it DROP COLUMN p1.f1; should be different from DROP ONLY COLUMN p1.f1; But the ONLY modifier was implicit for all the _non-final_ DROPs We could carve it out for users by _requiring_ ONLY if the column dropped is multiply inherited, but that would cut off the possibility that it is multiply inherited in some children and not in some other, i.e you could not have drop column automatically remove c13.f1 but keep c12.f1 for the following schema. create table p1 (f1 int, g1 int); create table p2 (f1 int, h1 int); create table c12 () inherits(p1, p2); create table p3 (i1 int); create table c13 () inherits(p1, p3); So I'd suggest we just postulate that for multiple inheritance dropping any columns still inherited from other peers will be implicitly "DROP ONLY" _as far as it concerns this child_ . then it would be clear why we have different behaviour for drop ONLY column p1.f1; drop column p2.f1; and drop ONLY column p2.f1; <-- this ONLY is implicit for c by virtue of p1.f1 being still around drop ONLY column p1.f1; > > It is quite unreasonable to expect that order of commands makes no > > difference. > > Why? > > I'll agree that it's not an overriding argument, but it is something > to shoot for if we can. And I'm not seeing the argument on the other > side. Just to reiterate: 1. All ALTER TABLE MyTable DROP COLUMN commands assume implicit ONLY when dropping columns multiply inherited from MyTable. 2. Making the final DROP implicitly NOT-ONLY in case there have been other DROPs of same column from other parents would make it non-deterministic if columns from child tables will be dropped when using DROP ONLY on a schema you dont know the full history for. 2.a It will probably also not be pg_dump-transparent, ie doing dump/reload between first and second drop column will get you different results. ----------------- Hannu
Hannu Krosing <hannu@tm.ee> writes:
> 1) --------------------------------
> create table p1 (f1 int, g1 int);
> create table p2 (f1 int, h1 int);
> create table c () inherits(p1, p2);
> drop column p2.f1; -- this DROP is in fact implicitly ONLY
Surely not? At least, I don't see why it should be thought of that way.
There's always a difference between DROP and DROP ONLY.
regards, tom lane
On Wed, 2002-09-25 at 04:13, Tom Lane wrote: > Hannu Krosing <hannu@tm.ee> writes: > > 1) -------------------------------- > > create table p1 (f1 int, g1 int); > > create table p2 (f1 int, h1 int); > > create table c () inherits(p1, p2); > > drop column p2.f1; -- this DROP is in fact implicitly ONLY > > Surely not? At least, I don't see why it should be thought of that way. > There's always a difference between DROP and DROP ONLY. What will be the difference in the user-visible schema ? ------------ Hannu
Hannu Krosing dijo: > On Wed, 2002-09-25 at 04:13, Tom Lane wrote: > > Hannu Krosing <hannu@tm.ee> writes: > > > 1) -------------------------------- > > > create table p1 (f1 int, g1 int); > > > create table p2 (f1 int, h1 int); > > > create table c () inherits(p1, p2); > > > drop column p2.f1; -- this DROP is in fact implicitly ONLY > > > > Surely not? At least, I don't see why it should be thought of that way. > > There's always a difference between DROP and DROP ONLY. > > What will be the difference in the user-visible schema ? If I understand the issue correctly, this is the key point to this discussion. The user will not see a difference in schemas, no matter which way you look at it. But to the system catalogs there are two ways of representing this situation: f1 being defined locally by c (and also inherited from p1) or not (and only inherited from p1). I think the difference is purely phylosophical, and there are no arguments that can convince either party that it is wrong. Anyway, there's always a set of commands that can make the user go from one representation to the other. He just has to be careful and know exactly which way the system will work. Whichever way it works, it should be clearly and carefully documented in the ALTER TABLE reference. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Cuando no hay humildad las personas se degradan" (A. Christie)
On Wed, 2002-09-25 at 04:33, Alvaro Herrera wrote: > Hannu Krosing dijo: > > > On Wed, 2002-09-25 at 04:13, Tom Lane wrote: > > > Hannu Krosing <hannu@tm.ee> writes: > > > > 1) -------------------------------- > > > > create table p1 (f1 int, g1 int); > > > > create table p2 (f1 int, h1 int); > > > > create table c () inherits(p1, p2); > > > > drop column p2.f1; -- this DROP is in fact implicitly ONLY > > > > > > Surely not? At least, I don't see why it should be thought of that way. > > > There's always a difference between DROP and DROP ONLY. > > > > What will be the difference in the user-visible schema ? > > If I understand the issue correctly, this is the key point to this > discussion. The user will not see a difference in schemas, no matter > which way you look at it. But to the system catalogs there are two ways > of representing this situation: f1 being defined locally by c (and also > inherited from p1) or not (and only inherited from p1). Ok, I think I'm beginning to see Tom's point. So what Tom wants is that doing DROP ONLY will push the definition down the hierarchy on first possibility only as a last resort. For me it feels assymmetric (unless we will make attislocal also int instead of boolean ;). This assymetric nature will manifest itself when we will have ADD COLUMN which can put back the DROP ONLY COLUMN and it has to determine weather to remove the COLUMN definition from the child. What does the current model do in the following case: create table p (f1 int, g1 int); create table c (f1 int) inherits(p); drop column c.f1; Will it just set attisinh = 1 on c.f1 ? what would drop column p.f1; have done - would it have left c.f1 intact? > I think the difference is purely phylosophical, and there are no > arguments that can convince either party that it is wrong. There seem to be actually 3 different possible behaviours for DROP COLUMN for hierarchies. 1. DROP ONLY - the weakest - drops the column and moves the "original" (or explicit, defined-here) definition down to all children if not already found there too. 2. DROP - midlevel - drops the column and its inherited definitions in children but stops at first foreign definition (defined locally or inherited from other parents). 3. DROP FORCE - strongest ( more or less what current drop seems to do.) - walks down the hierarchy and removes all definitions, weather inherited or local, only leaves definitions inherited from other parents. Perhaps it should just fail in case of multiply inherited field ? Maybe it was too early to put the DROP ONLY functionality in ? > Anyway, there's always a set of commands that can make the user go from > one representation to the other. He just has to be careful and know > exactly which way the system will work. Whichever way it works, it > should be clearly and carefully documented in the ALTER TABLE reference. Amen. -------------- Hannu
Hannu Krosing dijo: > For me it feels assymmetric (unless we will make attislocal also int > instead of boolean ;). This assymetric nature will manifest itself when > we will have ADD COLUMN which can put back the DROP ONLY COLUMN and it > has to determine weather to remove the COLUMN definition from the child. Well, the ADD COLUMN thing is something I haven't think about. Let's see: if I have a child with a local definition of the column I'm adding, I have to add one to its inhcount, that's clear. But do I have to reset its attislocal? > What does the current model do in the following case: > > create table p (f1 int, g1 int); > create table c (f1 int) inherits(p); > drop column c.f1; > > Will it just set attisinh = 1 on c.f1 ? No, it will forbid you to drop the column. That was the intention on the first place: if a column is inherited, you shouldn't be allowed to drop or rename it. You can only do so at the top of the inheritance tree, either recursively or non-recursively. And when you do it non-recursively, the first level is marked non-inherited. > There seem to be actually 3 different possible behaviours for DROP > COLUMN for hierarchies. Well, I'm not too eager to discuss this kind of thing: it's possible that multiple inheritance goes away in a future release, and all these issues will possibly vanish. But I'm not sure I understand the implications of "interfaces" (a la Java multiple inheritance). -- Alvaro Herrera (<alvherre[a]atentus.com>) "Acepta los honores y aplausos y perderas tu libertad"
Alvaro Herrera kirjutas K, 25.09.2002 kell 02:45: > Hannu Krosing dijo: > > > For me it feels assymmetric (unless we will make attislocal also int > > instead of boolean ;). This assymetric nature will manifest itself when > > we will have ADD COLUMN which can put back the DROP ONLY COLUMN and it > > has to determine weather to remove the COLUMN definition from the child. > > Well, the ADD COLUMN thing is something I haven't think about. Let's > see: if I have a child with a local definition of the column I'm adding, > I have to add one to its inhcount, that's clear. But do I have to reset > its attislocal? I'd guess that it should reset attislocal if ONLY is specified (to be symmetric with behaviour of drop ONLY). > > What does the current model do in the following case: > > > > create table p (f1 int, g1 int); > > create table c (f1 int) inherits(p); > > drop column c.f1; > > > > Will it just set attisinh = 1 on c.f1 ? > > No, it will forbid you to drop the column. That was the intention on > the first place: if a column is inherited, you shouldn't be allowed to > drop or rename it. You can only do so at the top of the inheritance > tree, either recursively or non-recursively. And when you do it > non-recursively, the first level is marked non-inherited. And my views differed from Tom's on weather to do it always or only when the column was dropped the last parent providing it for inheritance. Lets hope that possible move from INHERITS to (LIKE,...)UNDER will make these issues clearer and thus easier to discuss and agree upon. > > There seem to be actually 3 different possible behaviours for DROP > > COLUMN for hierarchies. > > Well, I'm not too eager to discuss this kind of thing: it's possible > that multiple inheritance goes away in a future release, and all these > issues will possibly vanish. But I'm not sure I understand the > implications of "interfaces" (a la Java multiple inheritance). I don't think that issues for inheriting multiple columns will vanish even for SQL99 way of doing nheritance (LIKE/UNDER), as there can be multiple LIKE's and afaik they too should track changes in parent columns. But I don't think that it is very important to reach concensus for 7.3 as the whole inheritance area in postgres will likely be changed. I think these will be items for discussion once 7.4 cycle starts. ------------- Hannu
En Thu, 19 Sep 2002 14:06:05 -0400 Tom Lane <tgl@sss.pgh.pa.us> escribió: > Alvaro Herrera <alvherre@atentus.com> writes: > > Tom Lane dijo: > >> One corner case is that I think we currently allow > >> > >> create table p (f1 int); > >> create table c (f1 int) inherits(p); > > > In this case, c.f1.attisinherited count is 2; thus when I drop f1 from > > p, it is not dropped from c. > > That seems right, but the problem I have with it is that the resulting > state of c.f1 is attisinherited = 1. This means that you cannot drop > c.f1. It seems to me that we should have this behavior: New patch attached. This one should answer your concerns. This is the idea implemented: > We might need two fields :-( ... a "locally defined" boolean and a > "number of times inherited" counter. Some discussion: > create table p (f1 int); > create table c (f1 int not null) inherits(p); > > drop column p.f1; > -- c.f1 GOES AWAY, because its inherit count went to zero In this case, the attached code preserves f1. It's not clear whether the user wants the column to stay or not, but if he is defining it twice, let him drop it twice if he wants it to go away. > Another interesting case is multiple inheritance. > > create table p1 (f1 int); > create table p2 (f1 int); > create table c () inherits(p1, p2); > > drop ONLY column p1.f1; > drop column p2.f1; > > After this sequence, what is the state of c.f1? Is it still there? > Should it be? If it is still there, will it be possible to get rid of > it with "drop column c.f1"? What if we did DROP ONLY on *both* > ancestors? Well, in this case the column is dropped. If the last drop is ONLY, the column will stay (regardless of what the first drop did). This one seems very tricky and I don't see a way to do otherwise. Other cases (such as the set of four you posted) are handled in the "natural" way you described. Regression tests for all those four are included, along another case that was the start of all this. Please review the patch. It should be current as of your commit of 20:30 today, but I'm not sure (anoncvs delays and all -- there are changes to the same files). -- Alvaro Herrera (<alvherre[a]atentus.com>) "Hay dos momentos en la vida de un hombre en los que no debería especular: cuando puede permitírselo y cuando no puede" (Mark Twain)
Attachment
En Mon, 23 Sep 2002 09:53:08 -0400 Tom Lane <tgl@sss.pgh.pa.us> escribió: > > You cannot add a column to a table that is inherited by another table > > that has a column with the same name: > > Yeah, this is an implementation shortcoming in ALTER ADD COLUMN: if it > finds an existing column of the same name in a child table, it should > test whether it's okay to "merge" the columns (same types, no conflict > in constraints/defaults, cf CREATE's behavior); if so, it should > increment the child column's attinhcount instead of failing. I have this almost ready. The thing I don't have quite clear yet is what to do with attislocal. IMHO it should not be touched in any case, but Hannu thinks that for symmetry it should be reset in some cases. Also, what do you mean by conflicts on defaults? I don't think the parent should take into consideration what the defaults are for its children. Same for constraints. -- Alvaro Herrera (<alvherre[a]atentus.com>) Si no sabes adonde vas, es muy probable que acabes en otra parte.
Alvaro Herrera <alvherre@atentus.com> writes:
> I have this almost ready. The thing I don't have quite clear yet is
> what to do with attislocal. IMHO it should not be touched in any case,
> but Hannu thinks that for symmetry it should be reset in some cases.
My feeling would be to leave it alone in all cases. If I have
create table p (f1 int);create table c (f2 text) inherits (p);
I would find it quite surprising if I could destroy c.f2 by adding
and then dropping p.f2.
> Also, what do you mean by conflicts on defaults? I don't think the
> parent should take into consideration what the defaults are for its
> children. Same for constraints.
Well, the rules will probably have to be different for this case than
they are when creating a child below an existing parent. In particular,
if the ADD COLUMN operation is trying to create constraints (including
a simple NOT NULL), I'm inclined to fail rather than merge if the
existing child column does not already have matching constraints.
It would seem surprising to me that creating a parent column in this
way could allow the formerly free-standing child column to suddenly
have constraints it didn't have before. Also, you'd have to scan the
child rows to see whether they all meet the constraint, which would
be slow. For example, if you wanted to do
alter table p add column f2 text not null;
in the above example, I think it is reasonable to insist that you first
do
alter table c alter column f2 set not null;
to make it perfectly clear all 'round that you are accepting an
alteration in the existing column.
regards, tom lane
Tom Lane kirjutas P, 29.09.2002 kell 04:00: > Alvaro Herrera <alvherre@atentus.com> writes: > > I have this almost ready. The thing I don't have quite clear yet is > > what to do with attislocal. IMHO it should not be touched in any case, > > but Hannu thinks that for symmetry it should be reset in some cases. I'd propose that ADD ONLY would pull topmost attislocal up (reset it from the (grand)child) whereas plain ADD would leave attislocal alone. The use of ONLY with this meaning is for the symmetry with DROP ONLY. > My feeling would be to leave it alone in all cases. If I have > > create table p (f1 int); > create table c (f2 text) inherits (p); > > I would find it quite surprising if I could destroy c.f2 by adding > and then dropping p.f2. This should depend on weather you drop ONLY Or are you also be surprised by this behaviour of DELETE CASCADE :) hannu=# create table c(i int); CREATE TABLE hannu=# insert into c values(1); INSERT 41595 1 hannu=# insert into c values(2); INSERT 41596 1 hannu=# create table p (pk int primary key); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'p_pkey' for table 'p' CREATE TABLE hannu=# insert into p values(1); INSERT 41601 1 hannu=# insert into p values(2); INSERT 41602 1 hannu=# alter table c add constraint fk foreign key (i) hannu-# references p on delete cascade; NOTICE: ALTER TABLE will create implicit trigger(s) for FOREIGN KEY check(s) ALTER TABLE hannu=# delete from p where pk=2; DELETE 1 hannu=# select * from c;i ---1 (1 row) Surprise: Where did i=2 go ?? What you are proposing is IMHO equivalent to making FOREIGN KEYs ON DELETE CASCADE behaviour dependant on weather the foreign key was created initially or added afterwards. > > Also, what do you mean by conflicts on defaults? I don't think the > > parent should take into consideration what the defaults are for its > > children. Same for constraints. > > Well, the rules will probably have to be different for this case than > they are when creating a child below an existing parent. In particular, > if the ADD COLUMN operation is trying to create constraints (including > a simple NOT NULL), I'm inclined to fail rather than merge if the > existing child column does not already have matching constraints. > It would seem surprising to me that creating a parent column in this > way could allow the formerly free-standing child column to suddenly > have constraints it didn't have before. Also, you'd have to scan the > child rows to see whether they all meet the constraint, which would > be slow. For example, if you wanted to do > > alter table p add column f2 text not null; > > in the above example, I think it is reasonable to insist that you first > do > > alter table c alter column f2 set not null; To this I strongly agree. ----------------- Hannu
Hannu Krosing <hannu@tm.ee> writes:
> I'd propose that ADD ONLY would pull topmost attislocal up (reset it
> from the (grand)child) whereas plain ADD would leave attislocal alone.
ADD ONLY? There is no such animal as ADD ONLY, and cannot be because
it implies making a parent inconsistent with its children. (Yes, I
know that the code takes that combination right now, but erroring out
instead is on the "must fix before release" list. Ditto for RENAME
ONLY.)
> The use of ONLY with this meaning is for the symmetry with DROP ONLY.
But it's not a symmetrical situation. The children must contain every
column in the parent; the reverse is not true. Some asymmetry in the
commands is therefore unavoidable.
>> I would find it quite surprising if I could destroy c.f2 by adding
>> and then dropping p.f2.
> This should depend on weather you drop ONLY
I disagree. Your analogy to a CASCADE foreign key is bad, because
the foreign key constraint is attached to the column that might lose
data. Thus you (presumably) know when you create the constraint what
you are risking. Losing existing child data because of manipulations
done only on the parent --- perhaps not even remembering that there
is a conflicting child column --- strikes me as dangerous. It seems
like an indirect, "action at a distance" behavior.
Here is another scenario: suppose p has many children, but only c42
has a column f2. If I "alter table p add column f2", now p and
all the c's will have f2. Suppose I realize that was a mistake.
Can I undo it with "alter table p drop column f2"? Yes, under my
proposal; no, under yours. In yours, the only way would be to
do a DROP ONLY on p and then retail DROPs on each of the other
children. This would be tedious and error-prone. If some random
subset of the children had f2, it'd be even worse --- it would
be difficult even to identify which children had f2 before the
ADD operation. IMHO this is a good example of why attislocal is
useful.
regards, tom lane
On Sun, 29 Sep 2002, Tom Lane wrote: > Hannu Krosing <hannu@tm.ee> writes: > > I'd propose that ADD ONLY would pull topmost attislocal up (reset it > > from the (grand)child) whereas plain ADD would leave attislocal alone. > > ADD ONLY? There is no such animal as ADD ONLY, and cannot be because > it implies making a parent inconsistent with its children. (Yes, I > know that the code takes that combination right now, but erroring out > instead is on the "must fix before release" list. Ditto for RENAME > ONLY.) I'm leaving right now and can't participate in the whole discussion, but I implemented "ADD ONLY" as a way to add the column only in the parent (all children should already have to column, errors if at least one doesn't or is different atttype), while "ADD" adds the column to children that don't have it and merges where already exist; it errors if children have different atttype etc. Should I rip the ADD ONLY part out? -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Pido que me den el Nobel por razones humanitarias" (Nicanor Parra)
On Sun, 2002-09-29 at 19:57, Tom Lane wrote: > Hannu Krosing <hannu@tm.ee> writes: > > I'd propose that ADD ONLY would pull topmost attislocal up (reset it > > from the (grand)child) whereas plain ADD would leave attislocal alone. > > ADD ONLY? There is no such animal as ADD ONLY, and cannot be because > it implies making a parent inconsistent with its children. I meant ADD ONLY to be the exact opposite of DROP ONLY - it adds parent column and removes attislocal from children. Simple ADD would _not_ remove attislocal from children with matching column. > > The use of ONLY with this meaning is for the symmetry with DROP ONLY. > > But it's not a symmetrical situation. The children must contain every > column in the parent; the reverse is not true. Some asymmetry in the > commands is therefore unavoidable. Perhaps some mirror command then: DROP ONLY <--> ADD ALL ? > >> I would find it quite surprising if I could destroy c.f2 by adding > >> and then dropping p.f2. > > > This should depend on weather you drop ONLY > > I disagree. Your analogy to a CASCADE foreign key is bad, because > the foreign key constraint is attached to the column that might lose > data. Thus you (presumably) know when you create the constraint what > you are risking. Losing existing child data because of manipulations > done only on the parent --- perhaps not even remembering that there > is a conflicting child column --- strikes me as dangerous. It seems > like an indirect, "action at a distance" behavior. What about warning the user and making him use FORCE in ambiguous cases (like when some children don't have that column) ? > Here is another scenario: suppose p has many children, but only c42 > has a column f2. If I "alter table p add column f2", now p and > all the c's will have f2. Suppose I realize that was a mistake. > Can I undo it with "alter table p drop column f2"? Yes, under my > proposal; no, under yours. "YES" under mine, unless you did "alter table ONLY p add column f2" , which would have removed the local definition from children. > In yours, the only way would be to > do a DROP ONLY on p and then retail DROPs on each of the other > children. This would be tedious and error-prone. If some random > subset of the children had f2, it'd be even worse --- it would > be difficult even to identify which children had f2 before the > ADD operation. Your proposal and mine are the same in case ONLY is not given. The option ADD ONLY is proposed just to make it easy to undo a DROP ONLY. Under your proposal I see no easy way to undo DROP ONLY (for example to do DROP instead). > IMHO this is a good example of why attislocal is useful. I don't doubt usefulness of attislocal, I just want to make sure it is used in a consistent manner. ------------- Hannu
On 29 Sep 2002, Hannu Krosing wrote: > On Sun, 2002-09-29 at 19:57, Tom Lane wrote: > > Hannu Krosing <hannu@tm.ee> writes: > > > I'd propose that ADD ONLY would pull topmost attislocal up (reset it > > > from the (grand)child) whereas plain ADD would leave attislocal alone. > > > > ADD ONLY? There is no such animal as ADD ONLY, and cannot be because > > it implies making a parent inconsistent with its children. > > I meant ADD ONLY to be the exact opposite of DROP ONLY - it adds parent > column and removes attislocal from children. Simple ADD would _not_ > remove attislocal from children with matching column. Consistency requires that it be exactly the opposite. When you ADD ONLY, you want only in the "local" table, so children still have a local definition; OTOH, when you ADD (recursively) you want all children to get non-local status. Suppose CREATE TABLE p (f1 int); CREATE TABLE c (f2 int) INHERITS (p); c.f2.attislocal = true Now, ALTER TABLE ONLY p ADD COLUMN f2 int should leavy c.f2.attislocal alone, while ALTER TABLE p ADD COLUMN f2 int should reset it. This is the opposite of your proposal, and I don't think it exists in Tom's proposal. I think this is also consistent with the fact that ONLY requires the column to exist in all children, while non-ONLY creates it where it doesn't exist, and merges (resetting attislocal if set -- it could be inherited from some other parent) where it exists. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Nunca se desea ardientemente lo que solo se desea por razon" (F. Alexandre)
On Mon, 2002-09-30 at 00:05, Alvaro Herrera wrote: > On 29 Sep 2002, Hannu Krosing wrote: > > > On Sun, 2002-09-29 at 19:57, Tom Lane wrote: > > > Hannu Krosing <hannu@tm.ee> writes: > > > > I'd propose that ADD ONLY would pull topmost attislocal up (reset it > > > > from the (grand)child) whereas plain ADD would leave attislocal alone. > > > > > > ADD ONLY? There is no such animal as ADD ONLY, and cannot be because > > > it implies making a parent inconsistent with its children. > > > > I meant ADD ONLY to be the exact opposite of DROP ONLY - it adds parent > > column and removes attislocal from children. Simple ADD would _not_ > > remove attislocal from children with matching column. > > Consistency requires that it be exactly the opposite. Consistency seems to mean different things to different people - an a "natural" meaning is often hard to see in a non-natural language (SQL). So is it "ADD ONLY to table" or "ADD the ONLY definition" or "ADD ONLY don't reset attislocal" or "ADD ONLY as opposite of DROP ONLY") But I'd be happy with any meaning, as long as the functionality is there and it is clearly documented. Your definition of "ADD to this table ONLY and leave other definitions alone" is easy to accept. > When you ADD > ONLY, you want only in the "local" table, so children still have a local > definition; OTOH, when you ADD (recursively) you want all children to > get non-local status. Perhaps ADD should either have ONLY or ALL and function without either only when there is no matching column in any of the child tables. > Suppose > CREATE TABLE p (f1 int); > CREATE TABLE c (f2 int) INHERITS (p); > c.f2.attislocal = true > > Now, > ALTER TABLE ONLY p ADD COLUMN f2 int > should leavy c.f2.attislocal alone, while > ALTER TABLE p ADD COLUMN f2 int > should reset it. > > This is the opposite of your proposal, and I don't think it exists in > Tom's proposal. I also like the ablility to undo accidental DROP ONLY, which is missing in Toms proposal. > I think this is also consistent with the fact that ONLY requires the > column to exist in all children, while non-ONLY creates it where it > doesn't exist, and merges (resetting attislocal if set -- it could be > inherited from some other parent) where it exists. For completeness there should be a third behaviour that would work like ONLY for existing columns in children, but add it to children where it is missing. This would be needed to effectively undo a DROP COLUMN where it was multiply inherited and/or locally defined in some children. ---------------------- Hannu
Alvaro Herrera <alvherre@atentus.com> writes:
> I implemented "ADD ONLY" as a way to add the column only in the parent
> (all children should already have to column, errors if at least one
> doesn't or is different atttype), while "ADD" adds the column to
> children that don't have it and merges where already exist; it errors if
> children have different atttype etc.
I fail to see the value in such a distinction. The end state is the same
in both cases, no?
regards, tom lane
Where are we with this patch? --------------------------------------------------------------------------- Alvaro Herrera wrote: > On 29 Sep 2002, Hannu Krosing wrote: > > > On Sun, 2002-09-29 at 19:57, Tom Lane wrote: > > > Hannu Krosing <hannu@tm.ee> writes: > > > > I'd propose that ADD ONLY would pull topmost attislocal up (reset it > > > > from the (grand)child) whereas plain ADD would leave attislocal alone. > > > > > > ADD ONLY? There is no such animal as ADD ONLY, and cannot be because > > > it implies making a parent inconsistent with its children. > > > > I meant ADD ONLY to be the exact opposite of DROP ONLY - it adds parent > > column and removes attislocal from children. Simple ADD would _not_ > > remove attislocal from children with matching column. > > Consistency requires that it be exactly the opposite. When you ADD > ONLY, you want only in the "local" table, so children still have a local > definition; OTOH, when you ADD (recursively) you want all children to > get non-local status. > > Suppose > CREATE TABLE p (f1 int); > CREATE TABLE c (f2 int) INHERITS (p); > c.f2.attislocal = true > > Now, > ALTER TABLE ONLY p ADD COLUMN f2 int > should leavy c.f2.attislocal alone, while > ALTER TABLE p ADD COLUMN f2 int > should reset it. > > This is the opposite of your proposal, and I don't think it exists in > Tom's proposal. > > I think this is also consistent with the fact that ONLY requires the > column to exist in all children, while non-ONLY creates it where it > doesn't exist, and merges (resetting attislocal if set -- it could be > inherited from some other parent) where it exists. > > -- > Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) > "Nunca se desea ardientemente lo que solo se desea por razon" (F. Alexandre) > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- 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:
> Where are we with this patch?
It's done as far as I'm concerned ;-). Not sure if Hannu still wants
to argue that the behavior is wrong ... it seems fine to me though ...
regards, tom lane
On Fri, 2002-10-04 at 01:00, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Where are we with this patch? > > It's done as far as I'm concerned ;-). Not sure if Hannu still wants > to argue that the behavior is wrong ... it seems fine to me though ... I stop arguing for now, "ONLY" can mean too many things ;) I can't promise that I don't bring some of it up again when we will start discussing a more general overhaul of our inheritance and OO . --------------- Hannu
On Thu, Oct 03, 2002 at 04:00:32PM -0400, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Where are we with this patch? > > It's done as far as I'm concerned ;-). Not sure if Hannu still wants > to argue that the behavior is wrong ... it seems fine to me though ... I still haven't submitted the ALTER TABLE/ADD COLUMN part. There's a little thing I want to change first. It's a different issue though (but related). -- Alvaro Herrera (<alvherre[a]atentus.com>) "El que vive para el futuro es un iluso, y el que vive para el pasado, un imbécil" (Luis Adler, "Los tripulantes de la noche")