]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
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:03 +0000 (00:17 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 12 May 2003 00:17:03 +0000 (00:17 +0000)
dropped.  The simplest fix for INSERT/UPDATE cases turns out to be for
preptlist.c to insert NULLs of a known-good type (I used INT4) rather
than making them match the deleted column's type.  Since the representation
of NULL is actually datatype-independent, this should work fine.
I also re-reverted the patch to disable the use_physical_tlist optimization
in the presence of dropped columns.  It still doesn't look worth the
trouble to be smarter, if there are no other bugs to fix.
Added a regression test to catch future problems in this area.

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

index ea70765d432b20a20159f36438ece7533aafb3b0..4c9c98e38562cc757b38449d626a85411974f368 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.243 2003/05/08 22:19:56 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.244 2003/05/12 00:17:02 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -76,7 +76,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);
 
 
 /* ----------------------------------------------------------------
@@ -925,8 +925,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)
@@ -960,6 +961,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;
 
@@ -984,6 +995,8 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 
        heap_close(attr_rel, RowExclusiveLock);
 
+       RemoveStatistics(rel, attnum);
+
        relation_close(rel, NoLock);
 }
 
@@ -1158,7 +1171,7 @@ heap_drop_with_catalog(Oid rid)
        /*
         * delete statistics
         */
-       RemoveStatistics(rel);
+       RemoveStatistics(rel, 0);
 
        /*
         * delete attribute tuples
@@ -1819,27 +1832,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 e9dd994cd214f2d2ad987d5b2edd459616f8678d..f50eb792811f745661d8a9b111a000579731026d 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.141 2003/05/11 20:25:50 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.142 2003/05/12 00:17:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -295,6 +295,12 @@ use_physical_tlist(RelOptInfo *rel)
         */
        if (rel->reloptkind != RELOPT_BASEREL)
                return false;
+       /*
+        * Can't do it if relation contains dropped columns.  This is detected
+        * in plancat.c, see notes there.
+        */
+       if (rel->varlist == NIL)
+               return false;
        /*
         * Can't do it if any system columns are requested, either.  (This could
         * possibly be fixed but would take some fragile assumptions in setrefs.c,
index dc8faa9d837c2caa1be22ecbf0dfe8a5387e7d08..00132a53cde494c70df21733a91d02bd0dbb8f77 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.60 2003/02/03 21:15:44 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.61 2003/05/12 00:17:03 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,31 +188,52 @@ 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);
                                        if (!att_tup->attisdropped)
+                                       {
+                                               new_expr = (Node *) makeConst(atttype,
+                                                                                                         att_tup->attlen,
+                                                                                                         (Datum) 0,
+                                                                                                         true, /* isnull */
+                                                                                                         att_tup->attbyval);
                                                new_expr = coerce_to_domain(new_expr,
                                                                                                        InvalidOid,
                                                                                                        atttype,
                                                                                                        COERCE_IMPLICIT_CAST);
+                                       }
+                                       else
+                                       {
+                                               /* Insert NULL for dropped column */
+                                               new_expr = (Node *) makeConst(INT4OID,
+                                                                                                         sizeof(int32),
+                                                                                                         (Datum) 0,
+                                                                                                         true, /* isnull */
+                                                                                                         true /* byval */);
+                                               /* label resdom with INT4, too */
+                                               atttype = INT4OID;
+                                               atttypmod = -1;
+                                       }
                                        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 */
-                                                                                                         att_tup->attbyval);
-                                       else
+                                       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 */);
+                                               /* label resdom with INT4, too */
+                                               atttype = INT4OID;
+                                               atttypmod = -1;
+                                       }
                                        break;
                                default:
                                        elog(ERROR, "expand_targetlist: unexpected command_type");
index c0c4775da8557871adff6cf0aab410bc100f02e3..9ec638a5a436cbfffffd1b58652e437b3a948e89 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/util/plancat.c,v 1.81 2003/05/11 20:25:50 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/util/plancat.c,v 1.82 2003/05/12 00:17:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -62,8 +62,15 @@ get_relation_info(Oid relationObjectId, RelOptInfo *rel)
        relation = heap_open(relationObjectId, AccessShareLock);
 
        /*
-        * Make list of physical Vars.  Note we do NOT ignore dropped columns;
-        * the intent is to model the physical tuples of the relation.
+        * Make list of physical Vars.  But if there are any dropped columns,
+        * punt and set varlist to NIL.  (XXX Ideally we would like to include
+        * dropped columns so that the varlist models the physical tuples
+        * of the relation.  However this creates problems for ExecTypeFromTL,
+        * which may be asked to build a tupdesc for a tlist that includes vars
+        * of no-longer-existent types.  In theory we could dig out the required
+        * info from the pg_attribute entries of the relation, but that data is
+        * not readily available to ExecTypeFromTL.  For now, punt and don't
+        * apply the physical-tlist optimization when there are dropped cols.)
         */
        numattrs = RelationGetNumberOfAttributes(relation);
 
@@ -71,6 +78,13 @@ get_relation_info(Oid relationObjectId, RelOptInfo *rel)
        {
                Form_pg_attribute att_tup = relation->rd_att->attrs[attrno - 1];
 
+               if (att_tup->attisdropped)
+               {
+                       /* found a dropped col, so punt */
+                       varlist = NIL;
+                       break;
+               }
+
                varlist = lappend(varlist,
                                                  makeVar(varno,
                                                                  attrno,
index 26c8c2ed423bb02acf8eebdcc818cd6ea20d67d1..ed6a17e9d48e4dc3c8aa8512b77b491f4cd853f6 100644 (file)
@@ -1253,3 +1253,44 @@ select * from p1;
 drop table p1 cascade;
 NOTICE:  Drop cascades to table c1
 NOTICE:  Drop cascades to constraint p1_a1 on table c1
+-- 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 9dec23499c391ad45aae6798b669254f1a166e64..c97b72cabefd29ab3dc9002afabedc9e9daf138b 100644 (file)
@@ -895,3 +895,21 @@ update p1 set a1 = a1 + 1, f2 = upper(f2);
 select * from p1;
 
 drop table p1 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;