Thread: ADD/DROPS inherits

ADD/DROPS inherits

From
Greg Stark
Date:
I couldn't figure out how to make use of the predigested constraints in the
relcache, so I continued on the tack I was on. I just stuf the entire
HeapTuple in a list and decompiled the constraint source only if I find a
constraint with a matching name.

Points I'm uncertain about:

. I throw an elog() error if there's a null conbin for a CHECK constraint. Is
  it possible for a valid CHECK constraint structure to have a null conbin?

. Memory management. Do I need the heap_copytuple or is that unnecessary?
  Would it be ok to simply store the actual HeapTuples as the scan proceeds?

. Locking -- all of it :)

I added some basic (very basic) regression tests and documentation. Did I find
the right places? Is that enough or should I add more?


Index: doc/src/sgml/ref/alter_table.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/alter_table.sgml,v
retrieving revision 1.84
diff -u -p -c -r1.84 alter_table.sgml
cvs diff: conflicting specifications of output style
*** doc/src/sgml/ref/alter_table.sgml    12 Feb 2006 19:11:00 -0000    1.84
--- doc/src/sgml/ref/alter_table.sgml    12 Jun 2006 21:30:54 -0000
*************** where <replaceable class="PARAMETER">act
*** 46,51 ****
--- 46,53 ----
      CLUSTER ON <replaceable class="PARAMETER">index_name</replaceable>
      SET WITHOUT CLUSTER
      SET WITHOUT OIDS
+     INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
+     NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
      OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
      SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable>
  </synopsis>
*************** where <replaceable class="PARAMETER">act
*** 250,255 ****
--- 252,303 ----
     </varlistentry>

     <varlistentry>
+     <term><literal>INHERIT <replaceable class="PARAMETER">parent_table</replaceable></literal></term>
+     <listitem>
+      <para>
+
+       This form adds a new parent table to the table. This won't add new
+       columns to the child table, instead all columns of the parent table must
+       already exist in the child table. They must have matching data types,
+       and if they have <literal>NOT NULL</literal> constraints in the parent
+       then they must also have <literal>NOT NULL</literal> constraints in the
+       child.
+
+       </para>
+       <para>
+
+       There must also be matching table constraints for all
+       <literal>CHECK</literal> table constraints of the parent. Currently
+       <literal>UNIQUE</literal>, <literal>PRIMARY KEY</literal>, and
+       <literal>FOREIGN KEY</literal> constraints are ignored however this may
+       change in the future.
+
+       </para>
+       <para>
+
+       The easiest way to create a suitable table is to create a table using
+       <literal>INHERITS</literal> and then remove it via <literal>NO
+       INHERIT</literal>. Alternatively create a table using
+       <literal>LIKE</literal> however note that <literal>LIKE</literal> does
+       not create the necessary constraints.
+
+      </para>
+
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><literal>NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable></literal></term>
+     <listitem>
+      <para>
+      This form removes a parent table from the list of parents of the table.
+      Queries against the parent table will no longer include records drawn
+      from the target table.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
      <term><literal>OWNER</literal></term>
      <listitem>
       <para>
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.184
diff -u -p -c -r1.184 tablecmds.c
cvs diff: conflicting specifications of output style
*** src/backend/commands/tablecmds.c    10 May 2006 23:18:39 -0000    1.184
--- src/backend/commands/tablecmds.c    12 Jun 2006 21:30:54 -0000
*************** typedef struct NewColumnValue
*** 159,166 ****
--- 159,168 ----
  static void truncate_check_rel(Relation rel);
  static List *MergeAttributes(List *schema, List *supers, bool istemp,
                  List **supOids, List **supconstr, int *supOidCount);
+ static void MergeAttributesIntoExisting(Relation rel, Relation relation);
  static bool change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
  static void StoreCatalogInheritance(Oid relationId, List *supers);
+ static void StoreCatalogInheritance1(Oid relationId, Oid parentOid, int16 seqNumber, Relation catalogRelation);
  static int    findAttrByName(const char *attributeName, List *schema);
  static void setRelhassubclassInRelation(Oid relationId, bool relhassubclass);
  static bool needs_toast_table(Relation rel);
*************** static void ATPrepSetTableSpace(AlteredT
*** 246,251 ****
--- 248,255 ----
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
  static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
                             bool enable, bool skip_system);
