Purely-cosmetic adjustments in tablecmds.c.

Move ATExecAlterColumnGenericOptions away from where it was unthinkingly
dropped, in the middle of a lot of ALTER COLUMN TYPE code.  I don't have
any high principles about where to put it instead, so let's just put it
after ALTER COLUMN TYPE and before ALTER OWNER, matching existing
decisions about how to order related code stanzas.

Also add the minimal function header comment that the original author
was too cool to bother with.

Along the way, upgrade header comments for nearby ALTER COLUMN TYPE
functions.

Discussion: https://postgr.es/m/14787.1561403130@sss.pgh.pa.us

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7d62a0f10a4ae099fac15eecc80f46ae623e1789..ba59fc708a9952acd211812083b738c220b5cbf9 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -458,8 +458,6 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                                           AlterTableCmd *cmd, LOCKMODE lockmode);
 static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
 static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
-static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
-                                                    List *options, LOCKMODE lockmode);
 static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
                                   LOCKMODE lockmode);
 static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
@@ -470,6 +468,8 @@ static void RebuildConstraintComment(AlteredTableInfo *tab, int pass,
                                     const char *conname);
 static void TryReuseIndex(Oid oldId, IndexStmt *stmt);
 static void TryReuseForeignKey(Oid oldId, Constraint *con);
+static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
+                                                    List *options, LOCKMODE lockmode);
 static void change_owner_fix_column_acls(Oid relationOid,
                                         Oid oldOwnerId, Oid newOwnerId);
 static void change_owner_recurse_to_sequences(Oid relationOid,
@@ -11048,118 +11048,12 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
    }
 }
 
-/*
- * Returns the address of the modified column
- */
-static ObjectAddress
-ATExecAlterColumnGenericOptions(Relation rel,
-                               const char *colName,
-                               List *options,
-                               LOCKMODE lockmode)
-{
-   Relation    ftrel;
-   Relation    attrel;
-   ForeignServer *server;
-   ForeignDataWrapper *fdw;
-   HeapTuple   tuple;
-   HeapTuple   newtuple;
-   bool        isnull;
-   Datum       repl_val[Natts_pg_attribute];
-   bool        repl_null[Natts_pg_attribute];
-   bool        repl_repl[Natts_pg_attribute];
-   Datum       datum;
-   Form_pg_foreign_table fttableform;
-   Form_pg_attribute atttableform;
-   AttrNumber  attnum;
-   ObjectAddress address;
-
-   if (options == NIL)
-       return InvalidObjectAddress;
-
-   /* First, determine FDW validator associated to the foreign table. */
-   ftrel = table_open(ForeignTableRelationId, AccessShareLock);
-   tuple = SearchSysCache1(FOREIGNTABLEREL, rel->rd_id);
-   if (!HeapTupleIsValid(tuple))
-       ereport(ERROR,
-               (errcode(ERRCODE_UNDEFINED_OBJECT),
-                errmsg("foreign table \"%s\" does not exist",
-                       RelationGetRelationName(rel))));
-   fttableform = (Form_pg_foreign_table) GETSTRUCT(tuple);
-   server = GetForeignServer(fttableform->ftserver);
-   fdw = GetForeignDataWrapper(server->fdwid);
-
-   table_close(ftrel, AccessShareLock);
-   ReleaseSysCache(tuple);
-
-   attrel = table_open(AttributeRelationId, RowExclusiveLock);
-   tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
-   if (!HeapTupleIsValid(tuple))
-       ereport(ERROR,
-               (errcode(ERRCODE_UNDEFINED_COLUMN),
-                errmsg("column \"%s\" of relation \"%s\" does not exist",
-                       colName, RelationGetRelationName(rel))));
-
-   /* Prevent them from altering a system attribute */
-   atttableform = (Form_pg_attribute) GETSTRUCT(tuple);
-   attnum = atttableform->attnum;
-   if (attnum <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("cannot alter system column \"%s\"", colName)));
-
-
-   /* Initialize buffers for new tuple values */
-   memset(repl_val, 0, sizeof(repl_val));
-   memset(repl_null, false, sizeof(repl_null));
-   memset(repl_repl, false, sizeof(repl_repl));
-
-   /* Extract the current options */
-   datum = SysCacheGetAttr(ATTNAME,
-                           tuple,
-                           Anum_pg_attribute_attfdwoptions,
-                           &isnull);
-   if (isnull)
-       datum = PointerGetDatum(NULL);
-
-   /* Transform the options */
-   datum = transformGenericOptions(AttributeRelationId,
-                                   datum,
-                                   options,
-                                   fdw->fdwvalidator);
-
-   if (PointerIsValid(DatumGetPointer(datum)))
-       repl_val[Anum_pg_attribute_attfdwoptions - 1] = datum;
-   else
-       repl_null[Anum_pg_attribute_attfdwoptions - 1] = true;
-
-   repl_repl[Anum_pg_attribute_attfdwoptions - 1] = true;
-
-   /* Everything looks good - update the tuple */
-
-   newtuple = heap_modify_tuple(tuple, RelationGetDescr(attrel),
-                                repl_val, repl_null, repl_repl);
-
-   CatalogTupleUpdate(attrel, &newtuple->t_self, newtuple);
-
-   InvokeObjectPostAlterHook(RelationRelationId,
-                             RelationGetRelid(rel),
-                             atttableform->attnum);
-   ObjectAddressSubSet(address, RelationRelationId,
-                       RelationGetRelid(rel), attnum);
-
-   ReleaseSysCache(tuple);
-
-   table_close(attrel, RowExclusiveLock);
-
-   heap_freetuple(newtuple);
-
-   return address;
-}
-
 /*
  * Cleanup after we've finished all the ALTER TYPE operations for a
  * particular relation.  We have to drop and recreate all the indexes
- * and constraints that depend on the altered columns.
+ * and constraints that depend on the altered columns.  We do the
+ * actual dropping here, but re-creation is managed by adding work
+ * queue entries to do those steps later.
  */
 static void
 ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
