Don't set a fast default for anything but a plain table
authorAndrew Dunstan <andrew@dunslane.net>
Fri, 18 Jun 2021 11:44:58 +0000 (07:44 -0400)
committerAndrew Dunstan <andrew@dunslane.net>
Fri, 18 Jun 2021 11:53:08 +0000 (07:53 -0400)
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
src/backend/commands/tablecmds.c
src/backend/utils/cache/relcache.c
src/test/regress/expected/fast_default.out
src/test/regress/sql/fast_default.sql

index 3c332e307dc761e18a9b1f4da5b0221a75cf19de..e32df1616a0482dc120ed87d563e06c11a52978b 100644 (file)
@@ -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();
index 95308db67ea3a9f39f9fc362d06c061987957849..8ef23f6442262c959e460efb810f5e470d2c9918 100644 (file)
@@ -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;
index e516012c17c2e2f2557dd1b1b511dde88c14eba5..c02dfd4fddfbd276bf9bac375a6f7d15f02b7094 100644 (file)
@@ -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;
index 10bc5ff757c211c4fa67f4bede754d5cb744c19a..91f25717b5a5b064ec9ceb01c6743100cf38930c 100644 (file)
@@ -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;
index 4589b9e58d1708fb26acbd15e1d428776aaf97f4..16a3b7ca51d1a3e764638c1ea08ebcf9aeea2470 100644 (file)
@@ -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;