+ static void ATExecAddInherits(Relation rel, RangeVar *parent);
+ static void ATExecDropInherits(Relation rel, RangeVar *parent);
  static void copy_relation_data(Relation rel, SMgrRelation dst);
  static void update_ri_trigger_args(Oid relid,
                         const char *oldname,
*************** static void
*** 1156,1165 ****
  StoreCatalogInheritance(Oid relationId, List *supers)
  {
      Relation    relation;
-     TupleDesc    desc;
      int16        seqNumber;
      ListCell   *entry;
-     HeapTuple    tuple;

      /*
       * sanity checks
--- 1160,1167 ----
*************** StoreCatalogInheritance(Oid relationId,
*** 1179,1194 ****
       * anymore, there's no need to look for indirect ancestors.)
       */
      relation = heap_open(InheritsRelationId, RowExclusiveLock);
-     desc = RelationGetDescr(relation);

      seqNumber = 1;
      foreach(entry, supers)
      {
!         Oid            parentOid = lfirst_oid(entry);
          Datum        datum[Natts_pg_inherits];
          char        nullarr[Natts_pg_inherits];
          ObjectAddress childobject,
                      parentobject;

          datum[0] = ObjectIdGetDatum(relationId);        /* inhrel */
          datum[1] = ObjectIdGetDatum(parentOid); /* inhparent */
--- 1181,1206 ----
       * anymore, there's no need to look for indirect ancestors.)
       */
      relation = heap_open(InheritsRelationId, RowExclusiveLock);

      seqNumber = 1;
      foreach(entry, supers)
      {
!         StoreCatalogInheritance1(relationId, lfirst_oid(entry), seqNumber, relation);
!         seqNumber += 1;
!     }
!
!     heap_close(relation, RowExclusiveLock);
! }
!
! static void
! StoreCatalogInheritance1(Oid relationId, Oid parentOid, int16 seqNumber, Relation relation)
! {
          Datum        datum[Natts_pg_inherits];
          char        nullarr[Natts_pg_inherits];
          ObjectAddress childobject,
                      parentobject;
+         HeapTuple    tuple;
+         TupleDesc desc = RelationGetDescr(relation);

          datum[0] = ObjectIdGetDatum(relationId);        /* inhrel */
          datum[1] = ObjectIdGetDatum(parentOid); /* inhparent */
*************** StoreCatalogInheritance(Oid relationId,
*** 1223,1232 ****
           */
          setRelhassubclassInRelation(parentOid, true);

-         seqNumber += 1;
-     }
-
-     heap_close(relation, RowExclusiveLock);
  }

  /*
--- 1235,1240 ----
*************** ATPrepCmd(List **wqueue, Relation rel, A
*** 2053,2058 ****
--- 2061,2068 ----
          case AT_DisableTrig:    /* DISABLE TRIGGER variants */
          case AT_DisableTrigAll:
          case AT_DisableTrigUser:
+         case AT_AddInherits:
+         case AT_DropInherits:
              ATSimplePermissions(rel, false);
              /* These commands never recurse */
              /* No command-specific prep needed */
*************** ATExecCmd(AlteredTableInfo *tab, Relatio
*** 2233,2238 ****
--- 2243,2254 ----
          case AT_DisableTrigUser:        /* DISABLE TRIGGER USER */
              ATExecEnableDisableTrigger(rel, NULL, false, true);
              break;
+         case AT_DropInherits:
+             ATExecDropInherits(rel, cmd->parent);
+             break;
+         case AT_AddInherits:
+             ATExecAddInherits(rel, cmd->parent);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
*************** ATExecEnableDisableTrigger(Relation rel,
*** 5880,5885 ****
--- 5896,6294 ----
      EnableDisableTrigger(rel, trigname, enable, skip_system);
  }

+ static char *
+ decompile_conbin(HeapTuple contuple, TupleDesc tupledesc)
+ {
+     Form_pg_constraint con = (Form_pg_constraint)(GETSTRUCT(contuple));
+     bool isnull;
+     Datum d;
+
+     d = fastgetattr(contuple, Anum_pg_constraint_conbin, tupledesc, &isnull);
+     if (isnull)
+         elog(ERROR, "conbin is null for constraint \"%s\"", NameStr(con->conname));
+     d = DirectFunctionCall2(pg_get_expr, d, ObjectIdGetDatum(con->conrelid));
+     return DatumGetCString(DirectFunctionCall1(textout,d));
+ }
+
+
+ static void
+ ATExecAddInherits(Relation rel, RangeVar *parent)
+ {
+     Relation     relation, catalogRelation;
+     SysScanDesc scan;
+     ScanKeyData key;
+     HeapTuple     inheritsTuple, constraintTuple;
+     int4         inhseqno;
+     ListCell       *elem;
+     List         *children, *constraints;
+     TupleDesc       tupleDesc;
+
+     relation = heap_openrv(parent, AccessShareLock); /* XXX is this enough locking? */
+     if (relation->rd_rel->relkind != RELKIND_RELATION)
+         ereport(ERROR,
+                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                  errmsg("inherited relation \"%s\" is not a table",
+                         parent->relname)));
+
+
+     /* Permanent rels cannot inherit from temporary ones */
+     if (!rel->rd_istemp && isTempNamespace(RelationGetNamespace(relation)))
+         ereport(ERROR,
+                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                  errmsg("cannot inherit from temporary relation \"%s\"",
+                         parent->relname)));
+
+     if (!pg_class_ownercheck(RelationGetRelid(relation), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(relation));
+
+     /* If parent has OIDs then all children must have OIDs */
+     if (relation->rd_rel->relhasoids && !rel->rd_rel->relhasoids)
+         ereport(ERROR,
+                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                  errmsg("table \"%s\" without OIDs cannot inherit from table \"%s\" with OIDs",
+                         RelationGetRelationName(rel), parent->relname)));
+
+     /*
+      * Reject duplications in the list of parents. -- this is the same check as
+      * when creating a table, but maybe we should check for the parent anywhere
+      * higher in the inheritance structure?
+      */
+     catalogRelation = heap_open(InheritsRelationId, RowExclusiveLock);
+     ScanKeyInit(&key,
+                 Anum_pg_inherits_inhrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     scan = systable_beginscan(catalogRelation, InheritsRelidSeqnoIndexId, true, SnapshotNow, 1, &key);
+     inhseqno = 0; /* inhseqno sequences are supposed to start at 1 */
+     while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan)))
+     {
+         Form_pg_inherits inh = (Form_pg_inherits) GETSTRUCT(inheritsTuple);
+         if (inh->inhparent == RelationGetRelid(relation))
+             ereport(ERROR,
+                     (errcode(ERRCODE_DUPLICATE_TABLE),
+                      errmsg("inherited relation \"%s\" duplicated",
+                             parent->relname)));
+         /* Accumulate the max seqno -- BUT if we spot a hole use it instead
+          * This means if you DROP and ADD the same parent the order will be maintained
+          * which will mean pg_dump won't gratuitously reorder your columns.
+          */
+         if (inh->inhseqno == inhseqno+1)
+             inhseqno = inh->inhseqno;
+     }
+     systable_endscan(scan);
+     heap_close(catalogRelation, RowExclusiveLock);
+
+     /* Get children because we have to manually recurse and also because we
+      * have to check for recursive inheritance graphs */
+
+     /* this routine is actually in the planner */
+     children = find_all_inheritors(RelationGetRelid(rel));
+
+     if (list_member_oid(children, RelationGetRelid(relation)))
+         ereport(ERROR,
+                 (errcode(ERRCODE_DUPLICATE_TABLE),
+                  errmsg("circular inheritance structure found, \"%s\" is already a child of \"%s\"",
+                         parent->relname, RelationGetRelationName(rel))));
+
+     foreach(elem, children)
+         {
+             Oid            childrelid = lfirst_oid(elem);
+             Relation    childrel;
+
+             childrel = relation_open(childrelid, AccessExclusiveLock);
+             MergeAttributesIntoExisting(childrel, relation);
+             relation_close(childrel, NoLock);
+         }
+
+
+
+     /* Now check the constraints
+      *
+      * Currently only check constraints are copied when making children so only
+      * check those -- foreign key constraints can and probably ought to be
+      * added but unique/primary key are a whole other ball of wax
+      */
+
+     /* XXX this is O(n^2) which may be issue with tables with many constraints. As
+      * long as tables usually have more like 10 constraints it shouldn't be an
+      * issue. even 100 constraints shouldn't be the end of the world.
+      */
+
+     /* First gather up the child's constraint definitions */
+     catalogRelation = heap_open(ConstraintRelationId, AccessShareLock);
+     tupleDesc = RelationGetDescr(catalogRelation);
+
+     ScanKeyInit(&key,
+                 Anum_pg_constraint_conrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId, true, SnapshotNow, 1, &key);
+     constraints = NIL;
+     while (HeapTupleIsValid(constraintTuple = systable_getnext(scan))) {
+         Form_pg_constraint con = (Form_pg_constraint)(GETSTRUCT(constraintTuple));
+         if (con->contype != CONSTRAINT_CHECK)
+             continue;
+         constraints = lappend(constraints, heap_copytuple(constraintTuple)); /* XXX Do I need the copytuple here? */
+     }
+     systable_endscan(scan);
+
+     /* Then loop through the parent's constraints */
+     ScanKeyInit(&key,
+                 Anum_pg_constraint_conrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(relation)));
+     scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId, true, SnapshotNow, 1, &key);
+     while (HeapTupleIsValid(constraintTuple = systable_getnext(scan))) {
+         bool found = 0;
+         Form_pg_constraint parent_con = (Form_pg_constraint)(GETSTRUCT(constraintTuple));
+         if (parent_con->contype != CONSTRAINT_CHECK)
+             continue;
+
+         foreach(elem, constraints) {
+             HeapTuple child_contuple = lfirst(elem);
+             Form_pg_constraint            child_con = (Form_pg_constraint)(GETSTRUCT(child_contuple));
+
+             if (strcmp(NameStr(parent_con->conname),
+                        NameStr(child_con->conname)))
+                 continue;
+
+             if (parent_con->condeferrable != child_con->condeferrable ||
+                 parent_con->condeferred != child_con->condeferred ||
+                 parent_con->contypid != child_con->contypid ||
+                 strcmp(decompile_conbin(constraintTuple, tupleDesc),
+                        decompile_conbin(child_contuple, tupleDesc))
+                 )
+                 ereport(ERROR,
+                         (errcode(ERRCODE_DATATYPE_MISMATCH),
+                          errmsg("constraint definition for CHECK constraint \"%s\" doesn't match",
+                                 NameStr(parent_con->conname))));
+
+             found = 1;
+             break;
+         }
+         if (!found)
+             ereport(ERROR,
+                     (errcode(ERRCODE_DATATYPE_MISMATCH),
+                      errmsg("child table missing constraint matching parent table constraint \"%s\"",
+                             NameStr(parent_con->conname))));
+
+     }
+     systable_endscan(scan);
+     heap_close(catalogRelation, AccessShareLock);
+
+     catalogRelation = heap_open(InheritsRelationId, RowExclusiveLock);
+     StoreCatalogInheritance1(RelationGetRelid(rel), RelationGetRelid(relation), inhseqno+1, catalogRelation);
+     heap_close(catalogRelation, RowExclusiveLock);
+
+     heap_close(relation, AccessShareLock);
+ }
+
+ static void
+ MergeAttributesIntoExisting(Relation rel, Relation relation)
+ {
+     Relation     attrdesc;
+     AttrNumber    parent_attno, child_attno;
+     TupleDesc    tupleDesc;
+     TupleConstr *constr;
+     HeapTuple     tuple;
+
+     child_attno = RelationGetNumberOfAttributes(rel);
+
+     tupleDesc = RelationGetDescr(relation);
+     constr = tupleDesc->constr;
+
+     for (parent_attno = 1; parent_attno <= tupleDesc->natts;
+          parent_attno++)
+     {
+         Form_pg_attribute attribute = tupleDesc->attrs[parent_attno - 1];
+         char       *attributeName = NameStr(attribute->attname);
+
+         /* Ignore dropped columns in the parent. */
+         if (attribute->attisdropped)
+             continue;
+
+         /* Does it conflict with an existing column? */
+         attrdesc = heap_open(AttributeRelationId, RowExclusiveLock);
+
+         tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), attributeName);
+         if (HeapTupleIsValid(tuple)) {
+             /*
+              * Yes, try to merge the two column definitions. They must
+              * have the same type and typmod.
+              */
+             Form_pg_attribute childatt = (Form_pg_attribute) GETSTRUCT(tuple);
+             if (attribute->atttypid  != childatt->atttypid ||
+                 attribute->atttypmod != childatt->atttypmod ||
+                 (attribute->attnotnull && !childatt->attnotnull))
+                 ereport(ERROR,
+                         (errcode(ERRCODE_DATATYPE_MISMATCH),
+                          errmsg("child table \"%s\" has different type for column \"%s\"",
+                                 RelationGetRelationName(rel), NameStr(attribute->attname))));
+
+             childatt->attinhcount++;
+             simple_heap_update(attrdesc, &tuple->t_self, tuple);
+             CatalogUpdateIndexes(attrdesc, tuple); /* XXX strength reduce openindexes to outside loop? */
+             heap_freetuple(tuple);
+
+             /* We don't touch default at all since we're not making any other
+              * DDL changes to the child */
+
+         } else {
+             /*
+              * No, create a new inherited column
+              *
+              * Creating inherited columns in this case seems to be unpopular.
+              * In the common use case of partitioned tables it's a foot-gun.
+              */
+             ereport(ERROR,
+                     (errcode(ERRCODE_DATATYPE_MISMATCH),
+                      errmsg("child table missing column \"%s\"",
+                             NameStr(attribute->attname))));
+         }
+         heap_close(attrdesc, RowExclusiveLock);
+     }
+
+ }
+
+ static void
+ ATExecDropInherits(Relation rel, RangeVar *parent)
+ {
+
+
+     Relation    catalogRelation;
+     SysScanDesc scan;
+     ScanKeyData key[2];
+     HeapTuple    inheritsTuple, attributeTuple, depTuple;
+     Oid            inhparent;
+     Oid            dropparent;
+     int         found = 0;
+
+     /* Get the OID of parent -- if no schema is specified use the regular
+      * search path and only drop the one table that's found. We could try to be
+      * clever and look at each parent and see if it matches but that would be
+      * inconsistent with other operations I think. */
+
+     Assert(rel);
+     Assert(parent);
+
+     dropparent = RangeVarGetRelid(parent, false);
+
+     /* Search through the direct parents of rel looking for dropparent oid */
+
+     catalogRelation = heap_open(InheritsRelationId, RowExclusiveLock);
+     ScanKeyInit(key,
+                 Anum_pg_inherits_inhrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     scan = systable_beginscan(catalogRelation, InheritsRelidSeqnoIndexId, true, SnapshotNow, 1, key);
+     while (!found && HeapTupleIsValid(inheritsTuple = systable_getnext(scan)))
+     {
+         inhparent = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent;
+         if (inhparent == dropparent) {
+             simple_heap_delete(catalogRelation, &inheritsTuple->t_self);
+             found = 1;
+         }
+     }
+     systable_endscan(scan);
+     heap_close(catalogRelation, RowExclusiveLock);
+
+
+     if (!found) {
+         /* would it be better to look up the actual schema of dropparent and
+          * make the error message explicitly name the qualified name it's
+          * trying to drop ?*/
+         if (parent->schemaname)
+             ereport(ERROR,
+                     (errcode(ERRCODE_UNDEFINED_TABLE),
+                      errmsg("relation \"%s.%s\" is not a parent of relation \"%s\"",
+                             parent->schemaname, parent->relname, RelationGetRelationName(rel))));
+         else
+             ereport(ERROR,
+                     (errcode(ERRCODE_UNDEFINED_TABLE),
+                      errmsg("relation \"%s\" is not a parent of relation \"%s\"",
+                             parent->relname, RelationGetRelationName(rel))));
+     }
+
+     /* Search through columns looking for matching columns from parent table */
+
+     catalogRelation = heap_open(AttributeRelationId, RowExclusiveLock);
+     ScanKeyInit(key,
+                 Anum_pg_attribute_attrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     scan = systable_beginscan(catalogRelation, AttributeRelidNumIndexId, true, SnapshotNow, 1, key);
+     while (HeapTupleIsValid(attributeTuple = systable_getnext(scan))) {
+         Form_pg_attribute att = ((Form_pg_attribute)GETSTRUCT(attributeTuple));
+         /* Not an inherited column at all
+          * (do NOT use islocal for this test--it can be true for inherited columns)
+          */
+         if (att->attinhcount == 0)
+             continue;
+         if (att->attisdropped) /* XXX Is this right? */
+             continue;
+         if (SearchSysCacheExistsAttName(dropparent, NameStr(att->attname))) {
+             /* Decrement inhcount and possibly set islocal to 1 */
+             HeapTuple copyTuple = heap_copytuple(attributeTuple);
+             Form_pg_attribute copy_att = ((Form_pg_attribute)GETSTRUCT(copyTuple));
+
+             copy_att->attinhcount--;
+             if (copy_att->attinhcount == 0)
+                 copy_att->attislocal = 1;
+
+             simple_heap_update(catalogRelation, ©Tuple->t_self, copyTuple);
+             /* XXX "Avoid using it for multiple tuples, since opening the
+              * indexes and building the index info structures is moderately
+              * expensive." Perhaps this can be moved outside the loop or else
+              * at least the CatalogOpenIndexes/CatalogCloseIndexes moved
+              * outside the loop but when I try that it seg faults?!*/
+             CatalogUpdateIndexes(catalogRelation, copyTuple);
+             heap_freetuple(copyTuple);
+         }
+     }
+     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],
+                 Anum_pg_depend_classid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 RelationRelationId);
+     ScanKeyInit(&key[1],
+                 Anum_pg_depend_objid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+
+     scan = systable_beginscan(catalogRelation, DependDependerIndexId, true,
+                               SnapshotNow, 2, key);
+
+     while (HeapTupleIsValid(depTuple = systable_getnext(scan))) {
+         Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(depTuple);
+
+         if (dep->refclassid == RelationRelationId &&
+             dep->refobjid == dropparent &&
+             dep->deptype == DEPENDENCY_NORMAL) {
+
+             /* Only delete a single dependency -- there shouldn't be more but just in case... */
+             simple_heap_delete(catalogRelation, &depTuple->t_self);
+
+             break;
+         }
+     }
+     systable_endscan(scan);
+
+     heap_close(catalogRelation, RowExclusiveLock);
+ }
+
+
+
  /*
   * ALTER TABLE CREATE TOAST TABLE
   *
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.335
diff -u -p -c -r1.335 copyfuncs.c
cvs diff: conflicting specifications of output style
*** src/backend/nodes/copyfuncs.c    30 Apr 2006 18:30:38 -0000    1.335
--- src/backend/nodes/copyfuncs.c    12 Jun 2006 21:30:54 -0000
*************** _copyAlterTableCmd(AlterTableCmd *from)
*** 1799,1804 ****
--- 1799,1805 ----
      COPY_SCALAR_FIELD(subtype);
      COPY_STRING_FIELD(name);
      COPY_NODE_FIELD(def);
+     COPY_NODE_FIELD(parent);
      COPY_NODE_FIELD(transform);
      COPY_SCALAR_FIELD(behavior);

Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.545
diff -u -p -c -r2.545 gram.y
cvs diff: conflicting specifications of output style
*** src/backend/parser/gram.y    27 May 2006 17:38:45 -0000    2.545
--- src/backend/parser/gram.y    12 Jun 2006 21:30:57 -0000
*************** alter_table_cmd:
*** 1514,1519 ****
--- 1514,1535 ----
                      n->subtype = AT_DisableTrigUser;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ALTER INHERITS ADD <parent> */
+             | INHERIT qualified_name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_AddInherits;
+                     n->parent = $2;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> alter INHERITS DROP <parent> */
+             | NO INHERIT qualified_name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DropInherits;
+                     n->parent = $3;
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.310
diff -u -p -c -r1.310 parsenodes.h
cvs diff: conflicting specifications of output style
*** src/include/nodes/parsenodes.h    30 Apr 2006 18:30:40 -0000    1.310
--- src/include/nodes/parsenodes.h    12 Jun 2006 21:31:06 -0000
*************** typedef enum AlterTableType
*** 874,880 ****
      AT_EnableTrigAll,            /* ENABLE TRIGGER ALL */
      AT_DisableTrigAll,            /* DISABLE TRIGGER ALL */
      AT_EnableTrigUser,            /* ENABLE TRIGGER USER */
!     AT_DisableTrigUser            /* DISABLE TRIGGER USER */
  } AlterTableType;

  typedef struct AlterTableCmd    /* one subcommand of an ALTER TABLE */
