Simplify some logic around setting pg_attribute.atthasdef.

DefineRelation was of the opinion that it could usefully pre-fill
atthasdef flags to eliminate work for StoreAttrDefault.  This is not
the case, however: the tupledesc that it's filling is not the one that
InsertPgAttributeTuples will work from.  The tupledesc used there is
made by RelationBuildLocalRelation, which deliberately doesn't copy
atthasdef.  Moreover, if this did happen as the code thinks, it would
be wrong for the case of plain "DEFAULT NULL" clauses, since we detect
and ignore simple-null-Const defaults later on.  Hence, remove the
useless code.

It also emerges that it's not really worth a special-case path in
StoreAttrDefault() for atthasdef already being set, because as far as
we can see that never happens: cases where an existing default gets
updated always do RemoveAttrDefault first, so as to clean up
possibly-no-longer-correct dependency entries.  If it were the case
the code would still work, anyway.

Also remove a nearby comment made moot by 5eaa0e92e.

Author: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com

diff --git a/src/backend/catalog/pg_attrdef.c b/src/backend/catalog/pg_attrdef.c
index f2773682dfbb82ea252e9c8bcc9260289d6154ce..1b6270b12132421cad4e8b7cf4e04db72d41cba4 100644 (file)
--- a/src/backend/catalog/pg_attrdef.c
+++ b/src/backend/catalog/pg_attrdef.c
@@ -43,6 +43,9 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
    Relation    attrrel;
    HeapTuple   atttup;
    Form_pg_attribute attStruct;
+   Datum       valuesAtt[Natts_pg_attribute] = {0};
+   bool        nullsAtt[Natts_pg_attribute] = {0};
+   bool        replacesAtt[Natts_pg_attribute] = {0};
    char        attgenerated;
    Oid         attrdefOid;
    ObjectAddress colobject,
@@ -92,20 +95,15 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
             attnum, RelationGetRelid(rel));
    attStruct = (Form_pg_attribute) GETSTRUCT(atttup);
    attgenerated = attStruct->attgenerated;
-   if (!attStruct->atthasdef)
-   {
-       Datum       valuesAtt[Natts_pg_attribute] = {0};
-       bool        nullsAtt[Natts_pg_attribute] = {0};
-       bool        replacesAtt[Natts_pg_attribute] = {0};
 
-       valuesAtt[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(true);
-       replacesAtt[Anum_pg_attribute_atthasdef - 1] = true;
+   valuesAtt[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(true);
+   replacesAtt[Anum_pg_attribute_atthasdef - 1] = true;
 
-       atttup = heap_modify_tuple(atttup, RelationGetDescr(attrrel),
-                                  valuesAtt, nullsAtt, replacesAtt);
+   atttup = heap_modify_tuple(atttup, RelationGetDescr(attrrel),
+                              valuesAtt, nullsAtt, replacesAtt);
+
+   CatalogTupleUpdate(attrrel, &atttup->t_self, atttup);
 
-       CatalogTupleUpdate(attrrel, &atttup->t_self, atttup);
-   }
    table_close(attrrel, RowExclusiveLock);
    heap_freetuple(atttup);
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7b7b09baaa8606510ec3184e5116848cb5d6acd1..13156391241eacc6dc36ca58446ee8f73ae64dea 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -941,10 +941,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
     * while raw defaults go into a list of RawColumnDefault structs that will
     * be processed by AddRelationNewConstraints.  (We can't deal with raw
     * expressions until we can do transformExpr.)
-    *
-    * We can set the atthasdef flags now in the tuple descriptor; this just
-    * saves StoreAttrDefault from having to do an immediate update of the
-    * pg_attribute rows.
     */
    rawDefaults = NIL;
    cookedDefaults = NIL;
@@ -953,11 +949,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
    foreach(listptr, stmt->tableElts)
    {
        ColumnDef  *colDef = lfirst(listptr);
-       Form_pg_attribute attr;
 
        attnum++;
-       attr = TupleDescAttr(descriptor, attnum - 1);
-
        if (colDef->raw_default != NULL)
        {
            RawColumnDefault *rawEnt;
@@ -969,7 +962,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
            rawEnt->raw_default = colDef->raw_default;
            rawEnt->generated = colDef->generated;
            rawDefaults = lappend(rawDefaults, rawEnt);
-           attr->atthasdef = true;
        }
        else if (colDef->cooked_default != NULL)
        {
@@ -987,10 +979,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
            cooked->inhcount = 0;   /* ditto */
            cooked->is_no_inherit = false;
            cookedDefaults = lappend(cookedDefaults, cooked);
-           attr->atthasdef = true;
        }
-
-       populate_compact_attribute(descriptor, attnum - 1);
    }
 
    /*
@@ -7363,8 +7352,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
     * and the later subcommands had been issued in new ALTER TABLE commands.
     *
     * We can skip this entirely for relations without storage, since Phase 3
-    * is certainly not going to touch them.  System attributes don't have
-    * interesting defaults, either.
+    * is certainly not going to touch them.
     */
    if (RELKIND_HAS_STORAGE(relkind))
    {