Apply fixes for problems with dropped columns whose types have also been
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 12 May 2003 00:17:34 +0000 (00:17 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 12 May 2003 00:17:34 +0000 (00:17 +0000)
dropped.  Add regression test, too.

src/backend/catalog/heap.c
src/backend/optimizer/prep/preptlist.c
src/test/regress/expected/alter_table.out
src/test/regress/sql/alter_table.sql

index 19ab896ac4ebe4c124fa85a4c6806c9d7a7f4dde..a2e4445fb7279da3606270454a21a60abcf89abd 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.232.2.1 2002/12/16 18:39:56 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.232.2.2 2003/05/12 00:17:33 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -75,7 +75,7 @@ static void StoreAttrDefault(Relation rel, AttrNumber attnum, char *adbin);
 static void StoreRelCheck(Relation rel, char *ccname, char *ccbin);
 static void StoreConstraints(Relation rel, TupleDesc tupdesc);
 static void SetRelationNumChecks(Relation rel, int numchecks);
-static void RemoveStatistics(Relation rel);
+static void RemoveStatistics(Relation rel, AttrNumber attnum);
 
 
 /* ----------------------------------------------------------------
@@ -911,8 +911,9 @@ DeleteAttributeTuples(Oid relid)
  *     RemoveAttributeById
  *
  * This is the guts of ALTER TABLE DROP COLUMN: actually mark the attribute
- * deleted in pg_attribute.  (Everything else needed, such as getting rid
- * of any pg_attrdef entry, is handled by dependency.c.)
+ * deleted in pg_attribute.  We also remove pg_statistic entries for it.
+ * (Everything else needed, such as getting rid of any pg_attrdef entry,
+ * is handled by dependency.c.)
  */
 void
 RemoveAttributeById(Oid relid, AttrNumber attnum)
@@ -946,6 +947,16 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
    /* Mark the attribute as dropped */
    attStruct->attisdropped = true;
 
+   /*
+    * Set the type OID to invalid.  A dropped attribute's type link cannot
+    * be relied on (once the attribute is dropped, the type might be too).
+    * Fortunately we do not need the type row --- the only really essential
+    * information is the type's typlen and typalign, which are preserved in
+    * the attribute's attlen and attalign.  We set atttypid to zero here
+    * as a means of catching code that incorrectly expects it to be valid.
+    */
+   attStruct->atttypid = InvalidOid;
+
    /* Remove any NOT NULL constraint the column may have */
    attStruct->attnotnull = false;
 
@@ -969,6 +980,8 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 
    heap_close(attr_rel, RowExclusiveLock);
 
+   RemoveStatistics(rel, attnum);
+
    relation_close(rel, NoLock);
 }
 
@@ -1143,7 +1156,7 @@ heap_drop_with_catalog(Oid rid)
    /*
     * delete statistics
     */
-   RemoveStatistics(rel);
+   RemoveStatistics(rel, 0);
 
    /*
     * delete attribute tuples
@@ -1817,27 +1830,45 @@ RemoveRelConstraints(Relation rel, const char *constrName,
 }
 
 
+/*
+ * RemoveStatistics --- remove entries in pg_statistic for a rel or column
+ *
+ * If attnum is zero, remove all entries for rel; else remove only the one
+ * for that column.
+ */
 static void
-RemoveStatistics(Relation rel)
+RemoveStatistics(Relation rel, AttrNumber attnum)
 {
    Relation    pgstatistic;
    SysScanDesc scan;
-   ScanKeyData key;
+   ScanKeyData key[2];
+   int         nkeys;
    HeapTuple   tuple;
 
    pgstatistic = heap_openr(StatisticRelationName, RowExclusiveLock);
 
-   ScanKeyEntryInitialize(&key, 0x0,
+   ScanKeyEntryInitialize(&key[0], 0x0,
                           Anum_pg_statistic_starelid, F_OIDEQ,
                           ObjectIdGetDatum(RelationGetRelid(rel)));
 
+   if (attnum == 0)
+       nkeys = 1;
+   else
+   {
+       ScanKeyEntryInitialize(&key[1], 0x0,
+                              Anum_pg_statistic_staattnum, F_INT2EQ,
+                              Int16GetDatum(attnum));
+       nkeys = 2;
+   }
+
    scan = systable_beginscan(pgstatistic, StatisticRelidAttnumIndex, true,
-                             SnapshotNow, 1, &key);
+                             SnapshotNow, nkeys, key);
 
    while (HeapTupleIsValid(tuple = systable_getnext(scan)))
        simple_heap_delete(pgstatistic, &tuple->t_self);
 
    systable_endscan(scan);
+
    heap_close(pgstatistic, RowExclusiveLock);
 }
 
