Thread: Re: [HACKERS] pg_attribute.attisinherited ?
En Sat, 24 Aug 2002 17:09:55 -0400
I said:
> > I remember Tom suggested adding something like attisinherited and
> > preventing this kind of operations on such attributes, because one can
> > do things such as [...]
Ok, I attach a patch that does this. It doesn't include regression
tests, docs nor the checks against unwanted operations; these will come
later if people think this is a good approach.
It passes 86 of 88 tests. The 2 failures are ordering issues (diff
below) I don't know what causes it.
Please review.
*** ./expected/select_having.out Wed Jun 26 17:58:56 2002
--- ./results/select_having.out Sat Aug 24 18:32:16 2002
***************
*** 26,33 ****
GROUP BY b, c HAVING b = 3;
b | c
---+----------
- 3 | BBBB
3 | bbbb
(2 rows)
SELECT lower(c), count(c) FROM test_having
--- 26,33 ----
GROUP BY b, c HAVING b = 3;
b | c
---+----------
3 | bbbb
+ 3 | BBBB
(2 rows)
SELECT lower(c), count(c) FROM test_having
***************
*** 43,50 ****
GROUP BY c HAVING count(*) > 2 OR min(a) = max(a);
c | max
----------+-----
- XXXX | 0
bbbb | 5
(2 rows)
DROP TABLE test_having;
--- 43,50 ----
GROUP BY c HAVING count(*) > 2 OR min(a) = max(a);
c | max
----------+-----
bbbb | 5
+ XXXX | 0
(2 rows)
DROP TABLE test_having;
======================================================================
*** ./expected/rules.out Mon Aug 19 01:08:30 2002
--- ./results/rules.out Sat Aug 24 18:32:46 2002
***************
*** 404,412 ****
----------------------+--------------+------------+------------+------------
gates | t | fired | $0.00 | $80,000.00
gates | t | hired | $80,000.00 | $0.00
- wiech | t | hired | $5,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
(5 rows)
insert into rtest_empmass values ('meyer', '4000.00');
--- 404,412 ----
----------------------+--------------+------------+------------+------------
gates | t | fired | $0.00 | $80,000.00
gates | t | hired | $80,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
+ wiech | t | hired | $5,000.00 | $0.00
(5 rows)
insert into rtest_empmass values ('meyer', '4000.00');
***************
*** 421,429 ****
maier | t | hired | $5,000.00 | $0.00
mayr | t | hired | $6,000.00 | $0.00
meyer | t | hired | $4,000.00 | $0.00
- wiech | t | hired | $5,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
(8 rows)
update rtest_empmass set salary = salary + '1000.00';
--- 421,429 ----
maier | t | hired | $5,000.00 | $0.00
mayr | t | hired | $6,000.00 | $0.00
meyer | t | hired | $4,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
+ wiech | t | hired | $5,000.00 | $0.00
(8 rows)
update rtest_empmass set salary = salary + '1000.00';
***************
*** 439,447 ****
mayr | t | honored | $7,000.00 | $6,000.00
meyer | t | hired | $4,000.00 | $0.00
meyer | t | honored | $5,000.00 | $4,000.00
- wiech | t | hired | $5,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
(11 rows)
delete from rtest_emp where ename = rtest_empmass.ename;
--- 439,447 ----
mayr | t | honored | $7,000.00 | $6,000.00
meyer | t | hired | $4,000.00 | $0.00
meyer | t | honored | $5,000.00 | $4,000.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
+ wiech | t | hired | $5,000.00 | $0.00
(11 rows)
delete from rtest_emp where ename = rtest_empmass.ename;
***************
*** 459,467 ****
meyer | t | fired | $0.00 | $5,000.00
meyer | t | hired | $4,000.00 | $0.00
meyer | t | honored | $5,000.00 | $4,000.00
- wiech | t | hired | $5,000.00 | $0.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
(14 rows)
--
--- 459,467 ----
meyer | t | fired | $0.00 | $5,000.00
meyer | t | hired | $4,000.00 | $0.00
meyer | t | honored | $5,000.00 | $4,000.00
wieck | t | honored | $6,000.00 | $5,000.00
wieck | t | honored | $7,000.00 | $6,000.00
+ wiech | t | hired | $5,000.00 | $0.00
(14 rows)
--
======================================================================
--
Alvaro Herrera (<alvherre[a]atentus.com>)
"Un poeta es un mundo encerrado en un hombre" (Victor Hugo)
Attachment
Hi Alvaro,
Yeah it is an issue that needs to be fixed. I'll have a look at your
patch once someone comments on those ordering issue?
Although I note that I've had ordering issues in the RULES test for ages
now on FreeBSD/Alpha...
Chris
On Sat, 24 Aug 2002, Alvaro Herrera wrote:
> En Sat, 24 Aug 2002 17:09:55 -0400
> I said:
>
> > > I remember Tom suggested adding something like attisinherited and
> > > preventing this kind of operations on such attributes, because one can
> > > do things such as [...]
>
> Ok, I attach a patch that does this. It doesn't include regression
> tests, docs nor the checks against unwanted operations; these will come
> later if people think this is a good approach.
>
> It passes 86 of 88 tests. The 2 failures are ordering issues (diff
> below) I don't know what causes it.
>
> Please review.
>
>
> *** ./expected/select_having.out Wed Jun 26 17:58:56 2002
> --- ./results/select_having.out Sat Aug 24 18:32:16 2002
> ***************
> *** 26,33 ****
> GROUP BY b, c HAVING b = 3;
> b | c
> ---+----------
> - 3 | BBBB
> 3 | bbbb
> (2 rows)
>
> SELECT lower(c), count(c) FROM test_having
> --- 26,33 ----
> GROUP BY b, c HAVING b = 3;
> b | c
> ---+----------
> 3 | bbbb
> + 3 | BBBB
> (2 rows)
>
> SELECT lower(c), count(c) FROM test_having
> ***************
> *** 43,50 ****
> GROUP BY c HAVING count(*) > 2 OR min(a) = max(a);
> c | max
> ----------+-----
> - XXXX | 0
> bbbb | 5
> (2 rows)
>
> DROP TABLE test_having;
> --- 43,50 ----
> GROUP BY c HAVING count(*) > 2 OR min(a) = max(a);
> c | max
> ----------+-----
> bbbb | 5
> + XXXX | 0
> (2 rows)
>
> DROP TABLE test_having;
>
> ======================================================================
>
> *** ./expected/rules.out Mon Aug 19 01:08:30 2002
> --- ./results/rules.out Sat Aug 24 18:32:46 2002
> ***************
> *** 404,412 ****
> ----------------------+--------------+------------+------------+------------
> gates | t | fired | $0.00 | $80,000.00
> gates | t | hired | $80,000.00 | $0.00
> - wiech | t | hired | $5,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> (5 rows)
>
> insert into rtest_empmass values ('meyer', '4000.00');
> --- 404,412 ----
> ----------------------+--------------+------------+------------+------------
> gates | t | fired | $0.00 | $80,000.00
> gates | t | hired | $80,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> + wiech | t | hired | $5,000.00 | $0.00
> (5 rows)
>
> insert into rtest_empmass values ('meyer', '4000.00');
> ***************
> *** 421,429 ****
> maier | t | hired | $5,000.00 | $0.00
> mayr | t | hired | $6,000.00 | $0.00
> meyer | t | hired | $4,000.00 | $0.00
> - wiech | t | hired | $5,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> (8 rows)
>
> update rtest_empmass set salary = salary + '1000.00';
> --- 421,429 ----
> maier | t | hired | $5,000.00 | $0.00
> mayr | t | hired | $6,000.00 | $0.00
> meyer | t | hired | $4,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> + wiech | t | hired | $5,000.00 | $0.00
> (8 rows)
>
> update rtest_empmass set salary = salary + '1000.00';
> ***************
> *** 439,447 ****
> mayr | t | honored | $7,000.00 | $6,000.00
> meyer | t | hired | $4,000.00 | $0.00
> meyer | t | honored | $5,000.00 | $4,000.00
> - wiech | t | hired | $5,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> (11 rows)
>
> delete from rtest_emp where ename = rtest_empmass.ename;
> --- 439,447 ----
> mayr | t | honored | $7,000.00 | $6,000.00
> meyer | t | hired | $4,000.00 | $0.00
> meyer | t | honored | $5,000.00 | $4,000.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> + wiech | t | hired | $5,000.00 | $0.00
> (11 rows)
>
> delete from rtest_emp where ename = rtest_empmass.ename;
> ***************
> *** 459,467 ****
> meyer | t | fired | $0.00 | $5,000.00
> meyer | t | hired | $4,000.00 | $0.00
> meyer | t | honored | $5,000.00 | $4,000.00
> - wiech | t | hired | $5,000.00 | $0.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> (14 rows)
>
> --
> --- 459,467 ----
> meyer | t | fired | $0.00 | $5,000.00
> meyer | t | hired | $4,000.00 | $0.00
> meyer | t | honored | $5,000.00 | $4,000.00
> wieck | t | honored | $6,000.00 | $5,000.00
> wieck | t | honored | $7,000.00 | $6,000.00
> + wiech | t | hired | $5,000.00 | $0.00
> (14 rows)
>
> --
>
> ======================================================================
>
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> "Un poeta es un mundo encerrado en un hombre" (Victor Hugo)
>
En Sun, 25 Aug 2002 17:34:21 +0800 (WST)
Christopher Kings-Lynne <chriskl@familyhealth.com.au> escribió:
Hi again,
> Yeah it is an issue that needs to be fixed.
I'm thinking about the ONLY part in the grammar in ALTER TABLE... DROP
COLUMN and RENAME COLUMN. I think they should not be there: they only
create noise and chances of ill behavior. If I modify only the parent
table, then I'm able to create a column on the child table with
different datatype and same name as new column on parent, causing
subsequent backend crash.
Consider
CREATE TABLE foo (a int);
CREATE TABLE bar () INHERITS (foo);
ALTER TABLE ONLY foo RENAME a TO b;
ALTER TABLE bar ADD COLUMN b TEXT;
regression=# INSERT INTO bar values (1, 'hello world');
INSERT 205625 1
regression=# SELECT * FROM foo;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!#
What does people think about removing the support for ONLY in these
directives?
But this is a different problem and requires a different patch.
Here I post a new version of attisinherited; this one includes the tests
in AlterTableDropColumn and renameatt so inherited columns can not be
dropped nor renamed. Please review this new version. Regression tests
are also included, as is the modification of catalog.sgml.
--
Alvaro Herrera (<alvherre[a]atentus.com>)
"El Maquinismo fue proscrito so pena de cosquilleo hasta la muerte"
(Ijon Tichy en Viajes, Stanislaw Lem)
Attachment
Alvaro Herrera <alvherre@atentus.com> writes:
> I'm thinking about the ONLY part in the grammar in ALTER TABLE... DROP
> COLUMN and RENAME COLUMN. I think they should not be there:
Local DROP COLUMN is fine: it just causes the column to become
non-inherited in any children. (Your patch for attisinherited will
need to cover this case.)
Local RENAME COLUMN does need to be prohibited, as does local ADD
COLUMN, as does local ALTER COLUMN if we ever allow changing column
type. Basically we need to prohibit the column from becoming
incompatible with its children.
I don't agree with the notion of changing the grammar to achieve that,
btw. Simpler and more friendly to add a specific error check in
(most likely place) utility/tcop.c. Then if you try to say ONLY you'll
get a more useful complaint than "parse error".
regards, tom lane
Tom Lane dijo: > Alvaro Herrera <alvherre@atentus.com> writes: > > I'm thinking about the ONLY part in the grammar in ALTER TABLE... DROP > > COLUMN and RENAME COLUMN. I think they should not be there: > > Local DROP COLUMN is fine: it just causes the column to become > non-inherited in any children. (Your patch for attisinherited will > need to cover this case.) Oh, I see. > Local RENAME COLUMN does need to be prohibited, as does local ADD > COLUMN, as does local ALTER COLUMN if we ever allow changing column > type. Basically we need to prohibit the column from becoming > incompatible with its children. > I don't agree with the notion of changing the grammar to achieve that, > btw. Simpler and more friendly to add a specific error check in > (most likely place) utility/tcop.c. Then if you try to say ONLY you'll > get a more useful complaint than "parse error". Uh, I added checks in the command itself (command/tablecmds.c), just because I had already done so and to not make tcop/utility.c messier than it already is; I can probably move the check if people thinks it's better. Also implemented is the change from inherited to non-inherited when local-dropping a column. I also changed the text of some error messages from "renameatt: cannot foo" to "ALTER TABLE: cannot foo". But my choose in wording of new error messages probably needs improvement (suggestions welcome). Please review; I have not received comments on whether this implementation is a good approach: note the signature change of TupleDescInitEntry(). -- Alvaro Herrera (<alvherre[a]atentus.com>) "La espina, desde que nace, ya pincha" (Proverbio africano)
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Alvaro Herrera wrote:
> Tom Lane dijo:
>
> > Alvaro Herrera <alvherre@atentus.com> writes:
> > > I'm thinking about the ONLY part in the grammar in ALTER TABLE... DROP
> > > COLUMN and RENAME COLUMN. I think they should not be there:
> >
> > Local DROP COLUMN is fine: it just causes the column to become
> > non-inherited in any children. (Your patch for attisinherited will
> > need to cover this case.)
>
> Oh, I see.
>
> > Local RENAME COLUMN does need to be prohibited, as does local ADD
> > COLUMN, as does local ALTER COLUMN if we ever allow changing column
> > type. Basically we need to prohibit the column from becoming
> > incompatible with its children.
>
> > I don't agree with the notion of changing the grammar to achieve that,
> > btw. Simpler and more friendly to add a specific error check in
> > (most likely place) utility/tcop.c. Then if you try to say ONLY you'll
> > get a more useful complaint than "parse error".
>
> Uh, I added checks in the command itself (command/tablecmds.c), just
> because I had already done so and to not make tcop/utility.c messier
> than it already is; I can probably move the check if people thinks it's
> better. Also implemented is the change from inherited to non-inherited
> when local-dropping a column.
>
> I also changed the text of some error messages from "renameatt: cannot
> foo" to "ALTER TABLE: cannot foo". But my choose in wording of new
> error messages probably needs improvement (suggestions welcome).
>
> Please review; I have not received comments on whether this
> implementation is a good approach: note the signature change of
> TupleDescInitEntry().
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> "La espina, desde que nace, ya pincha" (Proverbio africano)
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
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, Pennsylvania 19073
Patch withdrawn by author. --------------------------------------------------------------------------- Alvaro Herrera wrote: > Tom Lane dijo: > > > Alvaro Herrera <alvherre@atentus.com> writes: > > > I'm thinking about the ONLY part in the grammar in ALTER TABLE... DROP > > > COLUMN and RENAME COLUMN. I think they should not be there: > > > > Local DROP COLUMN is fine: it just causes the column to become > > non-inherited in any children. (Your patch for attisinherited will > > need to cover this case.) > > Oh, I see. > > > Local RENAME COLUMN does need to be prohibited, as does local ADD > > COLUMN, as does local ALTER COLUMN if we ever allow changing column > > type. Basically we need to prohibit the column from becoming > > incompatible with its children. > > > I don't agree with the notion of changing the grammar to achieve that, > > btw. Simpler and more friendly to add a specific error check in > > (most likely place) utility/tcop.c. Then if you try to say ONLY you'll > > get a more useful complaint than "parse error". > > Uh, I added checks in the command itself (command/tablecmds.c), just > because I had already done so and to not make tcop/utility.c messier > than it already is; I can probably move the check if people thinks it's > better. Also implemented is the change from inherited to non-inherited > when local-dropping a column. > > I also changed the text of some error messages from "renameatt: cannot > foo" to "ALTER TABLE: cannot foo". But my choose in wording of new > error messages probably needs improvement (suggestions welcome). > > Please review; I have not received comments on whether this > implementation is a good approach: note the signature change of > TupleDescInitEntry(). > > -- > Alvaro Herrera (<alvherre[a]atentus.com>) > "La espina, desde que nace, ya pincha" (Proverbio africano) Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org -- 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, Pennsylvania 19073
Bruce Momjian dijo: > Your patch has been added to the PostgreSQL unapplied patches list at: > > http://candle.pha.pa.us/cgi-bin/pgpatches > > I will try to apply it within the next 48 hours. Don't. I need to resync. Will post a new version later, hopefully today. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Aprender sin pensar es inutil; pensar sin aprender, peligroso" (Confucio)
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Your patch has been added to the PostgreSQL unapplied patches list at:
I'd like to review this before it's applied ...
regards, tom lane
En Tue, 27 Aug 2002 17:26:56 -0400 (EDT) Bruce Momjian <pgman@candle.pha.pa.us> escribió: > > Patch withdrawn by author. Ok, new version. Please remember to change catversion. Description of this patch: - Adds a new attribute in pg_attribute named attisinherited. - Creation of tables marks it true for attributes that are inherited - Addition of new attribute to existing inherited table marks the attribute as inherited for child tables. - Checked when trying to rename inherited attributes: if table has inheritors, only allow renaming if asked to recurse. Disallow renaming for child tables only. - Checked when trying to drop inherited attributes: if table has inheritors, mark attribute as non-inherited for direct inheritors. Disallow dropping for child tables only. As an added bonus - Check inheritance when adding new attributes (if table has inheritors, only allow new attribute if it's inherited also). -- Alvaro Herrera (<alvherre[a]atentus.com>) "Acepta los honores y aplausos y perderas tu libertad"
En Wed, 28 Aug 2002 03:59:48 -0400 Alvaro Herrera <alvherre@atentus.com> escribió: > En Tue, 27 Aug 2002 17:26:56 -0400 (EDT) > Bruce Momjian <pgman@candle.pha.pa.us> escribió: > > > > > Patch withdrawn by author. > > Ok, new version. Please remember to change catversion. Doh, patch really attached this time. -- Alvaro Herrera (<alvherre[a]atentus.com>) "Cuando miro a alguien, mas me atrae como cambia que quien es" (J. Binoche)
Attachment
Here is a tip. After sending too many emails with no attachment, I decided that as soon as I mention an attachment, I attach it, so I don't forget to do it at the end. I still send emails lacking attachments, but it happens less frequently. --------------------------------------------------------------------------- Alvaro Herrera wrote: > En Wed, 28 Aug 2002 03:59:48 -0400 > Alvaro Herrera <alvherre@atentus.com> escribi?: > > > En Tue, 27 Aug 2002 17:26:56 -0400 (EDT) > > Bruce Momjian <pgman@candle.pha.pa.us> escribi?: > > > > > > > > Patch withdrawn by author. > > > > Ok, new version. Please remember to change catversion. > > Doh, patch really attached this time. > > -- > Alvaro Herrera (<alvherre[a]atentus.com>) > "Cuando miro a alguien, mas me atrae como cambia que quien es" (J. Binoche) [ Attachment, skipping... ] > > ---------------------------(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, Pennsylvania 19073
[ Tom Lane will be reviewing this patch.]
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Alvaro Herrera wrote:
> En Tue, 27 Aug 2002 17:26:56 -0400 (EDT)
> Bruce Momjian <pgman@candle.pha.pa.us> escribi?:
>
> >
> > Patch withdrawn by author.
>
> Ok, new version. Please remember to change catversion.
>
> Description of this patch:
>
> - Adds a new attribute in pg_attribute named attisinherited.
> - Creation of tables marks it true for attributes that are inherited
> - Addition of new attribute to existing inherited table marks the
> attribute as inherited for child tables.
> - Checked when trying to rename inherited attributes: if table has
> inheritors, only allow renaming if asked to recurse. Disallow
> renaming for child tables only.
> - Checked when trying to drop inherited attributes: if table has
> inheritors, mark attribute as non-inherited for direct inheritors.
> Disallow dropping for child tables only.
>
> As an added bonus
> - Check inheritance when adding new attributes (if table has inheritors,
> only allow new attribute if it's inherited also).
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> "Acepta los honores y aplausos y perderas tu libertad"
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>
--
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, Pennsylvania 19073
Alvaro Herrera <alvherre@atentus.com> writes:
> - Adds a new attribute in pg_attribute named attisinherited.
> - Creation of tables marks it true for attributes that are inherited
> - Addition of new attribute to existing inherited table marks the
> attribute as inherited for child tables.
> - Checked when trying to rename inherited attributes: if table has
> inheritors, only allow renaming if asked to recurse. Disallow
> renaming for child tables only.
> - Checked when trying to drop inherited attributes: if table has
> inheritors, mark attribute as non-inherited for direct inheritors.
> Disallow dropping for child tables only.
I've applied this patch after a little editorializing. FYI ---
* copyfuncs.c,equalfuncs.c,outfuncs.c,readfuncs.c needed to be updated
for the field added to ColumnDef. In general, any time you alter the
definition of a Node structure, you gotta update these files.
* I didn't like having to touch all the callers of TupleDescInitEntry,
so I just made it initialize attisinherited to false. In the one
place where attisinherited might be set true, just update after return
from TupleDescInitEntry.
* Moved the checks for rename/drop ONLY with child tables into
tablecmds.c instead of utility.c, so that they'd be applied after
grabbing an exclusive lock on the table, not before. Otherwise a
child could be added after you look.
regards, tom lane
En Fri, 30 Aug 2002 15:29:45 -0400 Tom Lane <tgl@sss.pgh.pa.us> escribió: > Alvaro Herrera <alvherre@atentus.com> writes: > > - Adds a new attribute in pg_attribute named attisinherited. > > I've applied this patch after a little editorializing. FYI --- > > * copyfuncs.c,equalfuncs.c,outfuncs.c,readfuncs.c needed to be updated > for the field added to ColumnDef. In general, any time you alter the > definition of a Node structure, you gotta update these files. Ok, will make a note on that. > * I didn't like having to touch all the callers of TupleDescInitEntry, > so I just made it initialize attisinherited to false. In the one > place where attisinherited might be set true, just update after return > from TupleDescInitEntry. Yes, I had thought of doing that. It's much simpler and cleaner. > * Moved the checks for rename/drop ONLY with child tables into > tablecmds.c instead of utility.c, so that they'd be applied after > grabbing an exclusive lock on the table, not before. Otherwise a > child could be added after you look. Huh, that's where I had put them in the first place. I moved them to tcop without thinking about the locking issues. I'll be more careful on this also. Thank you, -- 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)