Fix assorted silliness in ATExecSetCompression().
authorTom Lane <[email protected]>
Sun, 21 Mar 2021 22:42:40 +0000 (18:42 -0400)
committerTom Lane <[email protected]>
Sun, 21 Mar 2021 22:43:07 +0000 (18:43 -0400)
It's not okay to scribble directly on a syscache entry.
Nor to continue accessing said entry after releasing it.

Also get rid of not-used local variables.

Per valgrind testing.

src/backend/commands/tablecmds.c

index 9b2800bf5e2f1cbf55c1b085961640ff2a96fc09..877c08814e0772b80f44cbceda833e8c756dbd8f 100644 (file)
@@ -15071,9 +15071,6 @@ ATExecSetCompression(AlteredTableInfo *tab,
    char       *compression;
    char        typstorage;
    Oid         cmoid;
-   Datum       values[Natts_pg_attribute];
-   bool        nulls[Natts_pg_attribute];
-   bool        replace[Natts_pg_attribute];
    ObjectAddress address;
 
    Assert(IsA(newValue, String));
@@ -15081,7 +15078,8 @@ ATExecSetCompression(AlteredTableInfo *tab,
 
    attrel = table_open(AttributeRelationId, RowExclusiveLock);
 
-   tuple = SearchSysCacheAttName(RelationGetRelid(rel), column);
+   /* copy the cache entry so we can scribble on it below */
+   tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), column);
    if (!HeapTupleIsValid(tuple))
        ereport(ERROR,
                (errcode(ERRCODE_UNDEFINED_COLUMN),
@@ -15105,32 +15103,32 @@ ATExecSetCompression(AlteredTableInfo *tab,
                    errmsg("column data type %s does not support compression",
                        format_type_be(atttableform->atttypid))));
 
-   /* initialize buffers for new tuple values */
-   memset(values, 0, sizeof(values));
-   memset(nulls, false, sizeof(nulls));
-   memset(replace, false, sizeof(replace));
-
    /* get the attribute compression method. */
    cmoid = GetAttributeCompression(atttableform, compression);
 
+   /* update pg_attribute entry */
    atttableform->attcompression = cmoid;
    CatalogTupleUpdate(attrel, &tuple->t_self, tuple);
 
    InvokeObjectPostAlterHook(RelationRelationId,
                              RelationGetRelid(rel),
-                             atttableform->attnum);
-
-   ReleaseSysCache(tuple);
+                             attnum);
 
-   /* apply changes to the index column as well */
+   /*
+    * Apply the change to indexes as well (only for simple index columns,
+    * matching behavior of index.c ConstructTupleDescriptor()).
+    */
    SetIndexStorageProperties(rel, attrel, attnum, cmoid, '\0', lockmode);
+
+   heap_freetuple(tuple);
+
    table_close(attrel, RowExclusiveLock);
 
    /* make changes visible */
    CommandCounterIncrement();
 
    ObjectAddressSubSet(address, RelationRelationId,
-                       RelationGetRelid(rel), atttableform->attnum);
+                       RelationGetRelid(rel), attnum);
    return address;
 }