Hello hackers,
I'm thinking about ALTER TABLE ... ADD COLUMN working properly when
child tables already contain the column.
I have two proposals. First one:
There are two cases: one when specifying ALTER TABLE ONLY, and other
when specifying recursive (not ONLY).
In the first (ONLY) case, child tables are checked for existance of the
column. If any child does not contain the column or it has a different
atttypid or attypmod (any other field that should be checked?), the
request is aborted. Else the column is added in the parent only, and
the child attributes have their attinhcount incremented.
In the second case, child tables are checked for existance of the
column. If the column doesn't exist, the procedure is called
recursively. If the column exists but has a different atttypid o
atttypmod, the request is aborted. Else, the attribute has its
attinhcount incremented and attislocal reset.
There are two differences between these two cases:
- ONLY does not add a column in childs. This seems natural. Obviously the scenario where this can lead to trouble
producesan error (inexistent attr in child).
- ONLY does not touch attislocal in childs.
The second may seems odd, but consider the following scenario:
CREATE TABLE p1 (f1 int);
CREATE TABLE p2 (f2 int);
CREATE TABLE c (f1 int) INHERITS (p1, p2);
In this case, c.f1.attislocal is true. Now suppose the user wants to
create p2.f1. If the recursive version is used, attislocal will be
reset and the scenario will be equivalent to
CREATE TABLE p1 (f1 int);
CREATE TABLE p2 (f1 int, f2 int);
CREATE TABLE c () INHERITS (p1, p2);
but the natural way would be
CREATE TABLE p1 (f1 int);
CREATE TABLE p2 (f1 int, f2 int);
CREATE TABLE c (f1 int) INHERITS (p1, p2);
which is what the ONLY case does.
Second proposal: Another way of doing this would be resetting attislocal
iff attinhcount is exactly zero. This assumes that if attislocal is
true and attinhcount is not zero, the user wants the column as locally
defined and we should keep it that way. On the other hand, if a child
table already has the column there's no way to push the definition up
the inheritance tree.
What do you think? I have actually a patch ready that implements the
first proposal; the second one, while simpler conceptually and in terms
of code, occurred to me just as I was writing this, and has an issue of
completeness.
--
Alvaro Herrera (<alvherre[a]atentus.com>)
"Las cosas son buenas o malas segun las hace nuestra opinion" (Lisias)
