Apply fixes for problems with dropped columns whose types have also been
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.
This commit is contained in:
Tom Lane
parent
a7c50cd92b
commit
d97c9b3662
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue