pg_dump --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE
I took a look at the open item concerning typed tables and pg_upgrade:
http://archives.postgresql.org/pgsql-hackers/2011-03/msg00767.php
"pg_dump --binary-upgrade" emits commands to recreate the dropped-column
situation of the database, and it does not currently understand that it must
alter the parent type when the subject is a typed table. Actually, pg_dump
doesn't handle dropped columns in composite types at all. pg_upgrade runs fine
on a database that received these commands, but the outcome is wrong:
create type t as (x int, y int);
create table has_a (tcol t);
insert into has_a values ('(1,2)');
table has_a; -- (1,2)
alter type t drop attribute y cascade, add attribute z int cascade;
table has_a; -- (1,)
table has_a; -- after pg_upgrade: (1,2)
Fixing that looks clear enough, but the right fix for the typed table issue is
less clear to me. The pg_attribute tuples for a typed table will include any
attributes dropped from the parent type after the table's creation, but not
those attributes dropped before the table's creation. Example:
create type t as (x int, y int);
create table is_a of t;
alter type t drop attribute y cascade;
create table is_a2 of t;
select * from pg_attribute where attrelid = 'is_a'::regclass;
select * from pg_attribute where attrelid = 'is_a2'::regclass;
To reproduce that catalog state, the dump would need to create the type, create
all typed tables predating the DROP ATTRIBUTE, and finally create typed tables
postdating the DROP ATTRIBUTE. That implies an extra dump entry for the DROP
ATTRIBUTE with the appropriate dependencies to compel that order of events. Is
there a better way?
nm
On Tue, Mar 29, 2011 at 5:50 PM, Noah Misch <noah@leadboat.com> wrote:
I took a look at the open item concerning typed tables and pg_upgrade:
http://archives.postgresql.org/pgsql-hackers/2011-03/msg00767.php
Thanks!
[ helpful summary of problem clipped ]
To reproduce that catalog state, the dump would need to create the type, create
all typed tables predating the DROP ATTRIBUTE, and finally create typed tables
postdating the DROP ATTRIBUTE. That implies an extra dump entry for the DROP
ATTRIBUTE with the appropriate dependencies to compel that order of events. Is
there a better way?
I think so. We have this same problem with regular table inheritance,
and the way we fix it is to jigger the tuple descriptor for the child
table so that it matches what we need, and THEN attach it to the
parent:
CREATE TABLE child (
a integer,
"........pg.dropped.2........" INTEGER /* dummy */
);
-- For binary upgrade, recreate inherited column.
UPDATE pg_catalog.pg_attribute
SET attislocal = false
WHERE attname = 'a'
AND attrelid = 'child'::pg_catalog.regclass;
-- For binary upgrade, recreate dropped column.
UPDATE pg_catalog.pg_attribute
SET attlen = 4, attalign = 'i', attbyval = false
WHERE attname = '........pg.dropped.2........'
AND attrelid = 'child'::pg_catalog.regclass;
ALTER TABLE ONLY child DROP COLUMN "........pg.dropped.2........";
-- For binary upgrade, set up inheritance this way.
ALTER TABLE ONLY child INHERIT parent;
I think we need to do something similar here -- use the same hack
shown above to get the dropped column into the right state, and then
jigger things so that the child is a typed table associated with the
parent. Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
INHERIT to accept a type name as the final argument. If used in this
way, it converts a typed table into a regular table or visca versa.
We could also do it with a direct catalog change, but there are some
dependencies that would need to be frobbed, which makes me a bit
reluctant to go that way.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On tis, 2011-03-29 at 17:50 -0400, Noah Misch wrote:
Fixing that looks clear enough, but the right fix for the typed table
issue is less clear to me. The pg_attribute tuples for a typed table
will include any attributes dropped from the parent type after the
table's creation, but not those attributes dropped before the table's
creation. Example:create type t as (x int, y int);
create table is_a of t;
alter type t drop attribute y cascade;
create table is_a2 of t;
select * from pg_attribute where attrelid = 'is_a'::regclass;
select * from pg_attribute where attrelid = 'is_a2'::regclass;To reproduce that catalog state, the dump would need to create the
type, create all typed tables predating the DROP ATTRIBUTE, and
finally create typed tables postdating the DROP ATTRIBUTE. That
implies an extra dump entry for the DROP ATTRIBUTE with the
appropriate dependencies to compel that order of events. Is
there a better way?
Maybe we could just copy the dropped attributes from the type when the
table is created. That might be as simple as removing the
if (attr->attisdropped)
continue;
in transformOfType().
On Wed, Mar 30, 2011 at 12:55 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Maybe we could just copy the dropped attributes from the type when the
table is created. That might be as simple as removing theif (attr->attisdropped)
continue;in transformOfType().
Seems a bit fragile...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Mar 30, 2011 at 10:14:03AM -0400, Robert Haas wrote:
On Tue, Mar 29, 2011 at 5:50 PM, Noah Misch <noah@leadboat.com> wrote:
To reproduce that catalog state, the dump would need to create the type, create
all typed tables predating the DROP ATTRIBUTE, and finally create typed tables
postdating the DROP ATTRIBUTE. ?That implies an extra dump entry for the DROP
ATTRIBUTE with the appropriate dependencies to compel that order of events. ?Is
there a better way?I think so. We have this same problem with regular table inheritance,
and the way we fix it is to jigger the tuple descriptor for the child
table so that it matches what we need, and THEN attach it to the
parent:
<snipped example>
I think we need to do something similar here -- use the same hack
shown above to get the dropped column into the right state, and then
jigger things so that the child is a typed table associated with the
parent.
Good idea; I agree.
Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
INHERIT to accept a type name as the final argument. If used in this
way, it converts a typed table into a regular table or visca versa.
Why extend ALTER TABLE ... INHERIT? I would have guessed independent syntax.
We could also do it with a direct catalog change, but there are some
dependencies that would need to be frobbed, which makes me a bit
reluctant to go that way.
Agreed; it's also an independently-useful capability to have.
On Wed, Mar 30, 2011 at 9:30 PM, Noah Misch <noah@leadboat.com> wrote:
Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
INHERIT to accept a type name as the final argument. If used in this
way, it converts a typed table into a regular table or visca versa.Why extend ALTER TABLE ... INHERIT? I would have guessed independent syntax.
I just didn't feel the need to invent something new, but we could if
someone would rather.
We could also do it with a direct catalog change, but there are some
dependencies that would need to be frobbed, which makes me a bit
reluctant to go that way.Agreed; it's also an independently-useful capability to have.
Yep.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Mar 30, 2011 at 09:37:56PM -0400, Robert Haas wrote:
On Wed, Mar 30, 2011 at 9:30 PM, Noah Misch <noah@leadboat.com> wrote:
Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
INHERIT to accept a type name as the final argument. ?If used in this
way, it converts a typed table into a regular table or visca versa.Why extend ALTER TABLE ... INHERIT? ?I would have guessed independent syntax.
I just didn't feel the need to invent something new, but we could if
someone would rather.We could also do it with a direct catalog change, but there are some
dependencies that would need to be frobbed, which makes me a bit
reluctant to go that way.Agreed; it's also an independently-useful capability to have.
Yep.
Implemented as attached. The first patch just adds the ALTER TABLE subcommands
to attach and detach a table from a composite type. A few open questions
concerning typed tables will probably yield minor changes to these subcommands.
I implemented them to be agnostic toward the outcome of those decisions.
The second patch updates pg_dump to use those new subcommands. It's based
significantly on Peter's recent patch. The new bits follow pg_dump's design for
table inheritance.
I tested pg_upgrade of these previously-mentioned test cases:
create type t as (x int, y int);
create table has_a (tcol t);
insert into has_a values ('(1,2)');
table has_a; -- (1,2)
alter type t drop attribute y cascade, add attribute z int cascade;
table has_a; -- (1,)
table has_a; -- after pg_upgrade: (1,2)
create type t as (x int, y int);
create table is_a of t;
alter type t drop attribute y cascade;
create table is_a2 of t;
select * from pg_attribute where attrelid = 'is_a'::regclass;
select * from pg_attribute where attrelid = 'is_a2'::regclass;
create type unused as (x int);
alter type unused drop attribute x;
I also tested a regular dump+reload of the regression database, and a pg_upgrade
of the same. The latter failed further along, due (indirectly) to this failure
to create a TOAST table:
create table p ();
create table ch () inherits (p);
alter table p add column a text;
select oid::regclass,reltoastrelid from pg_class where oid::regclass IN ('p','ch');
insert into ch values (repeat('x', 1000000));
If I "drop table a_star cascade" in the regression database before attempting
pg_upgrade, it completes cleanly.
nm
Attachments:
tt1v1-alter-of.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index c194862..4e02438 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 63,68 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 63,70 ----
RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
+ OF <replaceable class="PARAMETER">type_name</replaceable>
+ NOT OF
OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
***************
*** 491,496 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 493,522 ----
</varlistentry>
<varlistentry>
+ <term><literal>OF <replaceable class="PARAMETER">type_name</replaceable></literal></term>
+ <listitem>
+ <para>
+ This form links the table to a composite type as though <command>CREATE
+ TABLE OF</> had formed it. The table's list of column names and types
+ must precisely match that of the composite type; the presence of
+ an <literal>oid</> system column is permitted to differ. The table must
+ not inherit from any other table. These restrictions ensure
+ that <command>CREATE TABLE OF</> would permit an equivalent table
+ definition.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>NOT OF</literal></term>
+ <listitem>
+ <para>
+ This form dissociates a typed table from its type.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>OWNER</literal></term>
<listitem>
<para>
diff --git a/src/backend/commands/tablecindex bd18db3..0d657a3 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 81,86 ****
--- 81,87 ----
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
+ #include "utils/typcache.h"
/*
***************
*** 357,362 **** static void ATExecEnableDisableRule(Relation rel, char *rulename,
--- 358,366 ----
static void ATPrepAddInherit(Relation child_rel);
static void ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static void ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
+ static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid);
+ static void ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode);
+ static void ATExecDropOf(Relation rel, LOCKMODE lockmode);
static void ATExecGenericOptions(Relation rel, List *options);
static void copy_relation_data(SMgrRelation rel, SMgrRelation dst,
***************
*** 2679,2684 **** AlterTableGetLockLevel(List *cmds)
--- 2683,2698 ----
break;
/*
+ * These subcommands affect implicit row type conversion. They have
+ * affects similar to CREATE/DROP CAST on queries. We don't provide
+ * for invalidating parse trees as a result of such changes. Do
+ * avoid concurrent pg_class updates, though.
+ */
+ case AT_AddOf:
+ case AT_DropOf:
+ cmd_lockmode = ShareUpdateExclusiveLock;
+
+ /*
* These subcommands affect general strategies for performance and maintenance,
* though don't change the semantic results from normal data reads and writes.
* Delaying an ALTER TABLE behind currently active writes only delays the point
***************
*** 2935,2947 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
- ATSimplePermissions(rel, ATT_TABLE);
- /* These commands never recurse */
- /* No command-specific prep needed */
- pass = AT_PASS_MISC;
- break;
case AT_DropInherit: /* NO INHERIT */
ATSimplePermissions(rel, ATT_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
--- 2949,2959 ----
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
case AT_DropInherit: /* NO INHERIT */
+ case AT_AddOf: /* OF */
+ case AT_DropOf: /* NOT OF */
ATSimplePermissions(rel, ATT_TABLE);
+ /* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
***************
*** 3210,3215 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 3222,3233 ----
case AT_DropInherit:
ATExecDropInherit(rel, (RangeVar *) cmd->def, lockmode);
break;
+ case AT_AddOf:
+ ATExecAddOf(rel, (TypeName *) cmd->def, lockmode);
+ break;
+ case AT_DropOf:
+ ATExecDropOf(rel, lockmode);
+ break;
case AT_GenericOptions:
ATExecGenericOptions(rel, (List *) cmd->def);
break;
***************
*** 8333,8340 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple,
! depTuple;
List *connames;
bool found = false;
--- 8351,8357 ----
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple;
List *connames;
bool found = false;
***************
*** 8500,8510 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /*
! * Drop the dependency
! *
! * There's no convenient way to do this, so go trawling through pg_depend
! */
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
--- 8517,8545 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! drop_parent_dependency(RelationGetRelid(rel),
! RelationRelationId,
! RelationGetRelid(parent_rel));
!
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
! }
!
! /*
! * Drop the dependency created by StoreCatalogInheritance1 (CREATE TABLE
! * INHERITS/ALTER TABLE INHERIT -- refclassid will be RelationRelationId) or
! * heap_create_with_catalog (CREATE TABLE OF/ALTER TABLE OF -- refclassid will
! * be TypeRelationId). There's no convenient way to do this, so go trawling
! * through pg_depend.
! */
! static void
! drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid)
! {
! Relation catalogRelation;
! SysScanDesc scan;
! ScanKeyData key[3];
! HeapTuple depTuple;
!
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
***************
*** 8514,8520 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(RelationGetRelid(rel)));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
--- 8549,8555 ----
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
***************
*** 8527,8534 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == RelationRelationId &&
! dep->refobjid == RelationGetRelid(parent_rel) &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
--- 8562,8569 ----
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == refclassid &&
! dep->refobjid == refobjid &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
***************
*** 8536,8544 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
}
/*
--- 8571,8794 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
+ }
! /*
! * ALTER TABLE OF
! *
! * Attach a table to a composite type, as though it had been created with CREATE
! * TABLE OF. All attname, atttypid, atttypmod and attcollation must match. The
! * subject table must not have inheritance parents. These restrictions ensure
! * that you cannot create a configuration impossible with CREATE TABLE OF alone.
! */
! static void
! ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel),
! cycle_relid;
! Type typetuple;
! Form_pg_type typ;
! Oid typeid;
! Relation inheritsRelation,
! relationRelation;
! SysScanDesc scan;
! ScanKeyData key;
! AttrNumber table_attno,
! type_attno;
! TupleDesc typeTupleDesc,
! tableTupleDesc;
! ObjectAddress tableobj,
! typeobj;
! HeapTuple classtuple;
!
! /* Validate the type. */
! typetuple = typenameType(NULL, ofTypename, NULL);
! typ = (Form_pg_type) GETSTRUCT(typetuple);
! typeid = HeapTupleGetOid(typetuple);
!
! if (typ->typtype != TYPTYPE_COMPOSITE)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("type %s is not a composite type",
! format_type_be(typeid))));
!
! /* Fail if the table has any inheritance parents. */
! inheritsRelation = heap_open(InheritsRelationId, RowExclusiveLock);
! ScanKeyInit(&key,
! Anum_pg_inherits_inhrelid,
! BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
! scan = systable_beginscan(inheritsRelation, InheritsRelidSeqnoIndexId,
! true, SnapshotNow, 1, &key);
! if (HeapTupleIsValid(systable_getnext(scan)))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("typed tables cannot inherit")));
! systable_endscan(scan);
! heap_close(inheritsRelation, RowExclusiveLock);
!
! /*
! * Forbid reloftype cycles. This is subject to the same race condition as
! * the comparable code in ATExecAddInherit(); see discussion there.
! *
! * As a side effect, open and lock the type relations all the way up the
! * chain, keeping those locks until end of transaction. This prevents a
! * concurrent column change from invalidating our later analysis. We don't
! * normally take this sort of precaution for row types. Since we're opening
! * them anyway, we may as well do so.
! */
! cycle_relid = typ->typrelid;
! for (;;)
! {
! Relation typeRelation;
! Oid cycle_typeid;
! HeapTuple tup;
!
! /* Get the next reloftype from the current reloftype's relation. */
! typeRelation = relation_open(cycle_relid, AccessShareLock);
! cycle_typeid = typeRelation->rd_rel->reloftype;
! relation_close(typeRelation, NoLock);
!
! /* Might be finished. */
! if (!OidIsValid(cycle_typeid)) /* end of the chain */
! break;
! if (cycle_typeid == rel->rd_rel->reltype) /* cycle detected */
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_TABLE),
! errmsg("circular typed table relationship not allowed")));
!
! /* Continue up the chain: find the next typrelid. */
! tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(cycle_typeid));
! if (!HeapTupleIsValid(tup)) /* should not happen */
! elog(ERROR, "cache lookup failed for type %u", cycle_typeid);
! cycle_relid = ((Form_pg_type) GETSTRUCT(tup))->typrelid;
! ReleaseSysCache(tup);
! }
!
! /*
! * Check the tuple descriptors for compatibility. Unlike inheritance, we
! * require that the order also match. However, attnotnull need not match.
! * Also unlike inheritance, we do not require matching relhasoids.
! */
! typeTupleDesc = lookup_rowtype_tupdesc(typeid, -1);
! tableTupleDesc = RelationGetDescr(rel);
! table_attno = 1;
! for (type_attno = 1; type_attno <= typeTupleDesc->natts; type_attno++)
! {
! Form_pg_attribute type_attr,
! table_attr;
! const char *type_attname,
! *table_attname;
!
! /* Get the next non-dropped type attribute. */
! type_attr = typeTupleDesc->attrs[type_attno - 1];
! if (type_attr->attisdropped)
! continue;
! type_attname = NameStr(type_attr->attname);
!
! /* Get the next non-dropped table attribute. */
! do
! {
! if (table_attno > tableTupleDesc->natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table is missing column \"%s\"",
! type_attname)));
! table_attr = tableTupleDesc->attrs[table_attno++ - 1];
! } while (table_attr->attisdropped);
! table_attname = NameStr(table_attr->attname);
!
! /* Compare name. */
! if (strncmp(table_attname, type_attname, NAMEDATALEN) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has column \"%s\" where type requires \"%s\"",
! table_attname, type_attname)));
!
! /* Compare type. */
! if (table_attr->atttypid != type_attr->atttypid ||
! table_attr->atttypmod != type_attr->atttypmod ||
! table_attr->attcollation != type_attr->attcollation)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table \"%s\" has different type for column \"%s\"",
! RelationGetRelationName(rel), type_attname)));
! }
! DecrTupleDescRefCount(typeTupleDesc);
!
! /* Any remaining columns at the end of the table had better be dropped. */
! for (; table_attno <= tableTupleDesc->natts; table_attno++)
! {
! Form_pg_attribute table_attr = tableTupleDesc->attrs[table_attno - 1];
! if (!table_attr->attisdropped)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has extra column \"%s\"",
! NameStr(table_attr->attname))));
! }
!
! /* If the table was already typed, drop the existing dependency. */
! if (rel->rd_rel->reloftype)
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Record a dependency on the new type. */
! tableobj.classId = RelationRelationId;
! tableobj.objectId = relid;
! tableobj.objectSubId = 0;
! typeobj.classId = TypeRelationId;
! typeobj.objectId = typeid;
! typeobj.objectSubId = 0;
! recordDependencyOn(&tableobj, &typeobj, DEPENDENCY_NORMAL);
!
! /* Update pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! classtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(classtuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(classtuple))->reloftype = typeid;
! simple_heap_update(relationRelation, &classtuple->t_self, classtuple);
! heap_freetuple(classtuple);
! heap_close(relationRelation, RowExclusiveLock);
!
! ReleaseSysCache(typetuple);
! }
!
! /*
! * ALTER TABLE NOT OF
! *
! * Detach a typed table from its originating type. Just clear reloftype and
! * remove the dependency.
! */
! static void
! ATExecDropOf(Relation rel, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel);
! Relation relationRelation;
! HeapTuple tuple;
!
! if (!OidIsValid(rel->rd_rel->reloftype))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a typed table",
! RelationGetRelationName(rel))));
!
! /*
! * We don't bother to check ownership of the row type --- ownership of the
! * child is presumed enough rights. No particular lock required on the
! * row type, either.
! */
!
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Clear pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(tuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(tuple))->reloftype = InvalidOid;
! simple_heap_update(relationRelation, &tuple->t_self, tuple);
! heap_freetuple(tuple);
! heap_close(relationRelation, RowExclusiveLock);
}
/*
diff --git a/src/backend/parser/gram.y index a22ab66..1e4f8f6 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 1933,1938 **** alter_table_cmd:
--- 1933,1955 ----
n->def = (Node *) $3;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> OF <type_name> */
+ | OF any_name
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ TypeName *def = makeTypeNameFromNameList($2);
+ def->location = @2;
+ n->subtype = AT_AddOf;
+ n->def = (Node *) def;
+ $$ = (Node *)n;
+ }
+ /* ALTER TABLE <name> NOT OF */
+ | NOT OF
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DropOf;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> OWNER TO RoleId */
| OWNER TO RoleId
{
diff --git a/src/include/nodes/pindex d9eac76..e28c189 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1218,1223 **** typedef enum AlterTableType
--- 1218,1225 ----
AT_DisableRule, /* DISABLE RULE name */
AT_AddInherit, /* INHERIT parent */
AT_DropInherit, /* NO INHERIT parent */
+ AT_AddOf, /* OF <type_name> */
+ AT_DropOf, /* NOT OF */
AT_GenericOptions, /* OPTIONS (...) */
} AlterTableType;
diff --git a/src/test/regress/expecteindex d7d1b64..b7b0b8b 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1929,1931 **** Typed table of type: test_type2
--- 1929,1970 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+ --
+ -- typed tables: OF / NOT OF
+ --
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ERROR: table "tt1" has different type for column "y"
+ ALTER TABLE tt2 OF tt_t0;
+ ERROR: table "tt2" has different type for column "y"
+ ALTER TABLE tt3 OF tt_t0;
+ ERROR: table has column "y" where type requires "x"
+ ALTER TABLE tt4 OF tt_t0;
+ ERROR: table is missing column "y"
+ ALTER TABLE tt5 OF tt_t0;
+ ERROR: table has extra column "z"
+ ALTER TABLE tt6 OF tt_t0;
+ ERROR: typed tables cannot inherit
+ ALTER TABLE tt7 OF tt_t0;
+ ALTER TABLE tt7 OF tt0; -- reassign an already-typed table
+ ALTER TABLE tt0 OF tt7; -- no cycles allowed, though
+ ERROR: circular typed table relationship not allowed
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
+ Table "public.tt7"
+ Column | Type | Modifiers
+ --------+--------------+-----------
+ x | integer |
+ y | numeric(8,2) |
+
diff --git a/src/test/regress/sql/alter_table.sqindex 749584d..bd9dab1 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1359,1361 **** ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
--- 1359,1392 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+
+ --
+ -- typed tables: OF / NOT OF
+ --
+
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ALTER TABLE tt2 OF tt_t0;
+ ALTER TABLE tt3 OF tt_t0;
+ ALTER TABLE tt4 OF tt_t0;
+ ALTER TABLE tt5 OF tt_t0;
+ ALTER TABLE tt6 OF tt_t0;
+ ALTER TABLE tt7 OF tt_t0;
+
+ ALTER TABLE tt7 OF tt0; -- reassign an already-typed table
+ ALTER TABLE tt0 OF tt7; -- no cycles allowed, though
+
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
tt2v1-binary-upgrade.patchtext/plain; charset=us-asciiDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1ccdb4d..41ec645 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 7957,7962 **** static void
--- 7957,7963 ----
dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
{
PQExpBuffer q = createPQExpBuffer();
+ PQExpBuffer dropped = createPQExpBuffer();
PQExpBuffer delq = createPQExpBuffer();
PQExpBuffer labelq = createPQExpBuffer();
PQExpBuffer query = createPQExpBuffer();
***************
*** 7964,7971 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 7965,7976 ----
int ntups;
int i_attname;
int i_atttypdefn;
+ int i_attisdropped;
+ int i_attlen;
+ int i_attalign;
int i_typrelid;
int i;
+ int actual_atts;
/* Set proper schema search path so type references list correctly */
selectSourceSchema(tyinfo->dobj.namespace->dobj.name);
***************
*** 7975,7985 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
appendPQExpBuffer(query, "SELECT a.attname, "
"pg_catalog.format_type(a.atttypid, a.atttypmod) AS atttypdefn, "
! "typrelid "
"FROM pg_catalog.pg_type t, pg_catalog.pg_attribute a "
"WHERE t.oid = '%u'::pg_catalog.oid "
"AND a.attrelid = t.typrelid "
- "AND NOT a.attisdropped "
"ORDER BY a.attnum ",
tyinfo->dobj.catId.oid);
--- 7980,7989 ----
appendPQExpBuffer(query, "SELECT a.attname, "
"pg_catalog.format_type(a.atttypid, a.atttypmod) AS atttypdefn, "
! "a.attisdropped, a.attlen, a.attalign, typrelid "
"FROM pg_catalog.pg_type t, pg_catalog.pg_attribute a "
"WHERE t.oid = '%u'::pg_catalog.oid "
"AND a.attrelid = t.typrelid "
"ORDER BY a.attnum ",
tyinfo->dobj.catId.oid);
***************
*** 7990,7995 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 7994,8002 ----
i_attname = PQfnumber(res, "attname");
i_atttypdefn = PQfnumber(res, "atttypdefn");
+ i_attisdropped = PQfnumber(res, "attisdropped");
+ i_attlen = PQfnumber(res, "attlen");
+ i_attalign = PQfnumber(res, "attalign");
i_typrelid = PQfnumber(res, "typrelid");
if (binary_upgrade)
***************
*** 8003,8021 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
appendPQExpBuffer(q, "CREATE TYPE %s AS (",
fmtId(tyinfo->dobj.name));
for (i = 0; i < ntups; i++)
{
char *attname;
char *atttypdefn;
attname = PQgetvalue(res, i, i_attname);
atttypdefn = PQgetvalue(res, i, i_atttypdefn);
! appendPQExpBuffer(q, "\n\t%s %s", fmtId(attname), atttypdefn);
! if (i < ntups - 1)
appendPQExpBuffer(q, ",");
}
appendPQExpBuffer(q, "\n);\n");
/*
* DROP must be fully qualified in case same name appears in pg_catalog
--- 8010,8064 ----
appendPQExpBuffer(q, "CREATE TYPE %s AS (",
fmtId(tyinfo->dobj.name));
+ actual_atts = 0;
for (i = 0; i < ntups; i++)
{
char *attname;
char *atttypdefn;
+ bool attisdropped;
+ char *attlen;
+ char *attalign;
attname = PQgetvalue(res, i, i_attname);
atttypdefn = PQgetvalue(res, i, i_atttypdefn);
+ attisdropped = (PQgetvalue(res, i, i_attisdropped)[0] == 't');
+ attlen = PQgetvalue(res, i, i_attlen);
+ attalign = PQgetvalue(res, i, i_attalign);
! if (attisdropped && !binary_upgrade)
! continue;
!
! /* Format properly if not first attr */
! if (actual_atts++ > 0)
appendPQExpBuffer(q, ",");
+ appendPQExpBuffer(q, "\n\t");
+
+ if (!attisdropped)
+ appendPQExpBuffer(q, "%s %s", fmtId(attname), atttypdefn);
+ else /* binary_upgrade - see under dumpTableSchema() */
+ {
+ appendPQExpBuffer(q, "%s INTEGER /* dummy */", fmtId(attname));
+
+ /* stash separately for insertion after the CREATE TYPE */
+ appendPQExpBuffer(dropped,
+ "\n-- For binary upgrade, recreate dropped column.\n");
+ appendPQExpBuffer(dropped, "UPDATE pg_catalog.pg_attribute\n"
+ "SET attlen = %s, "
+ "attalign = '%s', attbyval = false\n"
+ "WHERE attname = ", attlen, attalign);
+ appendStringLiteralAH(dropped, attname, fout);
+ appendPQExpBuffer(dropped, "\n AND attrelid = ");
+ appendStringLiteralAH(dropped, fmtId(tyinfo->dobj.name), fout);
+ appendPQExpBuffer(dropped, "::pg_catalog.regclass;\n");
+
+ appendPQExpBuffer(dropped, "ALTER TYPE %s ",
+ fmtId(tyinfo->dobj.name));
+ appendPQExpBuffer(dropped, "DROP ATTRIBUTE %s;\n",
+ fmtId(attname));
+ }
}
appendPQExpBuffer(q, "\n);\n");
+ appendPQExpBufferStr(q, dropped->data);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
***************
*** 8051,8056 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 8094,8100 ----
PQclear(res);
destroyPQExpBuffer(q);
+ destroyPQExpBuffer(dropped);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(labelq);
destroyPQExpBuffer(query);
***************
*** 11987,11993 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
"UNLOGGED " : "",
reltypename,
fmtId(tbinfo->dobj.name));
! if (tbinfo->reloftype)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
actual_atts = 0;
for (j = 0; j < tbinfo->numatts; j++)
--- 12031,12037 ----
"UNLOGGED " : "",
reltypename,
fmtId(tbinfo->dobj.name));
! if (tbinfo->reloftype && !binary_upgrade)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
actual_atts = 0;
for (j = 0; j < tbinfo->numatts; j++)
***************
*** 12015,12021 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
bool has_notnull = (tbinfo->notnull[j]
&& (!tbinfo->inhNotNull[j] || binary_upgrade));
! if (tbinfo->reloftype && !has_default && !has_notnull)
continue;
/* Format properly if not first attr */
--- 12059,12066 ----
bool has_notnull = (tbinfo->notnull[j]
&& (!tbinfo->inhNotNull[j] || binary_upgrade));
! if (tbinfo->reloftype && !binary_upgrade &&
! !has_default && !has_notnull)
continue;
/* Format properly if not first attr */
***************
*** 12043,12049 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
/* Attribute type */
! if (tbinfo->reloftype)
{
appendPQExpBuffer(q, "WITH OPTIONS");
}
--- 12088,12094 ----
}
/* Attribute type */
! if (tbinfo->reloftype && !binary_upgrade)
{
appendPQExpBuffer(q, "WITH OPTIONS");
}
***************
*** 12109,12115 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (actual_atts)
appendPQExpBuffer(q, "\n)");
! else if (!tbinfo->reloftype)
{
/*
* We must have a parenthesized attribute list, even though empty,
--- 12154,12160 ----
if (actual_atts)
appendPQExpBuffer(q, "\n)");
! else if (!(tbinfo->reloftype && !binary_upgrade))
{
/*
* We must have a parenthesized attribute list, even though empty,
***************
*** 12251,12256 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
--- 12296,12308 ----
}
}
+ if (tbinfo->reloftype)
+ {
+ appendPQExpBuffer(q, "\n-- For binary upgrade, set up typed tables this way.\n");
+ appendPQExpBuffer(q, "ALTER TABLE ONLY %s OF %s;\n",
+ fmtId(tbinfo->dobj.name), tbinfo->reloftype);
+ }
+
appendPQExpBuffer(q, "\n-- For binary upgrade, set heap's relfrozenxid\n");
appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
"SET relfrozenxid = '%u'\n"
On Fri, Apr 8, 2011 at 5:12 PM, Noah Misch <noah@leadboat.com> wrote:
Implemented as attached. The first patch just adds the ALTER TABLE subcommands
to attach and detach a table from a composite type. A few open questions
concerning typed tables will probably yield minor changes to these subcommands.
I implemented them to be agnostic toward the outcome of those decisions.
I suppose one issue is whether anyone would care to bikeshed on the
proposed syntax. Any takers?
I think you only need an AccessShareLock on InheritsRelationId, since
you are only selecting from it.
If we adopt the elsewhere-proposed approach of forbidding the use of
rowtypes to create typed tables, the circularity-checking logic here
can become simpler. I think it's not actually water-tight right now:
rhaas=# create table a (x int);
CREATE TABLE
rhaas=# create table b of a;
CREATE TABLE
rhaas=# create table c () inherits (b);
CREATE TABLE
rhaas=# create table d of c;
CREATE TABLE
rhaas=# alter table a of d;
ALTER TABLE
pg_dump is not happy with this situation.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
If we adopt the elsewhere-proposed approach of forbidding the use of
rowtypes to create typed tables, the circularity-checking logic here
can become simpler. I think it's not actually water-tight right now:
rhaas=# create table a (x int);
CREATE TABLE
rhaas=# create table b of a;
CREATE TABLE
rhaas=# create table c () inherits (b);
CREATE TABLE
rhaas=# create table d of c;
CREATE TABLE
rhaas=# alter table a of d;
ALTER TABLE
"alter table a of d"? What the heck does that mean, and why would it be
a good idea?
regards, tom lane
On Wed, Apr 13, 2011 at 11:46:45PM -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
If we adopt the elsewhere-proposed approach of forbidding the use of
rowtypes to create typed tables, the circularity-checking logic here
can become simpler. I think it's not actually water-tight right now:rhaas=# create table a (x int);
CREATE TABLE
rhaas=# create table b of a;
CREATE TABLE
rhaas=# create table c () inherits (b);
CREATE TABLE
rhaas=# create table d of c;
CREATE TABLE
rhaas=# alter table a of d;
ALTER TABLE"alter table a of d"? What the heck does that mean, and why would it be
a good idea?
CREATE TABLE a ...; ...; ALTER TABLE a OF d; = CREATE TABLE a OF d;
It's a good idea as a heavy lifter for `pg_dump --binary-upgrade'. See the rest
of this thread for the full background.
Robert,
Thanks for the review.
On Wed, Apr 13, 2011 at 08:01:17PM -0700, Robert Haas wrote:
I think you only need an AccessShareLock on InheritsRelationId, since
you are only selecting from it.
True; fixed.
If we adopt the elsewhere-proposed approach of forbidding the use of
rowtypes to create typed tables, the circularity-checking logic here
can become simpler. I think it's not actually water-tight right now:rhaas=# create table a (x int);
CREATE TABLE
rhaas=# create table b of a;
CREATE TABLE
rhaas=# create table c () inherits (b);
CREATE TABLE
rhaas=# create table d of c;
CREATE TABLE
rhaas=# alter table a of d;
ALTER TABLEpg_dump is not happy with this situation.
Good test case.
Since we're going to forbid hanging a typed table off a table rowtype, I believe
the circularity check becomes entirely superfluous. I'm suspicious that I'm
missing some way to introduce problematic circularity using composite-typed
columns, but I couldn't come up with a problematic example. The current check
would not detect such a problem, if one does exist, anyway.
When we're done with the relkind-restriction patch, I'll post a new version of
this one. It will remove the circularity check and add a relkind check.
nm
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. It will remove the circularity check and add a relkind check.
Here it is. Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changes
Changes from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45
Attachments:
tt1v2-alter-of.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index c194862..4e02438 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 63,68 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 63,70 ----
RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
+ OF <replaceable class="PARAMETER">type_name</replaceable>
+ NOT OF
OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
***************
*** 491,496 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 493,522 ----
</varlistentry>
<varlistentry>
+ <term><literal>OF <replaceable class="PARAMETER">type_name</replaceable></literal></term>
+ <listitem>
+ <para>
+ This form links the table to a composite type as though <command>CREATE
+ TABLE OF</> had formed it. The table's list of column names and types
+ must precisely match that of the composite type; the presence of
+ an <literal>oid</> system column is permitted to differ. The table must
+ not inherit from any other table. These restrictions ensure
+ that <command>CREATE TABLE OF</> would permit an equivalent table
+ definition.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>NOT OF</literal></term>
+ <listitem>
+ <para>
+ This form dissociates a typed table from its type.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>OWNER</literal></term>
<listitem>
<para>
diff --git a/src/backend/commands/tablecindex 1f709a4..f0e6f24 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 81,86 ****
--- 81,87 ----
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
+ #include "utils/typcache.h"
/*
***************
*** 357,362 **** static void ATExecEnableDisableRule(Relation rel, char *rulename,
--- 358,366 ----
static void ATPrepAddInherit(Relation child_rel);
static void ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static void ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
+ static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid);
+ static void ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode);
+ static void ATExecDropOf(Relation rel, LOCKMODE lockmode);
static void ATExecGenericOptions(Relation rel, List *options);
static void copy_relation_data(SMgrRelation rel, SMgrRelation dst,
***************
*** 2684,2689 **** AlterTableGetLockLevel(List *cmds)
--- 2688,2703 ----
break;
/*
+ * These subcommands affect implicit row type conversion. They
+ * have affects similar to CREATE/DROP CAST on queries. We
+ * don't provide for invalidating parse trees as a result of
+ * such changes. Do avoid concurrent pg_class updates, though.
+ */
+ case AT_AddOf:
+ case AT_DropOf:
+ cmd_lockmode = ShareUpdateExclusiveLock;
+
+ /*
* These subcommands affect general strategies for performance
* and maintenance, though don't change the semantic results
* from normal data reads and writes. Delaying an ALTER TABLE
***************
*** 2942,2954 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
- ATSimplePermissions(rel, ATT_TABLE);
- /* These commands never recurse */
- /* No command-specific prep needed */
- pass = AT_PASS_MISC;
- break;
case AT_DropInherit: /* NO INHERIT */
ATSimplePermissions(rel, ATT_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
--- 2956,2966 ----
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
case AT_DropInherit: /* NO INHERIT */
+ case AT_AddOf: /* OF */
+ case AT_DropOf: /* NOT OF */
ATSimplePermissions(rel, ATT_TABLE);
+ /* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
***************
*** 3211,3216 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 3223,3234 ----
case AT_DropInherit:
ATExecDropInherit(rel, (RangeVar *) cmd->def, lockmode);
break;
+ case AT_AddOf:
+ ATExecAddOf(rel, (TypeName *) cmd->def, lockmode);
+ break;
+ case AT_DropOf:
+ ATExecDropOf(rel, lockmode);
+ break;
case AT_GenericOptions:
ATExecGenericOptions(rel, (List *) cmd->def);
break;
***************
*** 4046,4051 **** find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior be
--- 4064,4105 ----
/*
+ * check_of_type
+ *
+ * Check whether a type is suitable for CREATE TABLE OF/ALTER TABLE OF. If it
+ * isn't suitable, throw an error. Currently, we require that the type
+ * originated with CREATE TABLE AS. We could support any row type, but doing so
+ * would require handling a number of extra corner cases in the DDL commands.
+ */
+ void
+ check_of_type(HeapTuple typetuple)
+ {
+ Form_pg_type typ = (Form_pg_type) GETSTRUCT(typetuple);
+ bool typeOk = false;
+
+ if (typ->typtype == TYPTYPE_COMPOSITE)
+ {
+ Relation typeRelation;
+
+ Assert(OidIsValid(typ->typrelid));
+ typeRelation = relation_open(typ->typrelid, AccessShareLock);
+ typeOk = (typeRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE);
+ /*
+ * Close the parent rel, but keep our AccessShareLock on it until xact
+ * commit. That will prevent someone else from deleting or ALTERing
+ * the type before the typed table creation/conversion commits.
+ */
+ relation_close(typeRelation, NoLock);
+ }
+ if (!typeOk)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("type %s is not a composite type",
+ format_type_be(HeapTupleGetOid(typetuple)))));
+ }
+
+
+ /*
* ALTER TABLE ADD COLUMN
*
* Adds an additional attribute to a relation making the assumption that
***************
*** 8355,8362 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple,
! depTuple;
List *connames;
bool found = false;
--- 8409,8415 ----
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple;
List *connames;
bool found = false;
***************
*** 8522,8532 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /*
! * Drop the dependency
! *
! * There's no convenient way to do this, so go trawling through pg_depend
! */
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
--- 8575,8603 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! drop_parent_dependency(RelationGetRelid(rel),
! RelationRelationId,
! RelationGetRelid(parent_rel));
!
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
! }
!
! /*
! * Drop the dependency created by StoreCatalogInheritance1 (CREATE TABLE
! * INHERITS/ALTER TABLE INHERIT -- refclassid will be RelationRelationId) or
! * heap_create_with_catalog (CREATE TABLE OF/ALTER TABLE OF -- refclassid will
! * be TypeRelationId). There's no convenient way to do this, so go trawling
! * through pg_depend.
! */
! static void
! drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid)
! {
! Relation catalogRelation;
! SysScanDesc scan;
! ScanKeyData key[3];
! HeapTuple depTuple;
!
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
***************
*** 8536,8542 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(RelationGetRelid(rel)));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
--- 8607,8613 ----
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
***************
*** 8549,8556 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == RelationRelationId &&
! dep->refobjid == RelationGetRelid(parent_rel) &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
--- 8620,8627 ----
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == refclassid &&
! dep->refobjid == refobjid &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
***************
*** 8558,8566 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
}
/*
--- 8629,8807 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
+ }
! /*
! * ALTER TABLE OF
! *
! * Attach a table to a composite type, as though it had been created with CREATE
! * TABLE OF. All attname, atttypid, atttypmod and attcollation must match. The
! * subject table must not have inheritance parents. These restrictions ensure
! * that you cannot create a configuration impossible with CREATE TABLE OF alone.
! */
! static void
! ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel);
! Type typetuple;
! Form_pg_type typ;
! Oid typeid;
! Relation inheritsRelation,
! relationRelation;
! SysScanDesc scan;
! ScanKeyData key;
! AttrNumber table_attno,
! type_attno;
! TupleDesc typeTupleDesc,
! tableTupleDesc;
! ObjectAddress tableobj,
! typeobj;
! HeapTuple classtuple;
!
! /* Validate the type. */
! typetuple = typenameType(NULL, ofTypename, NULL);
! check_of_type(typetuple);
! typ = (Form_pg_type) GETSTRUCT(typetuple);
! typeid = HeapTupleGetOid(typetuple);
!
! /* Fail if the table has any inheritance parents. */
! inheritsRelation = heap_open(InheritsRelationId, AccessShareLock);
! ScanKeyInit(&key,
! Anum_pg_inherits_inhrelid,
! BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
! scan = systable_beginscan(inheritsRelation, InheritsRelidSeqnoIndexId,
! true, SnapshotNow, 1, &key);
! if (HeapTupleIsValid(systable_getnext(scan)))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("typed tables cannot inherit")));
! systable_endscan(scan);
! heap_close(inheritsRelation, AccessShareLock);
!
! /*
! * Check the tuple descriptors for compatibility. Unlike inheritance, we
! * require that the order also match. However, attnotnull need not match.
! * Also unlike inheritance, we do not require matching relhasoids.
! */
! typeTupleDesc = lookup_rowtype_tupdesc(typeid, -1);
! tableTupleDesc = RelationGetDescr(rel);
! table_attno = 1;
! for (type_attno = 1; type_attno <= typeTupleDesc->natts; type_attno++)
! {
! Form_pg_attribute type_attr,
! table_attr;
! const char *type_attname,
! *table_attname;
!
! /* Get the next non-dropped type attribute. */
! type_attr = typeTupleDesc->attrs[type_attno - 1];
! if (type_attr->attisdropped)
! continue;
! type_attname = NameStr(type_attr->attname);
!
! /* Get the next non-dropped table attribute. */
! do
! {
! if (table_attno > tableTupleDesc->natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table is missing column \"%s\"",
! type_attname)));
! table_attr = tableTupleDesc->attrs[table_attno++ - 1];
! } while (table_attr->attisdropped);
! table_attname = NameStr(table_attr->attname);
!
! /* Compare name. */
! if (strncmp(table_attname, type_attname, NAMEDATALEN) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has column \"%s\" where type requires \"%s\"",
! table_attname, type_attname)));
!
! /* Compare type. */
! if (table_attr->atttypid != type_attr->atttypid ||
! table_attr->atttypmod != type_attr->atttypmod ||
! table_attr->attcollation != type_attr->attcollation)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table \"%s\" has different type for column \"%s\"",
! RelationGetRelationName(rel), type_attname)));
! }
! DecrTupleDescRefCount(typeTupleDesc);
!
! /* Any remaining columns at the end of the table had better be dropped. */
! for (; table_attno <= tableTupleDesc->natts; table_attno++)
! {
! Form_pg_attribute table_attr = tableTupleDesc->attrs[table_attno - 1];
! if (!table_attr->attisdropped)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has extra column \"%s\"",
! NameStr(table_attr->attname))));
! }
!
! /* If the table was already typed, drop the existing dependency. */
! if (rel->rd_rel->reloftype)
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Record a dependency on the new type. */
! tableobj.classId = RelationRelationId;
! tableobj.objectId = relid;
! tableobj.objectSubId = 0;
! typeobj.classId = TypeRelationId;
! typeobj.objectId = typeid;
! typeobj.objectSubId = 0;
! recordDependencyOn(&tableobj, &typeobj, DEPENDENCY_NORMAL);
!
! /* Update pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! classtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(classtuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(classtuple))->reloftype = typeid;
! simple_heap_update(relationRelation, &classtuple->t_self, classtuple);
! heap_freetuple(classtuple);
! heap_close(relationRelation, RowExclusiveLock);
!
! ReleaseSysCache(typetuple);
! }
!
! /*
! * ALTER TABLE NOT OF
! *
! * Detach a typed table from its originating type. Just clear reloftype and
! * remove the dependency.
! */
! static void
! ATExecDropOf(Relation rel, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel);
! Relation relationRelation;
! HeapTuple tuple;
!
! if (!OidIsValid(rel->rd_rel->reloftype))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a typed table",
! RelationGetRelationName(rel))));
!
! /*
! * We don't bother to check ownership of the type --- ownership of the table
! * is presumed enough rights. No lock required on the type, either.
! */
!
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Clear pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(tuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(tuple))->reloftype = InvalidOid;
! simple_heap_update(relationRelation, &tuple->t_self, tuple);
! heap_freetuple(tuple);
! heap_close(relationRelation, RowExclusiveLock);
}
/*
diff --git a/src/backend/parser/gram.y index a22ab66..1e4f8f6 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 1933,1938 **** alter_table_cmd:
--- 1933,1955 ----
n->def = (Node *) $3;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> OF <type_name> */
+ | OF any_name
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ TypeName *def = makeTypeNameFromNameList($2);
+ def->location = @2;
+ n->subtype = AT_AddOf;
+ n->def = (Node *) def;
+ $$ = (Node *)n;
+ }
+ /* ALTER TABLE <name> NOT OF */
+ | NOT OF
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DropOf;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> OWNER TO RoleId */
| OWNER TO RoleId
{
diff --git a/src/backend/parser/index 4f1bb34..0078814 100644
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 825,859 **** transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
TupleDesc tupdesc;
int i;
Oid ofTypeId;
- bool typeOk = false;
AssertArg(ofTypename);
tuple = typenameType(NULL, ofTypename, NULL);
typ = (Form_pg_type) GETSTRUCT(tuple);
ofTypeId = HeapTupleGetOid(tuple);
ofTypename->typeOid = ofTypeId; /* cached for later */
- if (typ->typtype == TYPTYPE_COMPOSITE)
- {
- Relation typeRelation;
-
- Assert(OidIsValid(typ->typrelid));
- typeRelation = relation_open(typ->typrelid, AccessShareLock);
- typeOk = (typeRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE);
- /*
- * Close the parent rel, but keep our AccessShareLock on it until xact
- * commit. That will prevent someone else from deleting or ALTERing
- * the type before the typed table creation commits.
- */
- relation_close(typeRelation, NoLock);
- }
- if (!typeOk)
- 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);
for (i = 0; i < tupdesc->natts; i++)
{
--- 825,839 ----
TupleDesc tupdesc;
int i;
Oid ofTypeId;
AssertArg(ofTypename);
tuple = typenameType(NULL, ofTypename, NULL);
+ check_of_type(tuple);
typ = (Form_pg_type) GETSTRUCT(tuple);
ofTypeId = HeapTupleGetOid(tuple);
ofTypename->typeOid = ofTypeId; /* cached for later */
tupdesc = lookup_rowtype_tupdesc(ofTypeId, -1);
for (i = 0; i < tupdesc->natts; i++)
{
diff --git a/src/include/commands/tablecmindex d438352..3f971eb 100644
*** a/src/include/commands/tablecmds.h
--- b/src/include/commands/tablecmds.h
***************
*** 56,61 **** extern void find_composite_type_dependencies(Oid typeOid,
--- 56,63 ----
Relation origRelation,
const char *origTypeName);
+ extern void check_of_type(HeapTuple typetuple);
+
extern AttrNumber *varattnos_map(TupleDesc olddesc, TupleDesc newdesc);
extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema);
extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
diff --git a/src/include/nodes/parsenodindex c6337cf..24b4f72 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1219,1224 **** typedef enum AlterTableType
--- 1219,1226 ----
AT_DisableRule, /* DISABLE RULE name */
AT_AddInherit, /* INHERIT parent */
AT_DropInherit, /* NO INHERIT parent */
+ AT_AddOf, /* OF <type_name> */
+ AT_DropOf, /* NOT OF */
AT_GenericOptions, /* OPTIONS (...) */
} AlterTableType;
diff --git a/src/test/regress/expecteindex 5b1223b..8344d85 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1942,1944 **** Typed table of type: test_type2
--- 1942,1982 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+ --
+ -- typed tables: OF / NOT OF
+ --
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ERROR: table "tt1" has different type for column "y"
+ ALTER TABLE tt2 OF tt_t0;
+ ERROR: table "tt2" has different type for column "y"
+ ALTER TABLE tt3 OF tt_t0;
+ ERROR: table has column "y" where type requires "x"
+ ALTER TABLE tt4 OF tt_t0;
+ ERROR: table is missing column "y"
+ ALTER TABLE tt5 OF tt_t0;
+ ERROR: table has extra column "z"
+ ALTER TABLE tt6 OF tt_t0;
+ ERROR: typed tables cannot inherit
+ ALTER TABLE tt7 OF tt_t0;
+ CREATE TYPE tt_t1 AS (x int, y numeric(8,2));
+ ALTER TABLE tt7 OF tt_t1; -- reassign an already-typed table
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
+ Table "public.tt7"
+ Column | Type | Modifiers
+ --------+--------------+-----------
+ x | integer |
+ y | numeric(8,2) |
+
diff --git a/src/test/regress/sql/alter_table.sqindex 43a9ce9..25fa7d5 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1369,1371 **** ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
--- 1369,1401 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+
+ --
+ -- typed tables: OF / NOT OF
+ --
+
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ALTER TABLE tt2 OF tt_t0;
+ ALTER TABLE tt3 OF tt_t0;
+ ALTER TABLE tt4 OF tt_t0;
+ ALTER TABLE tt5 OF tt_t0;
+ ALTER TABLE tt6 OF tt_t0;
+ ALTER TABLE tt7 OF tt_t0;
+
+ CREATE TYPE tt_t1 AS (x int, y numeric(8,2));
+ ALTER TABLE tt7 OF tt_t1; -- reassign an already-typed table
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
tt2v2-binary-upgrade.patchtext/plain; charset=us-asciiDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c2f6180..1c6205b 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 7937,7942 **** static void
--- 7937,7943 ----
dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
{
PQExpBuffer q = createPQExpBuffer();
+ PQExpBuffer dropped = createPQExpBuffer();
PQExpBuffer delq = createPQExpBuffer();
PQExpBuffer labelq = createPQExpBuffer();
PQExpBuffer query = createPQExpBuffer();
***************
*** 7944,7952 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 7945,7957 ----
int ntups;
int i_attname;
int i_atttypdefn;
+ int i_attlen;
+ int i_attalign;
+ int i_attisdropped;
int i_attcollation;
int i_typrelid;
int i;
+ int actual_atts;
/* Set proper schema search path so type references list correctly */
selectSourceSchema(tyinfo->dobj.namespace->dobj.name);
***************
*** 7962,7967 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 7967,7973 ----
*/
appendPQExpBuffer(query, "SELECT a.attname, "
"pg_catalog.format_type(a.atttypid, a.atttypmod) AS atttypdefn, "
+ "a.attlen, a.attalign, a.attisdropped, "
"CASE WHEN a.attcollation <> at.typcollation "
"THEN a.attcollation ELSE 0 END AS attcollation, "
"ct.typrelid "
***************
*** 7979,7984 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 7985,7991 ----
/* We assume here that remoteVersion must be at least 70300 */
appendPQExpBuffer(query, "SELECT a.attname, "
"pg_catalog.format_type(a.atttypid, a.atttypmod) AS atttypdefn, "
+ "a.attlen, a.attalign, a.attisdropped, "
"0 AS attcollation, "
"ct.typrelid "
"FROM pg_catalog.pg_type ct, pg_catalog.pg_attribute a "
***************
*** 7996,8001 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 8003,8011 ----
i_attname = PQfnumber(res, "attname");
i_atttypdefn = PQfnumber(res, "atttypdefn");
+ i_attlen = PQfnumber(res, "attlen");
+ i_attalign = PQfnumber(res, "attalign");
+ i_attisdropped = PQfnumber(res, "attisdropped");
i_attcollation = PQfnumber(res, "attcollation");
i_typrelid = PQfnumber(res, "typrelid");
***************
*** 8010,8047 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
appendPQExpBuffer(q, "CREATE TYPE %s AS (",
fmtId(tyinfo->dobj.name));
for (i = 0; i < ntups; i++)
{
char *attname;
char *atttypdefn;
Oid attcollation;
attname = PQgetvalue(res, i, i_attname);
atttypdefn = PQgetvalue(res, i, i_atttypdefn);
attcollation = atooid(PQgetvalue(res, i, i_attcollation));
! appendPQExpBuffer(q, "\n\t%s %s", fmtId(attname), atttypdefn);
! /* Add collation if not default for the column type */
! if (OidIsValid(attcollation))
{
! CollInfo *coll;
! coll = findCollationByOid(attcollation);
! if (coll)
{
! /* always schema-qualify, don't try to be smart */
! appendPQExpBuffer(q, " COLLATE %s.",
! fmtId(coll->dobj.namespace->dobj.name));
! appendPQExpBuffer(q, "%s",
! fmtId(coll->dobj.name));
}
}
!
! if (i < ntups - 1)
! appendPQExpBuffer(q, ",");
}
appendPQExpBuffer(q, "\n);\n");
/*
* DROP must be fully qualified in case same name appears in pg_catalog
--- 8020,8094 ----
appendPQExpBuffer(q, "CREATE TYPE %s AS (",
fmtId(tyinfo->dobj.name));
+ actual_atts = 0;
for (i = 0; i < ntups; i++)
{
char *attname;
char *atttypdefn;
+ char *attlen;
+ char *attalign;
+ bool attisdropped;
Oid attcollation;
attname = PQgetvalue(res, i, i_attname);
atttypdefn = PQgetvalue(res, i, i_atttypdefn);
+ attlen = PQgetvalue(res, i, i_attlen);
+ attalign = PQgetvalue(res, i, i_attalign);
+ attisdropped = (PQgetvalue(res, i, i_attisdropped)[0] == 't');
attcollation = atooid(PQgetvalue(res, i, i_attcollation));
! if (attisdropped && !binary_upgrade)
! continue;
!
! /* Format properly if not first attr */
! if (actual_atts++ > 0)
! appendPQExpBuffer(q, ",");
! appendPQExpBuffer(q, "\n\t");
! if (!attisdropped)
{
! appendPQExpBuffer(q, "%s %s", fmtId(attname), atttypdefn);
! /* Add collation if not default for the column type */
! if (OidIsValid(attcollation))
{
! CollInfo *coll;
!
! coll = findCollationByOid(attcollation);
! if (coll)
! {
! /* always schema-qualify, don't try to be smart */
! appendPQExpBuffer(q, " COLLATE %s.",
! fmtId(coll->dobj.namespace->dobj.name));
! appendPQExpBuffer(q, "%s",
! fmtId(coll->dobj.name));
! }
}
}
! else /* binary_upgrade - see under dumpTableSchema() */
! {
! appendPQExpBuffer(q, "%s INTEGER /* dummy */", fmtId(attname));
!
! /* stash separately for insertion after the CREATE TYPE */
! appendPQExpBuffer(dropped,
! "\n-- For binary upgrade, recreate dropped column.\n");
! appendPQExpBuffer(dropped, "UPDATE pg_catalog.pg_attribute\n"
! "SET attlen = %s, "
! "attalign = '%s', attbyval = false\n"
! "WHERE attname = ", attlen, attalign);
! appendStringLiteralAH(dropped, attname, fout);
! appendPQExpBuffer(dropped, "\n AND attrelid = ");
! appendStringLiteralAH(dropped, fmtId(tyinfo->dobj.name), fout);
! appendPQExpBuffer(dropped, "::pg_catalog.regclass;\n");
!
! appendPQExpBuffer(dropped, "ALTER TYPE %s ",
! fmtId(tyinfo->dobj.name));
! appendPQExpBuffer(dropped, "DROP ATTRIBUTE %s;\n",
! fmtId(attname));
! }
}
appendPQExpBuffer(q, "\n);\n");
+ appendPQExpBufferStr(q, dropped->data);
/*
* DROP must be fully qualified in case same name appears in pg_catalog
***************
*** 8077,8082 **** dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
--- 8124,8130 ----
PQclear(res);
destroyPQExpBuffer(q);
+ destroyPQExpBuffer(dropped);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(labelq);
destroyPQExpBuffer(query);
***************
*** 12004,12010 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
"UNLOGGED " : "",
reltypename,
fmtId(tbinfo->dobj.name));
! if (tbinfo->reloftype)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
actual_atts = 0;
for (j = 0; j < tbinfo->numatts; j++)
--- 12052,12058 ----
"UNLOGGED " : "",
reltypename,
fmtId(tbinfo->dobj.name));
! if (tbinfo->reloftype && !binary_upgrade)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
actual_atts = 0;
for (j = 0; j < tbinfo->numatts; j++)
***************
*** 12032,12038 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
bool has_notnull = (tbinfo->notnull[j]
&& (!tbinfo->inhNotNull[j] || binary_upgrade));
! if (tbinfo->reloftype && !has_default && !has_notnull)
continue;
/* Format properly if not first attr */
--- 12080,12087 ----
bool has_notnull = (tbinfo->notnull[j]
&& (!tbinfo->inhNotNull[j] || binary_upgrade));
! if (tbinfo->reloftype && !binary_upgrade &&
! !has_default && !has_notnull)
continue;
/* Format properly if not first attr */
***************
*** 12060,12066 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
/* Attribute type */
! if (tbinfo->reloftype)
{
appendPQExpBuffer(q, "WITH OPTIONS");
}
--- 12109,12115 ----
}
/* Attribute type */
! if (tbinfo->reloftype && !binary_upgrade)
{
appendPQExpBuffer(q, "WITH OPTIONS");
}
***************
*** 12126,12132 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (actual_atts)
appendPQExpBuffer(q, "\n)");
! else if (!tbinfo->reloftype)
{
/*
* We must have a parenthesized attribute list, even though empty,
--- 12175,12181 ----
if (actual_atts)
appendPQExpBuffer(q, "\n)");
! else if (!(tbinfo->reloftype && !binary_upgrade))
{
/*
* We must have a parenthesized attribute list, even though empty,
***************
*** 12268,12273 **** dumpTableSchema(Archive *fout, TableInfo *tbinfo)
--- 12317,12329 ----
}
}
+ if (tbinfo->reloftype)
+ {
+ appendPQExpBuffer(q, "\n-- For binary upgrade, set up typed tables this way.\n");
+ appendPQExpBuffer(q, "ALTER TABLE ONLY %s OF %s;\n",
+ fmtId(tbinfo->dobj.name), tbinfo->reloftype);
+ }
+
appendPQExpBuffer(q, "\n-- For binary upgrade, set heap's relfrozenxid\n");
appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
"SET relfrozenxid = '%u'\n"
On Mon, Apr 18, 2011 at 7:50 PM, Noah Misch <noah@leadboat.com> wrote:
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. It will remove the circularity check and add a relkind check.Here it is. Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changesChanges from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45
I think there's a bug in the tt1v1 patch. I'm getting intermittent
regression test failures at this point:
ALTER TABLE tt7 OF tt_t1; -- reassign an
already-typed table
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost
In src/test/regress/log/postmaster.log:
TRAP: FailedAssertion("!(((bool)((relation)->rd_refcnt == 0)))", File:
"relcache.c", Line: 1756)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Apr 19, 2011 at 10:36:14PM -0400, Robert Haas wrote:
On Mon, Apr 18, 2011 at 7:50 PM, Noah Misch <noah@leadboat.com> wrote:
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. ?It will remove the circularity check and add a relkind check.Here it is. ?Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changesChanges from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45I think there's a bug in the tt1v1 patch. I'm getting intermittent
regression test failures at this point:ALTER TABLE tt7 OF tt_t1; -- reassign an
already-typed table
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lostIn src/test/regress/log/postmaster.log:
TRAP: FailedAssertion("!(((bool)((relation)->rd_refcnt == 0)))", File:
"relcache.c", Line: 1756)
How frequently does it happen for you? I ran `make check' about fifteen times
without hitting an error. I ran the new test cases under valgrind, and that
also came out clean.
Could you try a fresh build and see if it still happens? If it does, could you
grab a "bt full" and "p *relation->rd_rel" in gdb?
Thanks,
nm
On Wed, Apr 20, 2011 at 9:57 AM, Noah Misch <noah@leadboat.com> wrote:
On Tue, Apr 19, 2011 at 10:36:14PM -0400, Robert Haas wrote:
On Mon, Apr 18, 2011 at 7:50 PM, Noah Misch <noah@leadboat.com> wrote:
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. ?It will remove the circularity check and add a relkind check.Here it is. ?Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changesChanges from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45I think there's a bug in the tt1v1 patch. I'm getting intermittent
regression test failures at this point:ALTER TABLE tt7 OF tt_t1; -- reassign an
already-typed table
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lostIn src/test/regress/log/postmaster.log:
TRAP: FailedAssertion("!(((bool)((relation)->rd_refcnt == 0)))", File:
"relcache.c", Line: 1756)How frequently does it happen for you? I ran `make check' about fifteen times
without hitting an error. I ran the new test cases under valgrind, and that
also came out clean.Could you try a fresh build and see if it still happens? If it does, could you
grab a "bt full" and "p *relation->rd_rel" in gdb?
I reproduced it this morning after git clean -dfx, updating to the
latest master branch, and re-applying your patch. The most recent
time I reproduced it, it took 7 tries, but I think the average
frequency of failure is around 25%.
gdb info attached, courtesy of defining SLEEP_ON_ASSERT to 1 in
pg_config_manual.h
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
On Wed, Apr 20, 2011 at 01:10:19PM -0400, Robert Haas wrote:
On Wed, Apr 20, 2011 at 9:57 AM, Noah Misch <noah@leadboat.com> wrote:
On Tue, Apr 19, 2011 at 10:36:14PM -0400, Robert Haas wrote:
On Mon, Apr 18, 2011 at 7:50 PM, Noah Misch <noah@leadboat.com> wrote:
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. ?It will remove the circularity check and add a relkind check.Here it is. ?Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changesChanges from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45I think there's a bug in the tt1v1 patch. ?I'm getting intermittent
regression test failures at this point:
It neglected to call CatalogUpdateIndexes(); attached new version fixes that.
The failure was intermittent due to HOT; stubbing out HeapSatisfiesHOTUpdate()
with "return false" made the failure appear every time.
Thanks for the failure data.
nm
Attachments:
tt1v3-alter-of.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index c194862..4e02438 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 63,68 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 63,70 ----
RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
+ OF <replaceable class="PARAMETER">type_name</replaceable>
+ NOT OF
OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
***************
*** 491,496 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
--- 493,522 ----
</varlistentry>
<varlistentry>
+ <term><literal>OF <replaceable class="PARAMETER">type_name</replaceable></literal></term>
+ <listitem>
+ <para>
+ This form links the table to a composite type as though <command>CREATE
+ TABLE OF</> had formed it. The table's list of column names and types
+ must precisely match that of the composite type; the presence of
+ an <literal>oid</> system column is permitted to differ. The table must
+ not inherit from any other table. These restrictions ensure
+ that <command>CREATE TABLE OF</> would permit an equivalent table
+ definition.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>NOT OF</literal></term>
+ <listitem>
+ <para>
+ This form dissociates a typed table from its type.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>OWNER</literal></term>
<listitem>
<para>
diff --git a/src/backend/commands/tablecindex 1f709a4..bb77c53 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 81,86 ****
--- 81,87 ----
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
+ #include "utils/typcache.h"
/*
***************
*** 357,362 **** static void ATExecEnableDisableRule(Relation rel, char *rulename,
--- 358,366 ----
static void ATPrepAddInherit(Relation child_rel);
static void ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static void ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
+ static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid);
+ static void ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode);
+ static void ATExecDropOf(Relation rel, LOCKMODE lockmode);
static void ATExecGenericOptions(Relation rel, List *options);
static void copy_relation_data(SMgrRelation rel, SMgrRelation dst,
***************
*** 2684,2689 **** AlterTableGetLockLevel(List *cmds)
--- 2688,2703 ----
break;
/*
+ * These subcommands affect implicit row type conversion. They
+ * have affects similar to CREATE/DROP CAST on queries. We
+ * don't provide for invalidating parse trees as a result of
+ * such changes. Do avoid concurrent pg_class updates, though.
+ */
+ case AT_AddOf:
+ case AT_DropOf:
+ cmd_lockmode = ShareUpdateExclusiveLock;
+
+ /*
* These subcommands affect general strategies for performance
* and maintenance, though don't change the semantic results
* from normal data reads and writes. Delaying an ALTER TABLE
***************
*** 2942,2954 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
- ATSimplePermissions(rel, ATT_TABLE);
- /* These commands never recurse */
- /* No command-specific prep needed */
- pass = AT_PASS_MISC;
- break;
case AT_DropInherit: /* NO INHERIT */
ATSimplePermissions(rel, ATT_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
--- 2956,2966 ----
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
case AT_DropInherit: /* NO INHERIT */
+ case AT_AddOf: /* OF */
+ case AT_DropOf: /* NOT OF */
ATSimplePermissions(rel, ATT_TABLE);
+ /* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
***************
*** 3211,3216 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 3223,3234 ----
case AT_DropInherit:
ATExecDropInherit(rel, (RangeVar *) cmd->def, lockmode);
break;
+ case AT_AddOf:
+ ATExecAddOf(rel, (TypeName *) cmd->def, lockmode);
+ break;
+ case AT_DropOf:
+ ATExecDropOf(rel, lockmode);
+ break;
case AT_GenericOptions:
ATExecGenericOptions(rel, (List *) cmd->def);
break;
***************
*** 4046,4051 **** find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior be
--- 4064,4105 ----
/*
+ * check_of_type
+ *
+ * Check whether a type is suitable for CREATE TABLE OF/ALTER TABLE OF. If it
+ * isn't suitable, throw an error. Currently, we require that the type
+ * originated with CREATE TABLE AS. We could support any row type, but doing so
+ * would require handling a number of extra corner cases in the DDL commands.
+ */
+ void
+ check_of_type(HeapTuple typetuple)
+ {
+ Form_pg_type typ = (Form_pg_type) GETSTRUCT(typetuple);
+ bool typeOk = false;
+
+ if (typ->typtype == TYPTYPE_COMPOSITE)
+ {
+ Relation typeRelation;
+
+ Assert(OidIsValid(typ->typrelid));
+ typeRelation = relation_open(typ->typrelid, AccessShareLock);
+ typeOk = (typeRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE);
+ /*
+ * Close the parent rel, but keep our AccessShareLock on it until xact
+ * commit. That will prevent someone else from deleting or ALTERing
+ * the type before the typed table creation/conversion commits.
+ */
+ relation_close(typeRelation, NoLock);
+ }
+ if (!typeOk)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("type %s is not a composite type",
+ format_type_be(HeapTupleGetOid(typetuple)))));
+ }
+
+
+ /*
* ALTER TABLE ADD COLUMN
*
* Adds an additional attribute to a relation making the assumption that
***************
*** 8355,8362 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple,
! depTuple;
List *connames;
bool found = false;
--- 8409,8415 ----
ScanKeyData key[3];
HeapTuple inheritsTuple,
attributeTuple,
! constraintTuple;
List *connames;
bool found = false;
***************
*** 8522,8532 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /*
! * Drop the dependency
! *
! * There's no convenient way to do this, so go trawling through pg_depend
! */
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
--- 8575,8603 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! drop_parent_dependency(RelationGetRelid(rel),
! RelationRelationId,
! RelationGetRelid(parent_rel));
!
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
! }
!
! /*
! * Drop the dependency created by StoreCatalogInheritance1 (CREATE TABLE
! * INHERITS/ALTER TABLE INHERIT -- refclassid will be RelationRelationId) or
! * heap_create_with_catalog (CREATE TABLE OF/ALTER TABLE OF -- refclassid will
! * be TypeRelationId). There's no convenient way to do this, so go trawling
! * through pg_depend.
! */
! static void
! drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid)
! {
! Relation catalogRelation;
! SysScanDesc scan;
! ScanKeyData key[3];
! HeapTuple depTuple;
!
catalogRelation = heap_open(DependRelationId, RowExclusiveLock);
ScanKeyInit(&key[0],
***************
*** 8536,8542 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(RelationGetRelid(rel)));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
--- 8607,8613 ----
ScanKeyInit(&key[1],
Anum_pg_depend_objid,
BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
***************
*** 8549,8556 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == RelationRelationId &&
! dep->refobjid == RelationGetRelid(parent_rel) &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
--- 8620,8627 ----
{
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
! if (dep->refclassid == refclassid &&
! dep->refobjid == refobjid &&
dep->refobjsubid == 0 &&
dep->deptype == DEPENDENCY_NORMAL)
simple_heap_delete(catalogRelation, &depTuple->t_self);
***************
*** 8558,8566 **** ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode)
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
! /* keep our lock on the parent relation until commit */
! heap_close(parent_rel, NoLock);
}
/*
--- 8629,8809 ----
systable_endscan(scan);
heap_close(catalogRelation, RowExclusiveLock);
+ }
! /*
! * ALTER TABLE OF
! *
! * Attach a table to a composite type, as though it had been created with CREATE
! * TABLE OF. All attname, atttypid, atttypmod and attcollation must match. The
! * subject table must not have inheritance parents. These restrictions ensure
! * that you cannot create a configuration impossible with CREATE TABLE OF alone.
! */
! static void
! ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel);
! Type typetuple;
! Form_pg_type typ;
! Oid typeid;
! Relation inheritsRelation,
! relationRelation;
! SysScanDesc scan;
! ScanKeyData key;
! AttrNumber table_attno,
! type_attno;
! TupleDesc typeTupleDesc,
! tableTupleDesc;
! ObjectAddress tableobj,
! typeobj;
! HeapTuple classtuple;
!
! /* Validate the type. */
! typetuple = typenameType(NULL, ofTypename, NULL);
! check_of_type(typetuple);
! typ = (Form_pg_type) GETSTRUCT(typetuple);
! typeid = HeapTupleGetOid(typetuple);
!
! /* Fail if the table has any inheritance parents. */
! inheritsRelation = heap_open(InheritsRelationId, AccessShareLock);
! ScanKeyInit(&key,
! Anum_pg_inherits_inhrelid,
! BTEqualStrategyNumber, F_OIDEQ,
! ObjectIdGetDatum(relid));
! scan = systable_beginscan(inheritsRelation, InheritsRelidSeqnoIndexId,
! true, SnapshotNow, 1, &key);
! if (HeapTupleIsValid(systable_getnext(scan)))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("typed tables cannot inherit")));
! systable_endscan(scan);
! heap_close(inheritsRelation, AccessShareLock);
!
! /*
! * Check the tuple descriptors for compatibility. Unlike inheritance, we
! * require that the order also match. However, attnotnull need not match.
! * Also unlike inheritance, we do not require matching relhasoids.
! */
! typeTupleDesc = lookup_rowtype_tupdesc(typeid, -1);
! tableTupleDesc = RelationGetDescr(rel);
! table_attno = 1;
! for (type_attno = 1; type_attno <= typeTupleDesc->natts; type_attno++)
! {
! Form_pg_attribute type_attr,
! table_attr;
! const char *type_attname,
! *table_attname;
!
! /* Get the next non-dropped type attribute. */
! type_attr = typeTupleDesc->attrs[type_attno - 1];
! if (type_attr->attisdropped)
! continue;
! type_attname = NameStr(type_attr->attname);
!
! /* Get the next non-dropped table attribute. */
! do
! {
! if (table_attno > tableTupleDesc->natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table is missing column \"%s\"",
! type_attname)));
! table_attr = tableTupleDesc->attrs[table_attno++ - 1];
! } while (table_attr->attisdropped);
! table_attname = NameStr(table_attr->attname);
!
! /* Compare name. */
! if (strncmp(table_attname, type_attname, NAMEDATALEN) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has column \"%s\" where type requires \"%s\"",
! table_attname, type_attname)));
!
! /* Compare type. */
! if (table_attr->atttypid != type_attr->atttypid ||
! table_attr->atttypmod != type_attr->atttypmod ||
! table_attr->attcollation != type_attr->attcollation)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table \"%s\" has different type for column \"%s\"",
! RelationGetRelationName(rel), type_attname)));
! }
! DecrTupleDescRefCount(typeTupleDesc);
!
! /* Any remaining columns at the end of the table had better be dropped. */
! for (; table_attno <= tableTupleDesc->natts; table_attno++)
! {
! Form_pg_attribute table_attr = tableTupleDesc->attrs[table_attno - 1];
! if (!table_attr->attisdropped)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("table has extra column \"%s\"",
! NameStr(table_attr->attname))));
! }
!
! /* If the table was already typed, drop the existing dependency. */
! if (rel->rd_rel->reloftype)
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Record a dependency on the new type. */
! tableobj.classId = RelationRelationId;
! tableobj.objectId = relid;
! tableobj.objectSubId = 0;
! typeobj.classId = TypeRelationId;
! typeobj.objectId = typeid;
! typeobj.objectSubId = 0;
! recordDependencyOn(&tableobj, &typeobj, DEPENDENCY_NORMAL);
!
! /* Update pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! classtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(classtuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(classtuple))->reloftype = typeid;
! simple_heap_update(relationRelation, &classtuple->t_self, classtuple);
! CatalogUpdateIndexes(relationRelation, classtuple);
! heap_freetuple(classtuple);
! heap_close(relationRelation, RowExclusiveLock);
!
! ReleaseSysCache(typetuple);
! }
!
! /*
! * ALTER TABLE NOT OF
! *
! * Detach a typed table from its originating type. Just clear reloftype and
! * remove the dependency.
! */
! static void
! ATExecDropOf(Relation rel, LOCKMODE lockmode)
! {
! Oid relid = RelationGetRelid(rel);
! Relation relationRelation;
! HeapTuple tuple;
!
! if (!OidIsValid(rel->rd_rel->reloftype))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a typed table",
! RelationGetRelationName(rel))));
!
! /*
! * We don't bother to check ownership of the type --- ownership of the table
! * is presumed enough rights. No lock required on the type, either.
! */
!
! drop_parent_dependency(relid, TypeRelationId, rel->rd_rel->reloftype);
!
! /* Clear pg_class.reloftype */
! relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
! tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(tuple))
! elog(ERROR, "cache lookup failed for relation %u", relid);
! ((Form_pg_class) GETSTRUCT(tuple))->reloftype = InvalidOid;
! simple_heap_update(relationRelation, &tuple->t_self, tuple);
! CatalogUpdateIndexes(relationRelation, tuple);
! heap_freetuple(tuple);
! heap_close(relationRelation, RowExclusiveLock);
}
/*
diff --git a/src/backend/parser/gram.y index a22ab66..1e4f8f6 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 1933,1938 **** alter_table_cmd:
--- 1933,1955 ----
n->def = (Node *) $3;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> OF <type_name> */
+ | OF any_name
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ TypeName *def = makeTypeNameFromNameList($2);
+ def->location = @2;
+ n->subtype = AT_AddOf;
+ n->def = (Node *) def;
+ $$ = (Node *)n;
+ }
+ /* ALTER TABLE <name> NOT OF */
+ | NOT OF
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DropOf;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> OWNER TO RoleId */
| OWNER TO RoleId
{
diff --git a/src/backend/parser/index 4f1bb34..0078814 100644
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 825,859 **** transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
TupleDesc tupdesc;
int i;
Oid ofTypeId;
- bool typeOk = false;
AssertArg(ofTypename);
tuple = typenameType(NULL, ofTypename, NULL);
typ = (Form_pg_type) GETSTRUCT(tuple);
ofTypeId = HeapTupleGetOid(tuple);
ofTypename->typeOid = ofTypeId; /* cached for later */
- if (typ->typtype == TYPTYPE_COMPOSITE)
- {
- Relation typeRelation;
-
- Assert(OidIsValid(typ->typrelid));
- typeRelation = relation_open(typ->typrelid, AccessShareLock);
- typeOk = (typeRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE);
- /*
- * Close the parent rel, but keep our AccessShareLock on it until xact
- * commit. That will prevent someone else from deleting or ALTERing
- * the type before the typed table creation commits.
- */
- relation_close(typeRelation, NoLock);
- }
- if (!typeOk)
- 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);
for (i = 0; i < tupdesc->natts; i++)
{
--- 825,839 ----
TupleDesc tupdesc;
int i;
Oid ofTypeId;
AssertArg(ofTypename);
tuple = typenameType(NULL, ofTypename, NULL);
+ check_of_type(tuple);
typ = (Form_pg_type) GETSTRUCT(tuple);
ofTypeId = HeapTupleGetOid(tuple);
ofTypename->typeOid = ofTypeId; /* cached for later */
tupdesc = lookup_rowtype_tupdesc(ofTypeId, -1);
for (i = 0; i < tupdesc->natts; i++)
{
diff --git a/src/include/commands/tablecmindex d438352..3f971eb 100644
*** a/src/include/commands/tablecmds.h
--- b/src/include/commands/tablecmds.h
***************
*** 56,61 **** extern void find_composite_type_dependencies(Oid typeOid,
--- 56,63 ----
Relation origRelation,
const char *origTypeName);
+ extern void check_of_type(HeapTuple typetuple);
+
extern AttrNumber *varattnos_map(TupleDesc olddesc, TupleDesc newdesc);
extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema);
extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
diff --git a/src/include/nodes/parsenodindex c6337cf..24b4f72 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1219,1224 **** typedef enum AlterTableType
--- 1219,1226 ----
AT_DisableRule, /* DISABLE RULE name */
AT_AddInherit, /* INHERIT parent */
AT_DropInherit, /* NO INHERIT parent */
+ AT_AddOf, /* OF <type_name> */
+ AT_DropOf, /* NOT OF */
AT_GenericOptions, /* OPTIONS (...) */
} AlterTableType;
diff --git a/src/test/regress/expecteindex 5b1223b..8344d85 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1942,1944 **** Typed table of type: test_type2
--- 1942,1982 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+ --
+ -- typed tables: OF / NOT OF
+ --
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ERROR: table "tt1" has different type for column "y"
+ ALTER TABLE tt2 OF tt_t0;
+ ERROR: table "tt2" has different type for column "y"
+ ALTER TABLE tt3 OF tt_t0;
+ ERROR: table has column "y" where type requires "x"
+ ALTER TABLE tt4 OF tt_t0;
+ ERROR: table is missing column "y"
+ ALTER TABLE tt5 OF tt_t0;
+ ERROR: table has extra column "z"
+ ALTER TABLE tt6 OF tt_t0;
+ ERROR: typed tables cannot inherit
+ ALTER TABLE tt7 OF tt_t0;
+ CREATE TYPE tt_t1 AS (x int, y numeric(8,2));
+ ALTER TABLE tt7 OF tt_t1; -- reassign an already-typed table
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
+ Table "public.tt7"
+ Column | Type | Modifiers
+ --------+--------------+-----------
+ x | integer |
+ y | numeric(8,2) |
+
diff --git a/src/test/regress/sql/alter_table.sqindex 43a9ce9..25fa7d5 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1369,1371 **** ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
--- 1369,1401 ----
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
+
+ --
+ -- typed tables: OF / NOT OF
+ --
+
+ CREATE TYPE tt_t0 AS (z inet, x int, y numeric(8,2));
+ ALTER TYPE tt_t0 DROP ATTRIBUTE z;
+ CREATE TABLE tt0 (x int NOT NULL, y numeric(8,2)); -- OK
+ CREATE TABLE tt1 (x int, y bigint); -- wrong base type
+ CREATE TABLE tt2 (x int, y numeric(9,2)); -- wrong typmod
+ CREATE TABLE tt3 (y numeric(8,2), x int); -- wrong column order
+ CREATE TABLE tt4 (x int); -- too few columns
+ CREATE TABLE tt5 (x int, y numeric(8,2), z int); -- too few columns
+ CREATE TABLE tt6 () INHERITS (tt0); -- can't have a parent
+ CREATE TABLE tt7 (x int, q text, y numeric(8,2)) WITH OIDS;
+ ALTER TABLE tt7 DROP q; -- OK
+
+ ALTER TABLE tt0 OF tt_t0;
+ ALTER TABLE tt1 OF tt_t0;
+ ALTER TABLE tt2 OF tt_t0;
+ ALTER TABLE tt3 OF tt_t0;
+ ALTER TABLE tt4 OF tt_t0;
+ ALTER TABLE tt5 OF tt_t0;
+ ALTER TABLE tt6 OF tt_t0;
+ ALTER TABLE tt7 OF tt_t0;
+
+ CREATE TYPE tt_t1 AS (x int, y numeric(8,2));
+ ALTER TABLE tt7 OF tt_t1; -- reassign an already-typed table
+ ALTER TABLE tt7 NOT OF;
+ \d tt7
On Wed, Apr 20, 2011 at 6:36 PM, Noah Misch <noah@leadboat.com> wrote:
On Wed, Apr 20, 2011 at 01:10:19PM -0400, Robert Haas wrote:
On Wed, Apr 20, 2011 at 9:57 AM, Noah Misch <noah@leadboat.com> wrote:
On Tue, Apr 19, 2011 at 10:36:14PM -0400, Robert Haas wrote:
On Mon, Apr 18, 2011 at 7:50 PM, Noah Misch <noah@leadboat.com> wrote:
On Fri, Apr 15, 2011 at 11:58:30AM -0400, Noah Misch wrote:
When we're done with the relkind-restriction patch, I'll post a new version of
this one. ?It will remove the circularity check and add a relkind check.Here it is. ?Changes from tt1v1-alter-of.patch to tt1v2-alter-of.patch:
* Use transformOfType()'s relkind check in ATExecAddOf()
* Remove circularity check
* Open pg_inherits with AccessShareLock
* Fix terminology in ATExecDropOf() comment
* Rebase over pgindent changesChanges from tt2v1-binary-upgrade.patch to tt2v2-binary-upgrade.patch:
* Rebase over dumpCompositeType() changes from commit acfa1f45I think there's a bug in the tt1v1 patch. ?I'm getting intermittent
regression test failures at this point:It neglected to call CatalogUpdateIndexes(); attached new version fixes that.
The failure was intermittent due to HOT; stubbing out HeapSatisfiesHOTUpdate()
with "return false" made the failure appear every time.Thanks for the failure data.
Thanks for the patch!
I've now committed this part; the actual fix for pg_dump is still
outstanding. I am not too in love with the syntax you've chosen here,
but since I don't have a better idea I'll wait and see if anyone else
wants to bikeshed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
I've now committed this part; the actual fix for pg_dump is still
outstanding. I am not too in love with the syntax you've chosen here,
but since I don't have a better idea I'll wait and see if anyone else
wants to bikeshed.
How about "ALTER TABLE tabname [NOT] OF TYPE typename"? It's at least a
smidgeon less ambiguous.
regards, tom lane
On Wed, Apr 20, 2011 at 10:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I've now committed this part; the actual fix for pg_dump is still
outstanding. I am not too in love with the syntax you've chosen here,
but since I don't have a better idea I'll wait and see if anyone else
wants to bikeshed.How about "ALTER TABLE tabname [NOT] OF TYPE typename"? It's at least a
smidgeon less ambiguous.
I thought of that, but I hate to make CREATE TABLE and ALTER TABLE
almost-but-not-quite symmetrical. But one might well wonder why we
didn't decide on:
CREATE TABLE n OF TYPE t;
...rather than the actual syntax:
CREATE TABLE n OF t;
...which has brevity to recommend it, but likewise isn't terribly clear.
I presume someone will now refer to a standard of some kind....
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Apr 20, 2011 at 10:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
How about "ALTER TABLE tabname [NOT] OF TYPE typename"? It's at least a
smidgeon less ambiguous.
I thought of that, but I hate to make CREATE TABLE and ALTER TABLE
almost-but-not-quite symmetrical.
Oh, good point.
But one might well wonder why we didn't decide on:
CREATE TABLE n OF TYPE t;
...rather than the actual syntax:
CREATE TABLE n OF t;
...which has brevity to recommend it, but likewise isn't terribly clear.
I presume someone will now refer to a standard of some kind....
SQL:2008 11.3 <table definition>, the bits around <typed table clause>
to be specific.
The SQL committee's taste in syntax is, uh, not mine. They are
amazingly long-winded in places and then they go and do something
like this ...
regards, tom lane
On Apr 20, 2011, at 11:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
But one might well wonder why we didn't decide on:
CREATE TABLE n OF TYPE t;
...rather than the actual syntax:
CREATE TABLE n OF t;
...which has brevity to recommend it, but likewise isn't terribly clear.I presume someone will now refer to a standard of some kind....
SQL:2008 11.3 <table definition>, the bits around <typed table clause>
to be specific.
Right on schedule...
The SQL committee's taste in syntax is, uh, not mine. They are
amazingly long-winded in places and then they go and do something
like this ...
Not to mention that it won't do to use existing syntax (like function call notation) when you could invent bespoke syntax, ideally involving new keywords.
...Robert