From 306c318043a9c977a248b0fbef7ca091ff863ef1 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Fri, 18 Jun 2021 07:44:58 -0400 Subject: [PATCH] Don't set a fast default for anything but a plain table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fast default code added in Release 11 omitted to check that the table a fast default was being added to was a plain table. Thus one could be added to a foreign table, which predicably blows up. Here we perform that check. In addition, on the back branches, since some of these might have escaped into the wild, if we encounter a missing value for an attribute of something other than a plain table we ignore it. Fixes bug #17056 Backpatch to release 11, Reviewed by: Andres Freund, Álvaro Herrera and Tom Lane --- src/backend/catalog/heap.c | 9 ++++++++- src/backend/commands/tablecmds.c | 7 ++++--- src/backend/utils/cache/relcache.c | 19 ++++++++++++++++++- src/test/regress/expected/fast_default.out | 19 +++++++++++++++++++ src/test/regress/sql/fast_default.sql | 14 ++++++++++++++ 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 3c332e307dc..e32df1616a0 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2065,6 +2065,13 @@ SetAttrMissing(Oid relid, char *attname, char *value) /* lock the table the attribute belongs to */ tablerel = heap_open(relid, AccessExclusiveLock); + /* Don't do anything unless it's a plain table */ + if (tablerel->rd_rel->relkind != RELKIND_RELATION) + { + table_close(tablerel, AccessExclusiveLock); + return; + } + /* Lock the attribute row and get the data */ attrrel = heap_open(AttributeRelationId, RowExclusiveLock); atttup = SearchSysCacheAttName(relid, attname); @@ -2198,7 +2205,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, valuesAtt[Anum_pg_attribute_atthasdef - 1] = true; replacesAtt[Anum_pg_attribute_atthasdef - 1] = true; - if (add_column_mode) + if (rel->rd_rel->relkind == RELKIND_RELATION && add_column_mode) { expr2 = expression_planner(expr2); estate = CreateExecutorState(); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 95308db67ea..8ef23f64422 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10315,10 +10315,11 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, /* * Here we go --- change the recorded column type and collation. (Note - * heapTup is a copy of the syscache entry, so okay to scribble on.) - * First fix up the missing value if any. + * heapTup is a copy of the syscache entry, so okay to scribble on.) First + * fix up the missing value if any. There shouldn't be any missing values + * for anything except plain tables, but if there are, ignore them. */ - if (attTup->atthasmissing) + if (rel->rd_rel->relkind == RELKIND_RELATION && attTup->atthasmissing) { Datum missingval; bool missingNull; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index e516012c17c..c02dfd4fddf 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -546,6 +546,7 @@ RelationBuildTupleDesc(Relation relation) { Form_pg_attribute attp; int attnum; + bool atthasmissing; attp = (Form_pg_attribute) GETSTRUCT(pg_attribute_tuple); @@ -559,6 +560,22 @@ RelationBuildTupleDesc(Relation relation) attp, ATTRIBUTE_FIXED_PART_SIZE); + /* + * Fix atthasmissing flag - it's only for plain tables. Others + * should not have missing values set, but there may be some left from + * before when we placed that check, so this code defensively ignores + * such values. + */ + atthasmissing = attp->atthasmissing; + if (relation->rd_rel->relkind != RELKIND_RELATION && atthasmissing) + { + Form_pg_attribute nattp; + + atthasmissing = false; + nattp = TupleDescAttr(relation->rd_att, attnum - 1); + nattp->atthasmissing = false; + } + /* Update constraint/default info */ if (attp->attnotnull) constr->has_not_null = true; @@ -578,7 +595,7 @@ RelationBuildTupleDesc(Relation relation) } /* Likewise for a missing value */ - if (attp->atthasmissing) + if (atthasmissing) { Datum missingval; bool missingNull; diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index 10bc5ff757c..91f25717b5a 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -797,7 +797,26 @@ SELECT * FROM t WHERE a IS NULL; (1 row) ROLLBACK; +-- verify that a default set on a non-plain table doesn't set a missing +-- value on the attribute +CREATE FOREIGN DATA WRAPPER dummy; +CREATE SERVER s0 FOREIGN DATA WRAPPER dummy; +CREATE FOREIGN TABLE ft1 (c1 integer NOT NULL) SERVER s0; +ALTER FOREIGN TABLE ft1 ADD COLUMN c8 integer DEFAULT 0; +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10); +SELECT count(*) + FROM pg_attribute + WHERE attrelid = 'ft1'::regclass AND + (attmissingval IS NOT NULL OR atthasmissing); + count +------- + 0 +(1 row) + -- cleanup +DROP FOREIGN TABLE ft1; +DROP SERVER s0; +DROP FOREIGN DATA WRAPPER dummy; DROP TABLE vtype; DROP TABLE vtype2; DROP TABLE follower; diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 4589b9e58d1..16a3b7ca51d 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -524,8 +524,22 @@ SET LOCAL enable_seqscan = false; SELECT * FROM t WHERE a IS NULL; ROLLBACK; +-- verify that a default set on a non-plain table doesn't set a missing +-- value on the attribute +CREATE FOREIGN DATA WRAPPER dummy; +CREATE SERVER s0 FOREIGN DATA WRAPPER dummy; +CREATE FOREIGN TABLE ft1 (c1 integer NOT NULL) SERVER s0; +ALTER FOREIGN TABLE ft1 ADD COLUMN c8 integer DEFAULT 0; +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10); +SELECT count(*) + FROM pg_attribute + WHERE attrelid = 'ft1'::regclass AND + (attmissingval IS NOT NULL OR atthasmissing); -- cleanup +DROP FOREIGN TABLE ft1; +DROP SERVER s0; +DROP FOREIGN DATA WRAPPER dummy; DROP TABLE vtype; DROP TABLE vtype2; DROP TABLE follower;