--- 874,882 ----
      AT_EnableTrigAll,            /* ENABLE TRIGGER ALL */
      AT_DisableTrigAll,            /* DISABLE TRIGGER ALL */
      AT_EnableTrigUser,            /* ENABLE TRIGGER USER */
!     AT_DisableTrigUser,            /* DISABLE TRIGGER USER */
!     AT_AddInherits,                /* ADD INHERITS parent */
!     AT_DropInherits                /* DROP INHERITS parent */
  } AlterTableType;

  typedef struct AlterTableCmd    /* one subcommand of an ALTER TABLE */
*************** typedef struct AlterTableCmd    /* one subc
*** 883,888 ****
--- 885,891 ----
      AlterTableType subtype;        /* Type of table alteration to apply */
      char       *name;            /* column, constraint, or trigger to act on,
                                   * or new owner or tablespace */
+     RangeVar   *parent;            /* Parent table for add/drop inherits */
      Node       *def;            /* definition of new column, column type,
                                   * index, or constraint */
      Node       *transform;        /* transformation expr for ALTER TYPE */
Index: src/test/regress/expected/alter_table.out
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/alter_table.out,v
retrieving revision 1.94
diff -u -p -c -r1.94 alter_table.out
cvs diff: conflicting specifications of output style
*** src/test/regress/expected/alter_table.out    23 Mar 2006 00:19:30 -0000    1.94
--- src/test/regress/expected/alter_table.out    12 Jun 2006 21:31:12 -0000
*************** insert into atacc3 (test2) values (3);
*** 306,311 ****
--- 306,361 ----
  drop table atacc3;
  drop table atacc2;
  drop table atacc1;