@@ -11272,6 +11166,14 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
     */
 }
 
+/*
+ * Parse the previously-saved definition string for a constraint or index
+ * against the newly-established column data type(s), and queue up the
+ * resulting command parsetrees for execution.
+ *
+ * This might fail if, for example, you have a WHERE clause that uses an
+ * operator that's not available for the new column type.
+ */
 static void
 ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
                     List **wqueue, LOCKMODE lockmode, bool rewrite)
@@ -11566,6 +11468,116 @@ TryReuseForeignKey(Oid oldId, Constraint *con)
    ReleaseSysCache(tup);
 }
 
+/*
+ * ALTER COLUMN .. OPTIONS ( ... )
+ *
+ * Returns the address of the modified column
+ */
+static ObjectAddress
+ATExecAlterColumnGenericOptions(Relation rel,
+                               const char *colName,
+                               List *options,
+                               LOCKMODE lockmode)
+{
+   Relation    ftrel;
+   Relation    attrel;
+   ForeignServer *server;
+   ForeignDataWrapper *fdw;
+   HeapTuple   tuple;
+   HeapTuple   newtuple;
+   bool        isnull;
+   Datum       repl_val[Natts_pg_attribute];
+   bool        repl_null[Natts_pg_attribute];
+   bool        repl_repl[Natts_pg_attribute];
+   Datum       datum;
+   Form_pg_foreign_table fttableform;
+   Form_pg_attribute atttableform;
+   AttrNumber  attnum;
+   ObjectAddress address;
+
+   if (options == NIL)
+       return InvalidObjectAddress;
+
+   /* First, determine FDW validator associated to the foreign table. */
+   ftrel = table_open(ForeignTableRelationId, AccessShareLock);
+   tuple = SearchSysCache1(FOREIGNTABLEREL, rel->rd_id);
+   if (!HeapTupleIsValid(tuple))
+       ereport(ERROR,
+               (errcode(ERRCODE_UNDEFINED_OBJECT),
+                errmsg("foreign table \"%s\" does not exist",
+                       RelationGetRelationName(rel))));
+   fttableform = (Form_pg_foreign_table) GETSTRUCT(tuple);
+   server = GetForeignServer(fttableform->ftserver);
+   fdw = GetForeignDataWrapper(server->fdwid);
+
+   table_close(ftrel, AccessShareLock);
+   ReleaseSysCache(tuple);
+
+   attrel = table_open(AttributeRelationId, RowExclusiveLock);
+   tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+   if (!HeapTupleIsValid(tuple))
+       ereport(ERROR,
+               (errcode(ERRCODE_UNDEFINED_COLUMN),
+                errmsg("column \"%s\" of relation \"%s\" does not exist",
+                       colName, RelationGetRelationName(rel))));
+
+   /* Prevent them from altering a system attribute */
+   atttableform = (Form_pg_attribute) GETSTRUCT(tuple);
+   attnum = atttableform->attnum;
+   if (attnum <= 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("cannot alter system column \"%s\"", colName)));
+
+
+   /* Initialize buffers for new tuple values */
+   memset(repl_val, 0, sizeof(repl_val));
+   memset(repl_null, false, sizeof(repl_null));
+   memset(repl_repl, false, sizeof(repl_repl));
+
+   /* Extract the current options */
+   datum = SysCacheGetAttr(ATTNAME,
+                           tuple,
+                           Anum_pg_attribute_attfdwoptions,
+                           &isnull);
+   if (isnull)
+       datum = PointerGetDatum(NULL);
+
+   /* Transform the options */
+   datum = transformGenericOptions(AttributeRelationId,
+                                   datum,
+                                   options,
+                                   fdw->fdwvalidator);
+
+   if (PointerIsValid(DatumGetPointer(datum)))
+       repl_val[Anum_pg_attribute_attfdwoptions - 1] = datum;
+   else
+       repl_null[Anum_pg_attribute_attfdwoptions - 1] = true;
+
+   repl_repl[Anum_pg_attribute_attfdwoptions - 1] = true;
+
+   /* Everything looks good - update the tuple */
+
+   newtuple = heap_modify_tuple(tuple, RelationGetDescr(attrel),
+                                repl_val, repl_null, repl_repl);
+
+   CatalogTupleUpdate(attrel, &newtuple->t_self, newtuple);
+
+   InvokeObjectPostAlterHook(RelationRelationId,
+                             RelationGetRelid(rel),
+                             atttableform->attnum);
+   ObjectAddressSubSet(address, RelationRelationId,
+                       RelationGetRelid(rel), attnum);
+
+   ReleaseSysCache(tuple);
+
+   table_close(attrel, RowExclusiveLock);
+
+   heap_freetuple(newtuple);
+
+   return address;
+}
+
 /*
  * ALTER TABLE OWNER
  *