index 95687cb0d0d0d09eed4ce197f0223888e4d2926c..52c24542eb44ffce87c4f6af99aa398b8dc701fd 100644 (file)
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.57 2002/09/18 21:35:21 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.57.2.1 2003/05/12 00:17:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -171,6 +171,15 @@ expand_targetlist(List *tlist, int command_type,
             * the attribute, so that it gets copied to the new tuple. But
             * generate a NULL for dropped columns (we want to drop any
             * old values).
+            *
+            * When generating a NULL constant for a dropped column, we label
+            * it INT4 (any other guaranteed-to-exist datatype would do as
+            * well).  We can't label it with the dropped column's datatype
+            * since that might not exist anymore.  It does not really
+            * matter what we claim the type is, since NULL is NULL --- its
+            * representation is datatype-independent.  This could perhaps
+            * confuse code comparing the finished plan to the target
+            * relation, however.
             */
            Oid         atttype = att_tup->atttypid;
            int32       atttypmod = att_tup->atttypmod;
@@ -179,34 +188,57 @@ expand_targetlist(List *tlist, int command_type,
            switch (command_type)
            {
                case CMD_INSERT:
-                   new_expr = (Node *) makeConst(atttype,
-                                                 att_tup->attlen,
-                                                 (Datum) 0,
-                                                 true, /* isnull */
-                                                 att_tup->attbyval,
-                                                 false,        /* not a set */
-                                                 false);
                    if (!att_tup->attisdropped)
-                       new_expr = coerce_type_constraints(new_expr,
-                                                          atttype,
-                                                          COERCE_IMPLICIT_CAST);
-                   break;
-               case CMD_UPDATE:
-                   /* Insert NULLs for dropped columns */
-                   if (att_tup->attisdropped)
+                   {
                        new_expr = (Node *) makeConst(atttype,
                                                      att_tup->attlen,
                                                      (Datum) 0,
-                                                     true,     /* isnull */
+                                                     true, /* isnull */
                                                      att_tup->attbyval,
                                                      false,    /* not a set */
                                                      false);
+                       new_expr = coerce_type_constraints(new_expr,
+                                                          atttype,
+                                                          COERCE_IMPLICIT_CAST);
+                   }
                    else
+                   {
+                       /* Insert NULL for dropped column */
+                       new_expr = (Node *) makeConst(INT4OID,
+                                                     sizeof(int32),
+                                                     (Datum) 0,
+                                                     true, /* isnull */
+                                                     true, /* byval */
+                                                     false,    /* not a set */
+                                                     false);
+                       /* label resdom with INT4, too */
+                       atttype = INT4OID;
+                       atttypmod = -1;
+                   }
+                   break;
+               case CMD_UPDATE:
+                   if (!att_tup->attisdropped)
+                   {
                        new_expr = (Node *) makeVar(result_relation,
                                                    attrno,
                                                    atttype,
                                                    atttypmod,
                                                    0);
+                   }
+                   else
+                   {
+                       /* Insert NULL for dropped column */
+                       new_expr = (Node *) makeConst(INT4OID,
+                                                     sizeof(int32),
+                                                     (Datum) 0,
+                                                     true, /* isnull */
+                                                     true, /* byval */
+                                                     false,    /* not a set */
+                                                     false);
+                       /* label resdom with INT4, too */
+                       atttype = INT4OID;
+                       atttypmod = -1;
+                   }
                    break;
                default:
                    elog(ERROR, "expand_targetlist: unexpected command_type");
index 4d61811045846ff63aecb296058e67f61d965ee3..550fa28d77e08c983b4b512ca57417f3d03f241e 100644 (file)
@@ -1166,3 +1166,44 @@ order by relname, attnum;
 drop table p1, p2 cascade;
 NOTICE:  Drop cascades to table c1
 NOTICE:  Drop cascades to table gc1
+-- test that operations with a dropped column do not try to reference
+-- its datatype
+create domain mytype as text;
+create temp table foo (f1 text, f2 mytype, f3 text);
+insert into foo values('aa','bb','cc');
+select * from foo;
+ f1 | f2 | f3 
+----+----+----
+ aa | bb | cc
+(1 row)
+
+drop domain mytype cascade;
+NOTICE:  Drop cascades to table foo column f2
+select * from foo;
+ f1 | f3 
+----+----
+ aa | cc
+(1 row)
+
+insert into foo values('qq','rr');
+select * from foo;
+ f1 | f3 
+----+----
+ aa | cc
+ qq | rr
+(2 rows)
+
+update foo set f3 = 'zz';
+select * from foo;
+ f1 | f3 
+----+----
+ aa | zz
+ qq | zz
+(2 rows)
+
+select f3,max(f1) from foo group by f3;
+ f3 | max 
+----+-----
+ zz | qq
+(1 row)
+
index 7fcfb09df2b4f2246ee7f9f954420c4d873fe4f8..df9be014016f66d86f427c6b08ccc1e63f1cc700 100644 (file)
@@ -845,3 +845,21 @@ where relname in ('p1','p2','c1','gc1') and attnum > 0 and not attisdropped
 order by relname, attnum;
 
 drop table p1, p2 cascade;
+
+-- test that operations with a dropped column do not try to reference
+-- its datatype
+
+create domain mytype as text;
+create temp table foo (f1 text, f2 mytype, f3 text);
+
+insert into foo values('aa','bb','cc');
+select * from foo;
+
+drop domain mytype cascade;
+
+select * from foo;
+insert into foo values('qq','rr');
+select * from foo;
+update foo set f3 = 'zz';
+select * from foo;
+select f3,max(f1) from foo group by f3;