+ -- same things with one created with INHERIT
+ create table atacc1 (test int);
+ create table atacc2 (test2 int);
+ create table atacc3 (test3 int) inherits (atacc1, atacc2);
+ alter table atacc3 no inherit atacc2;
+ -- fail
+ alter table atacc3 no inherit atacc2;
+ ERROR:  relation "atacc2" is not a parent of relation "atacc3"
+ -- make sure it really isn't a child
+ insert into atacc3 (test2) values (3);
+ select test2 from atacc2;
+  test2
+ -------
+ (0 rows)
+
+ -- fail due to missing constraint
+ alter table atacc2 add constraint foo check (test2>0);
+ alter table atacc3 inherit atacc2;
+ ERROR:  child table missing constraint matching parent table constraint "foo"
+ -- fail due to missing column
+ alter table atacc3 rename test2 to testx;
+ alter table atacc3 inherit atacc2;
+ ERROR:  child table missing column "test2"
+ -- fail due to mismatched data type
+ alter table atacc3 add test2 bool;
+ alter table atacc3 add inherit atacc2;
+ alter table atacc3 drop test2;
+ -- succeed
+ alter table atacc3 add test2 int;
+ update atacc3 set test2 = 4 where test2 is null;
+ alter table atacc3 add constraint foo check (test2>0);
+ alter table atacc3 inherit atacc2;
+ -- fail due to duplicates and circular inheritance
+ alter table atacc3 inherit atacc2;
+ ERROR:  inherited relation "atacc2" duplicated
+ alter table atacc2 inherit atacc3;
+ ERROR:  circular inheritance structure found, "atacc3" is already a child of "atacc2"
+ alter table atacc2 inherit atacc2;
+ ERROR:  circular inheritance structure found, "atacc2" is already a child of "atacc2"
+ -- test that we really are a child now (should see 4 not 3 and cascade should go through)
+ select test2 from atacc2;
+  test2
+ -------
+      4
+ (1 row)
+
+ drop table atacc2 cascade;
+ NOTICE:  drop cascades to table atacc3
+ NOTICE:  drop cascades to constraint foo on table atacc3
+ drop table atacc1;
  -- let's try only to add only to the parent
  create table atacc1 (test int);
  create table atacc2 (test2 int);
