Re: [HACKERS] Typed table DDL loose ends
On Wed, 2011-04-20 at 10:44 -0400, Noah Misch wrote: If we add that ownership check, we'll protect some operations on the type. The cost is localized divergence from our principle that types have no usage restrictions. I'm of the opinion that it's not worth introducing that policy exception to block just some of these avenues of attack. I would not object to it, though. So that means we should leave it as is for now? Fine with me. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Wed, Apr 20, 2011 at 12:26:01AM +0300, Peter Eisentraut wrote: On Mon, 2011-04-18 at 19:34 -0400, Noah Misch wrote: On Mon, Apr 18, 2011 at 10:44:53PM +0300, Peter Eisentraut wrote: On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Users can CREATE TABLE OF on a type they don't own This in turns blocks the owner's ability to alter the table/type. However, we already have this hazard with composite-type columns. A TODO to address this broadly seems in order, but it's not a 9.1 issue. I think we should change that to mirror the inheritance policy: you have to be the owner of the parent. Note that this is newly relevant in 9.1, because you couldn't change composite types before. I pondered this angle further. The restriction is critical for inheritance, because attaching a new child table changes query behavior for the parent. Typed tables don't have that kind of action at a distance. Would we add that restriction to use of CREATE TABLE t (c comptype) as well, or just to CREATE TABLE t OF comptype? Well, that's a tough one. It would be a regression. I would say no to changing the former, because the fact that this prevents the type owner from changing the type is not a regression because you couldn't change types before at all. In 9.0, an unprivileged second party can use a composite-typed column to block DROP TYPE. If we change nothing, 9.1 will additionally permit blocking ALTER TYPE ALTER ATTRIBUTE, ALTER TYPE ADD ATTRIBUTE and ALTER TYPE DROP ATTRIBUTE. As you say, those are all new features. Adding an ownership check to CREATE TABLE OF would remove the last two vulnerabilities; a composite-typed column does not block them, but a typed table does block them. If we add that ownership check, we'll protect some operations on the type. The cost is localized divergence from our principle that types have no usage restrictions. I'm of the opinion that it's not worth introducing that policy exception to block just some of these avenues of attack. I would not object to it, though. Probably we need some privileges on types to address this properly. Agreed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 10:44:53PM +0300, Peter Eisentraut wrote: On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Inheriting from a typed table blocks further type DDL CREATE TYPE t AS (x int); CREATE TABLE parent OF t; CREATE TABLE child () INHERITS (parent); ALTER TYPE t ADD ATTRIBUTE y int CASCADE; -- ERROR: column must be added to child tables too We ought to just set INH_YES on the downstream command in ATTypedTableRecursion. If we get to that point, the user did choose ALTER TYPE CASCADE; it seems fair to assume he'd want inheritance recursion rather than a later error. Agreed. Patch attached for that. Apart from a comment, a test case and a doc update, it turned out to be a one-liner. diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index e889ffb..4bdf99f 100644 *** a/doc/src/sgml/ref/alter_type.sgml --- b/doc/src/sgml/ref/alter_type.sgml *** *** 122,128 ALTER TYPE replaceable class=PARAMETERname/replaceable ADD VALUE replacea listitem para Automatically propagate the operation to typed tables of the ! type being altered. /para /listitem /varlistentry --- 122,128 listitem para Automatically propagate the operation to typed tables of the ! type being altered and any descendants thereof. /para /listitem /varlistentry diff --git a/src/backend/commands/tableindex 1f709a4..9bab341 100644 *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *** *** 3858,3864 ATSimpleRecursion(List **wqueue, Relation rel, * ATTypedTableRecursion * * Propagate ALTER TYPE operations to the typed tables of that type. ! * Also check the RESTRICT/CASCADE behavior. */ static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, --- 3858,3865 * ATTypedTableRecursion * * Propagate ALTER TYPE operations to the typed tables of that type. ! * Also check the RESTRICT/CASCADE behavior. Given CASCADE, also permit ! * recursion to inheritance children of the typed tables. */ static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, *** *** 3880,3886 ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, childrel = relation_open(childrelid, lockmode); CheckTableNotInUse(childrel, ALTER TABLE); ! ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode); relation_close(childrel, NoLock); } } --- 3881,3887 childrel = relation_open(childrelid, lockmode); CheckTableNotInUse(childrel, ALTER TABLE); ! ATPrepCmd(wqueue, childrel, cmd, true, true, lockmode); relation_close(childrel, NoLock); } } diff --git a/src/test/regress/expected/index 5b1223b..6db648d 100644 *** a/src/test/regress/expected/alter_table.out --- b/src/test/regress/expected/alter_table.out *** *** 1845,1850 ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails --- 1845,1851 ERROR: cannot alter type test_type1 because column test_tbl1.y uses it CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; + CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2); \d test_type2 Composite type public.test_type2 Column | Type | Modifiers *** *** 1858,1863 Composite type public.test_type2 --- 1859,1865 +-+--- a | integer | b | text| + Number of child tables: 1 (Use \d+ to list them.) Typed table of type: test_type2 ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails *** *** 1879,1884 Composite type public.test_type2 --- 1881,1887 a | integer | b | text| c | text| + Number of child tables: 1 (Use \d+ to list them.) Typed table of type: test_type2 ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails *** *** 1900,1905 ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar CASCADE; --- 1903,1909 a | integer | b | character varying | c | text | + Number of child tables: 1 (Use \d+ to list them.) Typed table of type: test_type2 ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails *** *** 1919,1924 Composite type public.test_type2 --- 1923,1929 +-+--- a | integer | c | text| + Number of child tables: 1 (Use \d+ to list them.) Typed table of type: test_type2 ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa; -- fails *** *** 1938,1944 Composite type public.test_type2 --- 1943,1958 +-+--- aa | integer | c | text| + Number of child tables: 1 (Use \d+ to list
Re: [HACKERS] Typed table DDL loose ends
On Wed, Apr 20, 2011 at 10:53 AM, Noah Misch n...@leadboat.com wrote: On Mon, Apr 18, 2011 at 10:44:53PM +0300, Peter Eisentraut wrote: On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Inheriting from a typed table blocks further type DDL CREATE TYPE t AS (x int); CREATE TABLE parent OF t; CREATE TABLE child () INHERITS (parent); ALTER TYPE t ADD ATTRIBUTE y int CASCADE; -- ERROR: column must be added to child tables too We ought to just set INH_YES on the downstream command in ATTypedTableRecursion. If we get to that point, the user did choose ALTER TYPE CASCADE; it seems fair to assume he'd want inheritance recursion rather than a later error. Agreed. Patch attached for that. Apart from a comment, a test case and a doc update, it turned out to be a one-liner. I have committed this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, 2011-04-18 at 19:34 -0400, Noah Misch wrote: On Mon, Apr 18, 2011 at 10:44:53PM +0300, Peter Eisentraut wrote: On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Users can CREATE TABLE OF on a type they don't own This in turns blocks the owner's ability to alter the table/type. However, we already have this hazard with composite-type columns. A TODO to address this broadly seems in order, but it's not a 9.1 issue. I think we should change that to mirror the inheritance policy: you have to be the owner of the parent. Note that this is newly relevant in 9.1, because you couldn't change composite types before. Would we add that restriction to use of CREATE TABLE t (c comptype) as well, or just to CREATE TABLE t OF comptype? Well, that's a tough one. It would be a regression. I would say no to changing the former, because the fact that this prevents the type owner from changing the type is not a regression because you couldn't change types before at all. Probably we need some privileges on types to address this properly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Thu, Apr 14, 2011 at 9:38 PM, Noah Misch n...@leadboat.com wrote: On Thu, Apr 14, 2011 at 11:23:49AM -0700, Robert Haas wrote: On Thu, Apr 14, 2011 at 5:18 AM, Noah Misch n...@leadboat.com wrote: I guess my gut feeling is that it would make more sense to forbid it outright for 9.1, and we can look at relaxing that restriction later if we're so inclined. Much as with the problem Tom fixed in commit eb51af71f241e8cb199790dee9ad246bb36b3287, I'm concerned that there may be other cases that we're not thinking of right now, and while we could find them all and fix them, the amount of functionality gained is fairly marginal, and I don't really want to hold up the release while we bug-swat. Symmetry was the best cause I could find to continue allowing it, and your case in favor of reducing the bug surface is more compelling. ?Let's forbid it. OK. Care to propose a patch? Sure; attached. It requires that the type relation be RELKIND_COMPOSITE_TYPE. We hadn't explicitly discussed the use of foreign table, view, toast table, or sequence row types. The first two might have some value, someday; I'm sure nobody cares for the second two. It appears that this patch - which I've now committed - actually fixes two bugs. Without the patch, this leaves the tables out of step, because no relation lock is taken: S1: CREATE TYPE t AS (a int); S1: BEGIN; S1: CREATE TABLE t1 OF t; S2: ALTER TYPE t ADD ATTRIBUTE b int CASCADE; S1: COMMIT; I tweaked the comments accordingly, and also reverted your change to the error message, because I don't want to introduce new terminology here that we're not using anywhere else. I also thought about making this use parserOpenTable() rather than relation_open(), but it turns out that's not so easy, because the typed table name is getting shoved into a TypeName rather than a RangeVar. I don't think that's the choice I would have made but I'm not sure how excited it's worth getting about it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 10:20:21AM -0400, Robert Haas wrote: I tweaked the comments accordingly, and also reverted your change to the error message, because I don't want to introduce new terminology here that we're not using anywhere else. FWIW, the term stand-alone composite type appears twice in our documentation. I also thought about making this use parserOpenTable() rather than relation_open(), but it turns out that's not so easy, because the typed table name is getting shoved into a TypeName rather than a RangeVar. I don't think that's the choice I would have made but I'm not sure how excited it's worth getting about it. I hadn't thought about this angle. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 10:48 AM, Noah Misch n...@leadboat.com wrote: On Mon, Apr 18, 2011 at 10:20:21AM -0400, Robert Haas wrote: I tweaked the comments accordingly, and also reverted your change to the error message, because I don't want to introduce new terminology here that we're not using anywhere else. FWIW, the term stand-alone composite type appears twice in our documentation. Hmm, OK. Anyone else have an opinion on the relative merits of: ERROR: type stuff is not a composite type vs. ERROR: type stuff is not a stand-alone composite type The intent of adding stand-alone was, I believe, to clarify that it has to be a CREATE TYPE stuff AS ... type, not just a row type (that is, naturally, composite, in some less-pure sense). I'm not sure whether the extra word actually makes it more clear, though. Opinions? Suggestions? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
Robert Haas robertmh...@gmail.com writes: On Mon, Apr 18, 2011 at 10:48 AM, Noah Misch n...@leadboat.com wrote: FWIW, the term stand-alone composite type appears twice in our documentation. Hmm, OK. Anyone else have an opinion on the relative merits of: ERROR: type stuff is not a composite type vs. ERROR: type stuff is not a stand-alone composite type The intent of adding stand-alone was, I believe, to clarify that it has to be a CREATE TYPE stuff AS ... type, not just a row type (that is, naturally, composite, in some less-pure sense). I'm not sure whether the extra word actually makes it more clear, though. In 99.9% of the code and docs, a table rowtype is a perfectly good composite type. I agree with Noah that just saying composite type is inadequate here; but I'm not sure that stand-alone is a helpful adjective either. What about inverting the message phrasing, ie ERROR: type stuff must not be a table's row type You might need some extra logic to keep on giving is not a composite type in cases where it's not composite at all. But this is enough of a departure from our usual behavior that I think the error message had better be pretty darn clear. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 11:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Apr 18, 2011 at 10:48 AM, Noah Misch n...@leadboat.com wrote: FWIW, the term stand-alone composite type appears twice in our documentation. Hmm, OK. Anyone else have an opinion on the relative merits of: ERROR: type stuff is not a composite type vs. ERROR: type stuff is not a stand-alone composite type The intent of adding stand-alone was, I believe, to clarify that it has to be a CREATE TYPE stuff AS ... type, not just a row type (that is, naturally, composite, in some less-pure sense). I'm not sure whether the extra word actually makes it more clear, though. In 99.9% of the code and docs, a table rowtype is a perfectly good composite type. I agree with Noah that just saying composite type is inadequate here; but I'm not sure that stand-alone is a helpful adjective either. What about inverting the message phrasing, ie ERROR: type stuff must not be a table's row type It also can't be a view's row type, a sequence's row type, a foreign table's row type... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
Robert Haas robertmh...@gmail.com writes: On Mon, Apr 18, 2011 at 11:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: What about inverting the message phrasing, ie ERROR: type stuff must not be a table's row type It also can't be a view's row type, a sequence's row type, a foreign table's row type... Well, you could say relation's row type if you wanted to be formally correct, but I'm not convinced that's an improvement. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 11:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Apr 18, 2011 at 11:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: What about inverting the message phrasing, ie ERROR: type stuff must not be a table's row type It also can't be a view's row type, a sequence's row type, a foreign table's row type... Well, you could say relation's row type if you wanted to be formally correct, but I'm not convinced that's an improvement. Me neither, especially since composite types are also relations, in our parlance. I'm not strongly attached to or repulsed by any particular option, so however we end up doing it is OK with me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Table row types used in typed tables vs. ALTER TABLE This item was addressed, but the other ones were not, I think. * Inheriting from a typed table blocks further type DDL CREATE TYPE t AS (x int); CREATE TABLE parent OF t; CREATE TABLE child () INHERITS (parent); ALTER TYPE t ADD ATTRIBUTE y int CASCADE; -- ERROR: column must be added to child tables too We ought to just set INH_YES on the downstream command in ATTypedTableRecursion. If we get to that point, the user did choose ALTER TYPE CASCADE; it seems fair to assume he'd want inheritance recursion rather than a later error. Agreed. * Users can CREATE TABLE OF on a type they don't own This in turns blocks the owner's ability to alter the table/type. However, we already have this hazard with composite-type columns. A TODO to address this broadly seems in order, but it's not a 9.1 issue. I think we should change that to mirror the inheritance policy: you have to be the owner of the parent. Note that this is newly relevant in 9.1, because you couldn't change composite types before. * Can create a permanent table using a temp table row type CREATE TEMP TABLE tempt (); CREATE TABLE permt OF tempt; -- silently dropped on session exit Looks easy to fix, with no policy questions. This is now prohibited. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Mon, Apr 18, 2011 at 10:44:53PM +0300, Peter Eisentraut wrote: On Sat, 2011-04-09 at 21:57 -0400, Noah Misch wrote: * Users can CREATE TABLE OF on a type they don't own This in turns blocks the owner's ability to alter the table/type. However, we already have this hazard with composite-type columns. A TODO to address this broadly seems in order, but it's not a 9.1 issue. I think we should change that to mirror the inheritance policy: you have to be the owner of the parent. Note that this is newly relevant in 9.1, because you couldn't change composite types before. Would we add that restriction to use of CREATE TABLE t (c comptype) as well, or just to CREATE TABLE t OF comptype? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Wed, Apr 13, 2011 at 07:57:29PM -0700, Robert Haas wrote: On Sat, Apr 9, 2011 at 6:57 PM, Noah Misch n...@leadboat.com wrote: While looking at the typed table/pg_upgrade problem, I ran into a few smaller problems in the area. ?I'm not envisioning a need for much code shift to fix them, but there are a few points of policy. * Table row types used in typed tables vs. ALTER TABLE As previously noted: ?CREATE TABLE t (); ?CREATE TABLE is_a OF t; ?ALTER TABLE t ADD c int; ?\d is_a ?-- No columns At first I thought we should just forbid the use of table row types in CREATE TABLE OF. ?However, we've been quite systematic about not distinguishing between table row types and CREATE TYPE AS types; I've only found a distinction in ALTER TABLE/ALTER TYPE, where we direct you to the other command. ?It would be nice to preserve this heritage. ?That doesn't look particularly difficult; it may actually yield a net code reduction. I guess my gut feeling is that it would make more sense to forbid it outright for 9.1, and we can look at relaxing that restriction later if we're so inclined. Much as with the problem Tom fixed in commit eb51af71f241e8cb199790dee9ad246bb36b3287, I'm concerned that there may be other cases that we're not thinking of right now, and while we could find them all and fix them, the amount of functionality gained is fairly marginal, and I don't really want to hold up the release while we bug-swat. Symmetry was the best cause I could find to continue allowing it, and your case in favor of reducing the bug surface is more compelling. Let's forbid it. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Thu, Apr 14, 2011 at 5:18 AM, Noah Misch n...@leadboat.com wrote: I guess my gut feeling is that it would make more sense to forbid it outright for 9.1, and we can look at relaxing that restriction later if we're so inclined. Much as with the problem Tom fixed in commit eb51af71f241e8cb199790dee9ad246bb36b3287, I'm concerned that there may be other cases that we're not thinking of right now, and while we could find them all and fix them, the amount of functionality gained is fairly marginal, and I don't really want to hold up the release while we bug-swat. Symmetry was the best cause I could find to continue allowing it, and your case in favor of reducing the bug surface is more compelling. Let's forbid it. OK. Care to propose a patch? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Thu, Apr 14, 2011 at 11:23:49AM -0700, Robert Haas wrote:
On Thu, Apr 14, 2011 at 5:18 AM, Noah Misch n...@leadboat.com wrote:
I guess my gut feeling is that it would make more sense to forbid it
outright for 9.1, and we can look at relaxing that restriction later
if we're so inclined.
Much as with the problem Tom fixed in commit
eb51af71f241e8cb199790dee9ad246bb36b3287, I'm concerned that there may
be other cases that we're not thinking of right now, and while we
could find them all and fix them, the amount of functionality gained
is fairly marginal, and I don't really want to hold up the release
while we bug-swat.
Symmetry was the best cause I could find to continue allowing it, and your
case
in favor of reducing the bug surface is more compelling. ?Let's forbid it.
OK. Care to propose a patch?
Sure; attached. It requires that the type relation be RELKIND_COMPOSITE_TYPE.
We hadn't explicitly discussed the use of foreign table, view, toast table, or
sequence row types. The first two might have some value, someday; I'm sure
nobody cares for the second two.
nm
diff --git a/src/backend/parser/parse_utilcmd.c
b/src/backend/parser/parse_utilcmd.c
index eba890b..31b1fb0 100644
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***
*** 825,830 transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
--- 825,831
TupleDesc tupdesc;
int i;
Oid ofTypeId;
+ booltypeOk = false;
AssertArg(ofTypename);
***
*** 833,842 transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
ofTypeId = HeapTupleGetOid(tuple);
ofTypename-typeOid = ofTypeId; /* cached for later */
! if (typ-typtype != TYPTYPE_COMPOSITE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
!errmsg(type %s is not a composite type,
format_type_be(ofTypeId;
tupdesc = lookup_rowtype_tupdesc(ofTypeId, -1);
--- 834,852
ofTypeId = HeapTupleGetOid(tuple);
ofTypename-typeOid = ofTypeId; /* cached for later */
! if (typ-typtype == TYPTYPE_COMPOSITE)
! {
! RelationtypeRelation;
!
! Assert(OidIsValid(typ-typrelid));
! typeRelation = relation_open(typ-typrelid, AccessShareLock);
! typeOk = (typeRelation-rd_rel-relkind ==
RELKIND_COMPOSITE_TYPE);
! relation_close(typeRelation, NoLock);
! }
! if (!typeOk)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
!errmsg(type %s is not a stand-alone composite
type,
format_type_be(ofTypeId;
tupdesc = lookup_rowtype_tupdesc(ofTypeId, -1);
diff --git a/src/test/regress/expected/tyindex 0874a64..69ad58e 100644
*** a/src/test/regress/expected/typed_table.out
--- b/src/test/regress/expected/typed_table.out
***
*** 91,96 DETAIL: drop cascades to table persons
--- 91,98
drop cascades to function get_all_persons()
drop cascades to table persons2
drop cascades to table persons3
+ CREATE TABLE persons5 OF stuff; -- only CREATE TYPE AS types may be used
+ ERROR: type stuff is not a stand-alone composite type
DROP TABLE stuff;
-- implicit casting
CREATE TYPE person_type AS (id int, name text);
diff --git a/src/test/regress/sql/typed_table.sqindex b0d452c..25aaccb 100644
*** a/src/test/regress/sql/typed_table.sql
--- b/src/test/regress/sql/typed_table.sql
***
*** 46,51 CREATE TABLE persons4 OF person_type (
--- 46,53
DROP TYPE person_type RESTRICT;
DROP TYPE person_type CASCADE;
+ CREATE TABLE persons5 OF stuff; -- only CREATE TYPE AS types may be used
+
DROP TABLE stuff;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typed table DDL loose ends
On Sat, Apr 9, 2011 at 6:57 PM, Noah Misch n...@leadboat.com wrote: While looking at the typed table/pg_upgrade problem, I ran into a few smaller problems in the area. I'm not envisioning a need for much code shift to fix them, but there are a few points of policy. * Table row types used in typed tables vs. ALTER TABLE As previously noted: CREATE TABLE t (); CREATE TABLE is_a OF t; ALTER TABLE t ADD c int; \d is_a -- No columns At first I thought we should just forbid the use of table row types in CREATE TABLE OF. However, we've been quite systematic about not distinguishing between table row types and CREATE TYPE AS types; I've only found a distinction in ALTER TABLE/ALTER TYPE, where we direct you to the other command. It would be nice to preserve this heritage. That doesn't look particularly difficult; it may actually yield a net code reduction. I guess my gut feeling is that it would make more sense to forbid it outright for 9.1, and we can look at relaxing that restriction later if we're so inclined. Much as with the problem Tom fixed in commit eb51af71f241e8cb199790dee9ad246bb36b3287, I'm concerned that there may be other cases that we're not thinking of right now, and while we could find them all and fix them, the amount of functionality gained is fairly marginal, and I don't really want to hold up the release while we bug-swat. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Typed table DDL loose ends
While looking at the typed table/pg_upgrade problem, I ran into a few smaller problems in the area. I'm not envisioning a need for much code shift to fix them, but there are a few points of policy. * Table row types used in typed tables vs. ALTER TABLE As previously noted: CREATE TABLE t (); CREATE TABLE is_a OF t; ALTER TABLE t ADD c int; \d is_a -- No columns At first I thought we should just forbid the use of table row types in CREATE TABLE OF. However, we've been quite systematic about not distinguishing between table row types and CREATE TYPE AS types; I've only found a distinction in ALTER TABLE/ALTER TYPE, where we direct you to the other command. It would be nice to preserve this heritage. That doesn't look particularly difficult; it may actually yield a net code reduction. There is a minor policy question: when should ALTER TABLE behave like ALTER TYPE ... RESTRICT, if ever? Would using the inheritance recursion decision (driven by ONLY, *, and sql_inheritance) be sufficiently reasonable, or do we need a distinct signal? I can't envision a case where you'd want to recurse to inheritance children but error on typed table children (YMMV). ALTER TABLE DROP COLUMN currently uses RESTRICT/CASCADE in a completely different sense, so any syntactic signal would need to step around that. * Inheriting from a typed table blocks further type DDL CREATE TYPE t AS (x int); CREATE TABLE parent OF t; CREATE TABLE child () INHERITS (parent); ALTER TYPE t ADD ATTRIBUTE y int CASCADE; -- ERROR: column must be added to child tables too We ought to just set INH_YES on the downstream command in ATTypedTableRecursion. If we get to that point, the user did choose ALTER TYPE CASCADE; it seems fair to assume he'd want inheritance recursion rather than a later error. * Users can CREATE TABLE OF on a type they don't own This in turns blocks the owner's ability to alter the table/type. However, we already have this hazard with composite-type columns. A TODO to address this broadly seems in order, but it's not a 9.1 issue. * Can create a permanent table using a temp table row type CREATE TEMP TABLE tempt (); CREATE TABLE permt OF tempt; -- silently dropped on session exit Looks easy to fix, with no policy questions. Does any of this appear incorrect or unreasonable? Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