Index: src/test/regress/sql/alter_table.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/alter_table.sql,v
retrieving revision 1.54
diff -u -p -c -r1.54 alter_table.sql
cvs diff: conflicting specifications of output style
*** src/test/regress/sql/alter_table.sql    12 Feb 2006 19:11:01 -0000    1.54
--- src/test/regress/sql/alter_table.sql    12 Jun 2006 21:31:12 -0000
*************** drop table atacc3;
*** 336,341 ****
--- 336,375 ----
  drop table atacc2;
  drop table atacc1;

+ -- same things with one created with INHERIT
+ create table atacc1 (test int);
+ create table atacc2 (test2 int);
+ create table atacc3 (test3 int) inherits (atacc1, atacc2);
+ alter table atacc3 no inherit atacc2;
+ -- fail
+ alter table atacc3 no inherit atacc2;
+ -- make sure it really isn't a child
+ insert into atacc3 (test2) values (3);
+ select test2 from atacc2;
+ -- fail due to missing constraint
+ alter table atacc2 add constraint foo check (test2>0);
+ alter table atacc3 inherit atacc2;
+ -- fail due to missing column
+ alter table atacc3 rename test2 to testx;
+ alter table atacc3 inherit atacc2;
+ -- fail due to mismatched data type
+ alter table atacc3 add test2 bool;
+ alter table atacc3 add inherit atacc2;
+ alter table atacc3 drop test2;
+ -- succeed
+ alter table atacc3 add test2 int;
+ update atacc3 set test2 = 4 where test2 is null;
+ alter table atacc3 add constraint foo check (test2>0);
+ alter table atacc3 inherit atacc2;
+ -- fail due to duplicates and circular inheritance
+ alter table atacc3 inherit atacc2;
+ alter table atacc2 inherit atacc3;
+ alter table atacc2 inherit atacc2;
+ -- test that we really are a child now (should see 4 not 3 and cascade should go through)
+ select test2 from atacc2;
+ drop table atacc2 cascade;
+ drop table atacc1;
+
  -- let's try only to add only to the parent

  create table atacc1 (test int);


--
greg

Re: ADD/DROPS inherits

From
Simon Riggs
Date:
On Mon, 2006-06-12 at 17:39 -0400, Greg Stark wrote:

> Points I'm uncertain about:
>
> . I throw an elog() error if there's a null conbin for a CHECK constraint. Is
>   it possible for a valid CHECK constraint structure to have a null conbin?

ruleutils shows: elog(ERROR, "null conbin for constraint %u"

> I added some basic (very basic) regression tests

Should we fail if columns in the wrong order from the parent? I thought
that was one of the restrictions you discovered?

Can we test for
    ALTER TABLE child NO INHERIT parent1 INHERIT parent2
That was on Hannu's wish list.

Is INHERIT allowed or disallowed with other ALTER TABLE options?
If it is allowed, can we test for something that will fail and something
that would pass, e.g. ALTER TABLE DROP column1 INHERITS parent -- where
the parent passes on column1.

When I read those tests, it makes me think this should be INHERITS and
NOT INHERITS (not great English, but then neither is NO INHERIT).
ISTM it might become confusing between INHERITS and INHERIT.
Waddyathink?

--
  Simon Riggs
  EnterpriseDB          http://www.enterprisedb.com


Re: ADD/DROPS inherits

From
Greg Stark
Date:
Simon Riggs <simon@2ndquadrant.com> writes:

> On Mon, 2006-06-12 at 17:39 -0400, Greg Stark wrote:
>
> > Points I'm uncertain about:
> >
> > . I throw an elog() error if there's a null conbin for a CHECK constraint. Is
> >   it possible for a valid CHECK constraint structure to have a null conbin?
>
> ruleutils shows: elog(ERROR, "null conbin for constraint %u"

I'm unclear what you mean by this. This doesn't look like what I would expect
the error to look like if it was triggered. And the "%u" makes it appear as if
the name of the constraint it "%u" which is passing strange too.

How do I reproduce this?

> > I added some basic (very basic) regression tests
>
> Should we fail if columns in the wrong order from the parent? I thought
> that was one of the restrictions you discovered?

I don't think we can complain about wrongly ordered columns. Tom pointed out
something as simple as adding more columns to the parent can create a
non-standard ordering since it adds them after the local columns. And in any
case verifying the order in complicated cases involving multiple parents and
locally defined columns would be nigh impossible anyways.

> Can we test for
>     ALTER TABLE child NO INHERIT parent1 INHERIT parent2
> That was on Hannu's wish list.
>
> Is INHERIT allowed or disallowed with other ALTER TABLE options?
> If it is allowed, can we test for something that will fail and something
> that would pass, e.g. ALTER TABLE DROP column1 INHERITS parent -- where
> the parent passes on column1.

Both those two cases both work, so you just want more regression tests?
No problem.

Note that operations aren't done in a strictly left-to-right order. For
instance ADD/DROP columns are done before INHERIT/NO INHERIT.

And "ALTER TABLE child NO INHERIT parent 1, INHERIT parent2" will treat
attislocal subtly different from the reverse.


> When I read those tests, it makes me think this should be INHERITS and
> NOT INHERITS (not great English, but then neither is NO INHERIT).
> ISTM it might become confusing between INHERITS and INHERIT.
> Waddyathink?

None of these syntaxes are particularly more or less appealing than any other
to me. I'm still trying to think of something better.

--
greg