The MergeAttributes() and related code in and around tablecmds.c is huge and ancient, with many things bolted on over time, and difficult to deal with. I took some time to make careful incremental updates and refactorings to make the code easier to follow, more compact, and more modern in appearance. I also found several pieces of obsolete code along the way. This resulted in the attached long patch series. Each patch tries to make a single change and can be considered incrementally. At the end, the code is shorter, better factored, and I hope easier to understand. There shouldn't be any change in behavior.
From 60a671aeb03293bdec65fd86f2a393c3aced6eb9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 17:10:25 +0200
Subject: [PATCH 01/17] Remove obsolete comment about OID support

---
 src/backend/catalog/heap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 2a0d82aedd..9196dcd39f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -843,9 +843,9 @@ AddNewAttributeTuples(Oid new_rel_oid,
        }
 
        /*
-        * Next we add the system attributes.  Skip OID if rel has no OIDs. Skip
-        * all for a view or type relation.  We don't bother with making 
datatype
-        * dependencies here, since presumably all these types are pinned.
+        * Next we add the system attributes.  Skip all for a view or type
+        * relation.  We don't bother with making datatype dependencies here,
+        * since presumably all these types are pinned.
         */
        if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
        {

base-commit: b381d9637030c163c3b1f8a9d3de51dfc1b4ee58
-- 
2.41.0

From e12a69675919fb026c008e37649518f3d52e2a90 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 22 Jun 2023 12:05:06 +0200
Subject: [PATCH 02/17] Remove ancient special case code for adding oid columns

The special handling of negative attribute numbers in
ATExecAddColumn() was introduced to support SET WITH OIDS (commit
6d1e361852).  But that feature doesn't exist anymore, so we can revert
to the previous, simpler version.  In passing, also remove an obsolete
comment about OID support.
---
 src/backend/commands/tablecmds.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d985278ac6..a5493705aa 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2449,8 +2449,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
 
        /*
         * Scan the parents left-to-right, and merge their attributes to form a
-        * list of inherited attributes (inhSchema).  Also check to see if we 
need
-        * to inherit an OID column.
+        * list of inherited attributes (inhSchema).
         */
        child_attno = 0;
        foreach(entry, supers)
@@ -6944,7 +6943,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        attribute.attrelid = myrelid;
        namestrcpy(&(attribute.attname), colDef->colname);
        attribute.atttypid = typeOid;
-       attribute.attstattarget = (newattnum > 0) ? -1 : 0;
+       attribute.attstattarget = -1;
        attribute.attlen = tform->typlen;
        attribute.attnum = newattnum;
        if (list_length(colDef->typeName->arrayBounds) > PG_INT16_MAX)
@@ -7067,7 +7066,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
         * is certainly not going to touch them.  System attributes don't have
         * interesting defaults, either.
         */
-       if (RELKIND_HAS_STORAGE(relkind) && attribute.attnum > 0)
+       if (RELKIND_HAS_STORAGE(relkind))
        {
                /*
                 * For an identity column, we can't use build_column_default(),
-- 
2.41.0

From 58af867b3e3990e787a33c5d5023753e623dffe0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 14:43:55 +0200
Subject: [PATCH 03/17] Remove ancient special case code for dropping oid
 columns

The special handling of negative attribute numbers in
RemoveAttributeById() was introduced to support SET WITHOUT OIDS
(commit 24614a9880).  But that feature doesn't exist anymore, so we
can revert to the previous, simpler version.
---
 src/backend/catalog/heap.c | 99 +++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 56 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 9196dcd39f..4c30c7d461 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1666,68 +1666,56 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
                         attnum, relid);
        attStruct = (Form_pg_attribute) GETSTRUCT(tuple);
 
-       if (attnum < 0)
-       {
-               /* System attribute (probably OID) ... just delete the row */
-
-               CatalogTupleDelete(attr_rel, &tuple->t_self);
-       }
-       else
-       {
-               /* Dropping user attributes is lots harder */
+       /* Mark the attribute as dropped */
+       attStruct->attisdropped = true;
 
-               /* Mark the attribute as dropped */
-               attStruct->attisdropped = true;
-
-               /*
-                * Set the type OID to invalid.  A dropped attribute's type link
-                * cannot be relied on (once the attribute is dropped, the type 
might
-                * be too). Fortunately we do not need the type row --- the only
-                * really essential information is the type's typlen and 
typalign,
-                * which are preserved in the attribute's attlen and attalign.  
We set
-                * atttypid to zero here as a means of catching code that 
incorrectly
-                * expects it to be valid.
-                */
-               attStruct->atttypid = InvalidOid;
-
-               /* Remove any NOT NULL constraint the column may have */
-               attStruct->attnotnull = false;
+       /*
+        * Set the type OID to invalid.  A dropped attribute's type link cannot 
be
+        * relied on (once the attribute is dropped, the type might be too).
+        * Fortunately we do not need the type row --- the only really essential
+        * information is the type's typlen and typalign, which are preserved in
+        * the attribute's attlen and attalign.  We set atttypid to zero here 
as a
+        * means of catching code that incorrectly expects it to be valid.
+        */
+       attStruct->atttypid = InvalidOid;
 
-               /* We don't want to keep stats for it anymore */
-               attStruct->attstattarget = 0;
+       /* Remove any NOT NULL constraint the column may have */
+       attStruct->attnotnull = false;
 
-               /* Unset this so no one tries to look up the generation 
expression */
-               attStruct->attgenerated = '\0';
+       /* We don't want to keep stats for it anymore */
+       attStruct->attstattarget = 0;
 
-               /*
-                * Change the column name to something that isn't likely to 
conflict
-                */
-               snprintf(newattname, sizeof(newattname),
-                                "........pg.dropped.%d........", attnum);
-               namestrcpy(&(attStruct->attname), newattname);
+       /* Unset this so no one tries to look up the generation expression */
+       attStruct->attgenerated = '\0';
 
-               /* clear the missing value if any */
-               if (attStruct->atthasmissing)
-               {
-                       Datum           valuesAtt[Natts_pg_attribute] = {0};
-                       bool            nullsAtt[Natts_pg_attribute] = {0};
-                       bool            replacesAtt[Natts_pg_attribute] = {0};
-
-                       /* update the tuple - set atthasmissing and 
attmissingval */
-                       valuesAtt[Anum_pg_attribute_atthasmissing - 1] =
-                               BoolGetDatum(false);
-                       replacesAtt[Anum_pg_attribute_atthasmissing - 1] = true;
-                       valuesAtt[Anum_pg_attribute_attmissingval - 1] = 
(Datum) 0;
-                       nullsAtt[Anum_pg_attribute_attmissingval - 1] = true;
-                       replacesAtt[Anum_pg_attribute_attmissingval - 1] = true;
-
-                       tuple = heap_modify_tuple(tuple, 
RelationGetDescr(attr_rel),
-                                                                         
valuesAtt, nullsAtt, replacesAtt);
-               }
+       /*
+        * Change the column name to something that isn't likely to conflict
+        */
+       snprintf(newattname, sizeof(newattname),
+                        "........pg.dropped.%d........", attnum);
+       namestrcpy(&(attStruct->attname), newattname);
 
-               CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
+       /* clear the missing value if any */
+       if (attStruct->atthasmissing)
+       {
+               Datum           valuesAtt[Natts_pg_attribute] = {0};
+               bool            nullsAtt[Natts_pg_attribute] = {0};
+               bool            replacesAtt[Natts_pg_attribute] = {0};
+
+               /* update the tuple - set atthasmissing and attmissingval */
+               valuesAtt[Anum_pg_attribute_atthasmissing - 1] =
+                       BoolGetDatum(false);
+               replacesAtt[Anum_pg_attribute_atthasmissing - 1] = true;
+               valuesAtt[Anum_pg_attribute_attmissingval - 1] = (Datum) 0;
+               nullsAtt[Anum_pg_attribute_attmissingval - 1] = true;
+               replacesAtt[Anum_pg_attribute_attmissingval - 1] = true;
+
+               tuple = heap_modify_tuple(tuple, RelationGetDescr(attr_rel),
+                                                                 valuesAtt, 
nullsAtt, replacesAtt);
        }
 
+       CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
+
        /*
         * Because updating the pg_attribute row will trigger a relcache flush 
for
         * the target relation, we need not do anything else to notify other
@@ -1736,8 +1724,7 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 
        table_close(attr_rel, RowExclusiveLock);
 
-       if (attnum > 0)
-               RemoveStatistics(relid, attnum);
+       RemoveStatistics(relid, attnum);
 
        relation_close(rel, NoLock);
 }
-- 
2.41.0

From 2720347760b843ed6dc870270df0e8b43a1aab93 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 22 Jun 2023 14:17:34 +0200
Subject: [PATCH 04/17] Make more use of makeColumnDef()

Since we already have it, we might as well make full use of it,
instead of assembling ColumnDef by hand in several places.
---
 src/backend/commands/sequence.c    | 29 +++++++----------------
 src/backend/commands/tablecmds.c   | 14 +----------
 src/backend/parser/parse_utilcmd.c | 37 ++++++------------------------
 3 files changed, 16 insertions(+), 64 deletions(-)

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index ef01449678..c6fea33676 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -172,40 +172,27 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
        stmt->tableElts = NIL;
        for (i = SEQ_COL_FIRSTCOL; i <= SEQ_COL_LASTCOL; i++)
        {
-               ColumnDef  *coldef = makeNode(ColumnDef);
-
-               coldef->inhcount = 0;
-               coldef->is_local = true;
-               coldef->is_not_null = true;
-               coldef->is_from_type = false;
-               coldef->storage = 0;
-               coldef->raw_default = NULL;
-               coldef->cooked_default = NULL;
-               coldef->collClause = NULL;
-               coldef->collOid = InvalidOid;
-               coldef->constraints = NIL;
-               coldef->location = -1;
-
-               null[i - 1] = false;
+               ColumnDef  *coldef;
 
                switch (i)
                {
                        case SEQ_COL_LASTVAL:
-                               coldef->typeName = makeTypeNameFromOid(INT8OID, 
-1);
-                               coldef->colname = "last_value";
+                               coldef = makeColumnDef("last_value", INT8OID, 
-1, InvalidOid);
                                value[i - 1] = 
Int64GetDatumFast(seqdataform.last_value);
                                break;
                        case SEQ_COL_LOG:
-                               coldef->typeName = makeTypeNameFromOid(INT8OID, 
-1);
-                               coldef->colname = "log_cnt";
+                               coldef = makeColumnDef("log_cnt", INT8OID, -1, 
InvalidOid);
                                value[i - 1] = Int64GetDatum((int64) 0);
                                break;
                        case SEQ_COL_CALLED:
-                               coldef->typeName = makeTypeNameFromOid(BOOLOID, 
-1);
-                               coldef->colname = "is_called";
+                               coldef = makeColumnDef("is_called", BOOLOID, 
-1, InvalidOid);
                                value[i - 1] = BoolGetDatum(false);
                                break;
                }
+
+               coldef->is_not_null = true;
+               null[i - 1] = false;
+
                stmt->tableElts = lappend(stmt->tableElts, coldef);
        }
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a5493705aa..a6482a6d72 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2677,27 +2677,15 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                /*
                                 * No, create a new inherited column
                                 */
-                               def = makeNode(ColumnDef);
-                               def->colname = pstrdup(attributeName);
-                               def->typeName = 
makeTypeNameFromOid(attribute->atttypid,
-                                                                               
                        attribute->atttypmod);
+                               def = makeColumnDef(attributeName, 
attribute->atttypid, attribute->atttypmod, attribute->attcollation);
                                def->inhcount = 1;
                                def->is_local = false;
                                def->is_not_null = attribute->attnotnull;
-                               def->is_from_type = false;
                                def->storage = attribute->attstorage;
-                               def->raw_default = NULL;
-                               def->cooked_default = NULL;
                                def->generated = attribute->attgenerated;
-                               def->collClause = NULL;
-                               def->collOid = attribute->attcollation;
-                               def->constraints = NIL;
-                               def->location = -1;
                                if 
(CompressionMethodIsValid(attribute->attcompression))
                                        def->compression =
                                                
pstrdup(GetCompressionMethodName(attribute->attcompression));
-                               else
-                                       def->compression = NULL;
                                inhSchema = lappend(inhSchema, def);
                                newattmap->attnums[parent_attno - 1] = 
++child_attno;
                        }
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index d67580fc77..53420306ec 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1019,7 +1019,6 @@ transformTableLikeClause(CreateStmtContext *cxt, 
TableLikeClause *table_like_cla
        {
                Form_pg_attribute attribute = TupleDescAttr(tupleDesc,
                                                                                
                        parent_attno - 1);
-               char       *attributeName = NameStr(attribute->attname);
                ColumnDef  *def;
 
                /*
@@ -1029,26 +1028,15 @@ transformTableLikeClause(CreateStmtContext *cxt, 
TableLikeClause *table_like_cla
                        continue;
 
                /*
-                * Create a new column, which is marked as NOT inherited.
-                *
+                * Create a new column definition
+                */
+               def = makeColumnDef(NameStr(attribute->attname), 
attribute->atttypid, attribute->atttypmod, attribute->attcollation);
+
+               /*
                 * For constraints, ONLY the NOT NULL constraint is inherited 
by the
                 * new column definition per SQL99.
                 */
-               def = makeNode(ColumnDef);
-               def->colname = pstrdup(attributeName);
-               def->typeName = makeTypeNameFromOid(attribute->atttypid,
-                                                                               
        attribute->atttypmod);
-               def->inhcount = 0;
-               def->is_local = true;
                def->is_not_null = attribute->attnotnull;
-               def->is_from_type = false;
-               def->storage = 0;
-               def->raw_default = NULL;
-               def->cooked_default = NULL;
-               def->collClause = NULL;
-               def->collOid = attribute->attcollation;
-               def->constraints = NIL;
-               def->location = -1;
 
                /*
                 * Add to column list
@@ -1476,20 +1464,9 @@ transformOfType(CreateStmtContext *cxt, TypeName 
*ofTypename)
                if (attr->attisdropped)
                        continue;
 
-               n = makeNode(ColumnDef);
-               n->colname = pstrdup(NameStr(attr->attname));
-               n->typeName = makeTypeNameFromOid(attr->atttypid, 
attr->atttypmod);
-               n->inhcount = 0;
-               n->is_local = true;
-               n->is_not_null = false;
+               n = makeColumnDef(NameStr(attr->attname), attr->atttypid, 
attr->atttypmod, attr->attcollation);
                n->is_from_type = true;
-               n->storage = 0;
-               n->raw_default = NULL;
-               n->cooked_default = NULL;
-               n->collClause = NULL;
-               n->collOid = attr->attcollation;
-               n->constraints = NIL;
-               n->location = -1;
+
                cxt->columns = lappend(cxt->columns, n);
        }
        ReleaseTupleDesc(tupdesc);
-- 
2.41.0

From fcdd42c9507bd048afd5443f74b1b8bad31b7595 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Jun 2023 13:13:12 +0200
Subject: [PATCH 05/17] Clean up MergeAttributesIntoExisting()

Make variable naming clearer and more consistent.  Move some variables
to smaller scope.  Remove some unnecessary intermediate variables.
Try to save some vertical space.

Apply analogous changes to nearby MergeConstraintsIntoExisting() and
RemoveInheritance() for consistency.
---
 src/backend/commands/tablecmds.c | 115 +++++++++++--------------------
 1 file changed, 42 insertions(+), 73 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a6482a6d72..62b555aa20 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15112,84 +15112,67 @@ static void
 MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
 {
        Relation        attrrel;
-       AttrNumber      parent_attno;
-       int                     parent_natts;
-       TupleDesc       tupleDesc;
-       HeapTuple       tuple;
-       bool            child_is_partition = false;
+       TupleDesc       parent_desc;
 
        attrrel = table_open(AttributeRelationId, RowExclusiveLock);
+       parent_desc = RelationGetDescr(parent_rel);
 
-       tupleDesc = RelationGetDescr(parent_rel);
-       parent_natts = tupleDesc->natts;
-
-       /* If parent_rel is a partitioned table, child_rel must be a partition 
*/
-       if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-               child_is_partition = true;
-
-       for (parent_attno = 1; parent_attno <= parent_natts; parent_attno++)
+       for (AttrNumber parent_attno = 1; parent_attno <= parent_desc->natts; 
parent_attno++)
        {
-               Form_pg_attribute attribute = TupleDescAttr(tupleDesc,
-                                                                               
                        parent_attno - 1);
-               char       *attributeName = NameStr(attribute->attname);
+               Form_pg_attribute parent_att = TupleDescAttr(parent_desc, 
parent_attno - 1);
+               char       *parent_attname = NameStr(parent_att->attname);
+               HeapTuple       tuple;
 
                /* Ignore dropped columns in the parent. */
-               if (attribute->attisdropped)
+               if (parent_att->attisdropped)
                        continue;
 
                /* Find same column in child (matching on column name). */
-               tuple = SearchSysCacheCopyAttName(RelationGetRelid(child_rel),
-                                                                               
  attributeName);
+               tuple = SearchSysCacheCopyAttName(RelationGetRelid(child_rel), 
parent_attname);
                if (HeapTupleIsValid(tuple))
                {
-                       /* Check they are same type, typmod, and collation */
-                       Form_pg_attribute childatt = (Form_pg_attribute) 
GETSTRUCT(tuple);
+                       Form_pg_attribute child_att = (Form_pg_attribute) 
GETSTRUCT(tuple);
 
-                       if (attribute->atttypid != childatt->atttypid ||
-                               attribute->atttypmod != childatt->atttypmod)
+                       if (parent_att->atttypid != child_att->atttypid ||
+                               parent_att->atttypmod != child_att->atttypmod)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                 errmsg("child table \"%s\" has 
different type for column \"%s\"",
-                                                               
RelationGetRelationName(child_rel),
-                                                               
attributeName)));
+                                                               
RelationGetRelationName(child_rel), parent_attname)));
 
-                       if (attribute->attcollation != childatt->attcollation)
+                       if (parent_att->attcollation != child_att->attcollation)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_COLLATION_MISMATCH),
                                                 errmsg("child table \"%s\" has 
different collation for column \"%s\"",
-                                                               
RelationGetRelationName(child_rel),
-                                                               
attributeName)));
+                                                               
RelationGetRelationName(child_rel), parent_attname)));
 
                        /*
                         * Check child doesn't discard NOT NULL property.  
(Other
                         * constraints are checked elsewhere.)
                         */
-                       if (attribute->attnotnull && !childatt->attnotnull)
+                       if (parent_att->attnotnull && !child_att->attnotnull)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
-                                                errmsg("column \"%s\" in child 
table must be marked NOT NULL",
-                                                               
attributeName)));
+                                                errmsg("column \"%s\" in child 
table must be marked NOT NULL", parent_attname)));
 
                        /*
                         * Child column must be generated if and only if parent 
column is.
                         */
-                       if (attribute->attgenerated && !childatt->attgenerated)
+                       if (parent_att->attgenerated && 
!child_att->attgenerated)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
-                                                errmsg("column \"%s\" in child 
table must be a generated column",
-                                                               
attributeName)));
-                       if (childatt->attgenerated && !attribute->attgenerated)
+                                                errmsg("column \"%s\" in child 
table must be a generated column", parent_attname)));
+                       if (child_att->attgenerated && 
!parent_att->attgenerated)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
-                                                errmsg("column \"%s\" in child 
table must not be a generated column",
-                                                               
attributeName)));
+                                                errmsg("column \"%s\" in child 
table must not be a generated column", parent_attname)));
 
                        /*
                         * OK, bump the child column's inheritance count.  (If 
we fail
                         * later on, this change will just roll back.)
                         */
-                       childatt->attinhcount++;
-                       if (childatt->attinhcount < 0)
+                       child_att->attinhcount++;
+                       if (child_att->attinhcount < 0)
                                ereport(ERROR,
                                                
errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                                                errmsg("too many inheritance 
parents"));
@@ -15199,10 +15182,10 @@ MergeAttributesIntoExisting(Relation child_rel, 
Relation parent_rel)
                         * is same in all partitions. (Note: there are only 
inherited
                         * attributes in partitions)
                         */
-                       if (child_is_partition)
+                       if (parent_rel->rd_rel->relkind == 
RELKIND_PARTITIONED_TABLE)
                        {
-                               Assert(childatt->attinhcount == 1);
-                               childatt->attislocal = false;
+                               Assert(child_att->attinhcount == 1);
+                               child_att->attislocal = false;
                        }
 
                        CatalogTupleUpdate(attrrel, &tuple->t_self, tuple);
@@ -15212,8 +15195,7 @@ MergeAttributesIntoExisting(Relation child_rel, 
Relation parent_rel)
                {
                        ereport(ERROR,
                                        (errcode(ERRCODE_DATATYPE_MISMATCH),
-                                        errmsg("child table is missing column 
\"%s\"",
-                                                       attributeName)));
+                                        errmsg("child table is missing column 
\"%s\"", parent_attname)));
                }
        }
 
@@ -15240,26 +15222,19 @@ MergeAttributesIntoExisting(Relation child_rel, 
Relation parent_rel)
 static void
 MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
 {
-       Relation        catalog_relation;
-       TupleDesc       tuple_desc;
+       Relation        constraintrel;
        SysScanDesc parent_scan;
        ScanKeyData parent_key;
        HeapTuple       parent_tuple;
-       bool            child_is_partition = false;
-
-       catalog_relation = table_open(ConstraintRelationId, RowExclusiveLock);
-       tuple_desc = RelationGetDescr(catalog_relation);
 
-       /* If parent_rel is a partitioned table, child_rel must be a partition 
*/
-       if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-               child_is_partition = true;
+       constraintrel = table_open(ConstraintRelationId, RowExclusiveLock);
 
        /* Outer loop scans through the parent's constraint definitions */
        ScanKeyInit(&parent_key,
                                Anum_pg_constraint_conrelid,
                                BTEqualStrategyNumber, F_OIDEQ,
                                ObjectIdGetDatum(RelationGetRelid(parent_rel)));
-       parent_scan = systable_beginscan(catalog_relation, 
ConstraintRelidTypidNameIndexId,
+       parent_scan = systable_beginscan(constraintrel, 
ConstraintRelidTypidNameIndexId,
                                                                         true, 
NULL, 1, &parent_key);
 
        while (HeapTupleIsValid(parent_tuple = systable_getnext(parent_scan)))
@@ -15282,7 +15257,7 @@ MergeConstraintsIntoExisting(Relation child_rel, 
Relation parent_rel)
                                        Anum_pg_constraint_conrelid,
                                        BTEqualStrategyNumber, F_OIDEQ,
                                        
ObjectIdGetDatum(RelationGetRelid(child_rel)));
-               child_scan = systable_beginscan(catalog_relation, 
ConstraintRelidTypidNameIndexId,
+               child_scan = systable_beginscan(constraintrel, 
ConstraintRelidTypidNameIndexId,
                                                                                
true, NULL, 1, &child_key);
 
                while (HeapTupleIsValid(child_tuple = 
systable_getnext(child_scan)))
@@ -15293,24 +15268,21 @@ MergeConstraintsIntoExisting(Relation child_rel, 
Relation parent_rel)
                        if (child_con->contype != CONSTRAINT_CHECK)
                                continue;
 
-                       if (strcmp(NameStr(parent_con->conname),
-                                          NameStr(child_con->conname)) != 0)
+                       if (strcmp(NameStr(parent_con->conname), 
NameStr(child_con->conname)) != 0)
                                continue;
 
-                       if (!constraints_equivalent(parent_tuple, child_tuple, 
tuple_desc))
+                       if (!constraints_equivalent(parent_tuple, child_tuple, 
RelationGetDescr(constraintrel)))
                                ereport(ERROR,
                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                 errmsg("child table \"%s\" has 
different definition for check constraint \"%s\"",
-                                                               
RelationGetRelationName(child_rel),
-                                                               
NameStr(parent_con->conname))));
+                                                               
RelationGetRelationName(child_rel), NameStr(parent_con->conname))));
 
                        /* If the child constraint is "no inherit" then cannot 
merge */
                        if (child_con->connoinherit)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                                                 errmsg("constraint \"%s\" 
conflicts with non-inherited constraint on child table \"%s\"",
-                                                               
NameStr(child_con->conname),
-                                                               
RelationGetRelationName(child_rel))));
+                                                               
NameStr(child_con->conname), RelationGetRelationName(child_rel))));
 
                        /*
                         * If the child constraint is "not valid" then cannot 
merge with a
@@ -15320,8 +15292,7 @@ MergeConstraintsIntoExisting(Relation child_rel, 
Relation parent_rel)
                                ereport(ERROR,
                                                
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                                                 errmsg("constraint \"%s\" 
conflicts with NOT VALID constraint on child table \"%s\"",
-                                                               
NameStr(child_con->conname),
-                                                               
RelationGetRelationName(child_rel))));
+                                                               
NameStr(child_con->conname), RelationGetRelationName(child_rel))));
 
                        /*
                         * OK, bump the child constraint's inheritance count.  
(If we fail
@@ -15340,13 +15311,13 @@ MergeConstraintsIntoExisting(Relation child_rel, 
Relation parent_rel)
                         * inherited only once since it cannot have multiple 
parents and
                         * it is never considered local.
                         */
-                       if (child_is_partition)
+                       if (parent_rel->rd_rel->relkind == 
RELKIND_PARTITIONED_TABLE)
                        {
                                Assert(child_con->coninhcount == 1);
                                child_con->conislocal = false;
                        }
 
-                       CatalogTupleUpdate(catalog_relation, 
&child_copy->t_self, child_copy);
+                       CatalogTupleUpdate(constraintrel, &child_copy->t_self, 
child_copy);
                        heap_freetuple(child_copy);
 
                        found = true;
@@ -15363,7 +15334,7 @@ MergeConstraintsIntoExisting(Relation child_rel, 
Relation parent_rel)
        }
 
        systable_endscan(parent_scan);
-       table_close(catalog_relation, RowExclusiveLock);
+       table_close(constraintrel, RowExclusiveLock);
 }
 
 /*
@@ -15506,11 +15477,9 @@ RemoveInheritance(Relation child_rel, Relation 
parent_rel, bool expect_detached)
                                constraintTuple;
        List       *connames;
        bool            found;
-       bool            child_is_partition = false;
+       bool            is_partitioning;
 
-       /* If parent_rel is a partitioned table, child_rel must be a partition 
*/
-       if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-               child_is_partition = true;
+       is_partitioning = (parent_rel->rd_rel->relkind == 
RELKIND_PARTITIONED_TABLE);
 
        found = DeleteInheritsTuple(RelationGetRelid(child_rel),
                                                                
RelationGetRelid(parent_rel),
@@ -15518,7 +15487,7 @@ RemoveInheritance(Relation child_rel, Relation 
parent_rel, bool expect_detached)
                                                                
RelationGetRelationName(child_rel));
        if (!found)
        {
-               if (child_is_partition)
+               if (is_partitioning)
                        ereport(ERROR,
                                        (errcode(ERRCODE_UNDEFINED_TABLE),
                                         errmsg("relation \"%s\" is not a 
partition of relation \"%s\"",
@@ -15648,7 +15617,7 @@ RemoveInheritance(Relation child_rel, Relation 
parent_rel, bool expect_detached)
        drop_parent_dependency(RelationGetRelid(child_rel),
                                                   RelationRelationId,
                                                   RelationGetRelid(parent_rel),
-                                                  
child_dependency_type(child_is_partition));
+                                                  
child_dependency_type(is_partitioning));
 
        /*
         * Post alter hook of this inherits. Since object_access_hook doesn't 
take
-- 
2.41.0

From 17517897f90eb7b2be100dade5579b55d406334e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Jun 2023 13:34:07 +0200
Subject: [PATCH 06/17] Clean up MergeCheckConstraint()

If the constraint is not already in the list, add it ourselves,
instead of making the caller do it.  This makes the interface more
consistent with other "merge" functions in this file.
---
 src/backend/commands/tablecmds.c | 47 ++++++++++++++------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 62b555aa20..a905140e32 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -351,7 +351,7 @@ static void RangeVarCallbackForTruncate(const RangeVar 
*relation,
                                                                                
Oid relId, Oid oldRelId, void *arg);
 static List *MergeAttributes(List *schema, List *supers, char relpersistence,
                                                         bool is_partition, 
List **supconstr);
-static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
+static List *MergeCheckConstraint(List *constraints, const char *name, Node 
*expr);
 static void MergeAttributesIntoExisting(Relation child_rel, Relation 
parent_rel);
 static void MergeConstraintsIntoExisting(Relation child_rel, Relation 
parent_rel);
 static void StoreCatalogInheritance(Oid relationId, List *supers,
@@ -2811,24 +2811,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                                                           name,
                                                                           
RelationGetRelationName(relation))));
 
-                               /* check for duplicate */
-                               if (!MergeCheckConstraint(constraints, name, 
expr))
-                               {
-                                       /* nope, this is a new one */
-                                       CookedConstraint *cooked;
-
-                                       cooked = (CookedConstraint *) 
palloc(sizeof(CookedConstraint));
-                                       cooked->contype = CONSTR_CHECK;
-                                       cooked->conoid = InvalidOid;    /* 
until created */
-                                       cooked->name = pstrdup(name);
-                                       cooked->attnum = 0; /* not used for 
constraints */
-                                       cooked->expr = expr;
-                                       cooked->skip_validation = false;
-                                       cooked->is_local = false;
-                                       cooked->inhcount = 1;
-                                       cooked->is_no_inherit = false;
-                                       constraints = lappend(constraints, 
cooked);
-                               }
+                               constraints = MergeCheckConstraint(constraints, 
name, expr);
                        }
                }
 
@@ -3158,13 +3141,16 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
  *
  * constraints is a list of CookedConstraint structs for previous constraints.
  *
- * Returns true if merged (constraint is a duplicate), or false if it's
- * got a so-far-unique name, or throws error if conflict.
+ * If the constraint is a duplicate, then the existing constraint's
+ * inheritance count is updated.  If the constraint doesn't match or conflict
+ * with an existing one, a new constraint is appended to the list.  If there
+ * is a conflict (same name but different expression), throw an error.
  */
-static bool
-MergeCheckConstraint(List *constraints, char *name, Node *expr)
+static List *
+MergeCheckConstraint(List *constraints, const char *name, Node *expr)
 {
        ListCell   *lc;
+       CookedConstraint *newcon;
 
        foreach(lc, constraints)
        {
@@ -3178,13 +3164,13 @@ MergeCheckConstraint(List *constraints, char *name, 
Node *expr)
 
                if (equal(expr, ccon->expr))
                {
-                       /* OK to merge */
+                       /* OK to merge constraint with existing */
                        ccon->inhcount++;
                        if (ccon->inhcount < 0)
                                ereport(ERROR,
                                                
errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                                                errmsg("too many inheritance 
parents"));
-                       return true;
+                       return constraints;
                }
 
                ereport(ERROR,
@@ -3193,7 +3179,16 @@ MergeCheckConstraint(List *constraints, char *name, Node 
*expr)
                                                name)));
        }
 
-       return false;
+       /*
+        * Constraint couldn't be merged with an existing one and also didn't
+        * conflict with an existing one, so add it as a new one to the list.
+        */
+       newcon = palloc0_object(CookedConstraint);
+       newcon->contype = CONSTR_CHECK;
+       newcon->name = pstrdup(name);
+       newcon->expr = expr;
+       newcon->inhcount = 1;
+       return lappend(constraints, newcon);
 }
 
 
-- 
2.41.0

From 6987b295c86205536cba75ecd9d399c740ce64bb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Jun 2023 14:10:12 +0200
Subject: [PATCH 07/17] MergeAttributes() and related variable renaming

Mainly, rename "schema" to "columns" and related changes.  The
previous naming has long been confusing.
---
 src/backend/access/common/tupdesc.c |  10 +--
 src/backend/commands/tablecmds.c    | 109 +++++++++++++---------------
 src/include/access/tupdesc.h        |   4 +-
 3 files changed, 59 insertions(+), 64 deletions(-)

diff --git a/src/backend/access/common/tupdesc.c 
b/src/backend/access/common/tupdesc.c
index 7c5c390503..253d6c86f8 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -782,12 +782,12 @@ TupleDescInitEntryCollation(TupleDesc desc,
 /*
  * BuildDescForRelation
  *
- * Given a relation schema (list of ColumnDef nodes), build a TupleDesc.
+ * Given a list of ColumnDef nodes, build a TupleDesc.
  *
  * Note: tdtypeid will need to be filled in later on.
  */
 TupleDesc
-BuildDescForRelation(List *schema)
+BuildDescForRelation(const List *columns)
 {
        int                     natts;
        AttrNumber      attnum;
@@ -803,13 +803,13 @@ BuildDescForRelation(List *schema)
        /*
         * allocate a new tuple descriptor
         */
-       natts = list_length(schema);
+       natts = list_length(columns);
        desc = CreateTemplateTupleDesc(natts);
        has_not_null = false;
 
        attnum = 0;
 
-       foreach(l, schema)
+       foreach(l, columns)
        {
                ColumnDef  *entry = lfirst(l);
                AclResult       aclresult;
@@ -891,7 +891,7 @@ BuildDescForRelation(List *schema)
  * with functions returning RECORD.
  */
 TupleDesc
-BuildDescFromLists(List *names, List *types, List *typmods, List *collations)
+BuildDescFromLists(const List *names, const List *types, const List *typmods, 
const List *collations)
 {
        int                     natts;
        AttrNumber      attnum;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a905140e32..031b2dc423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -349,7 +349,7 @@ static void truncate_check_perms(Oid relid, Form_pg_class 
reltuple);
 static void truncate_check_activity(Relation rel);
 static void RangeVarCallbackForTruncate(const RangeVar *relation,
                                                                                
Oid relId, Oid oldRelId, void *arg);
-static List *MergeAttributes(List *schema, List *supers, char relpersistence,
+static List *MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                                         bool is_partition, 
List **supconstr);
 static List *MergeCheckConstraint(List *constraints, const char *name, Node 
*expr);
 static void MergeAttributesIntoExisting(Relation child_rel, Relation 
parent_rel);
@@ -359,7 +359,7 @@ static void StoreCatalogInheritance(Oid relationId, List 
*supers,
 static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
                                                                         int32 
seqNumber, Relation inhRelation,
                                                                         bool 
child_is_partition);
-static int     findAttrByName(const char *attributeName, List *schema);
+static int     findAttrByName(const char *attributeName, const List *columns);
 static void AlterIndexNamespaces(Relation classRel, Relation rel,
                                                                 Oid oldNspOid, 
Oid newNspOid, ObjectAddresses *objsMoved);
 static void AlterSeqNamespaces(Relation classRel, Relation rel,
@@ -2289,7 +2289,7 @@ storage_name(char c)
  *             Returns new schema given initial schema and superclasses.
  *
  * Input arguments:
- * 'schema' is the column/attribute definition for the table. (It's a list
+ * 'columns' is the column/attribute definition for the table. (It's a list
  *             of ColumnDef's.) It is destructively changed.
  * 'supers' is a list of OIDs of parent relations, already locked by caller.
  * 'relpersistence' is the persistence type of the table.
@@ -2346,16 +2346,16 @@ storage_name(char c)
  *----------
  */
 static List *
-MergeAttributes(List *schema, List *supers, char relpersistence,
+MergeAttributes(List *columns, const List *supers, char relpersistence,
                                bool is_partition, List **supconstr)
 {
-       List       *inhSchema = NIL;
+       List       *inh_columns = NIL;
        List       *constraints = NIL;
        bool            have_bogus_defaults = false;
        int                     child_attno;
        static Node bogus_marker = {0}; /* marks conflicting defaults */
-       List       *saved_schema = NIL;
-       ListCell   *entry;
+       List       *saved_columns = NIL;
+       ListCell   *lc;
 
        /*
         * Check for and reject tables with too many columns. We perform this
@@ -2368,7 +2368,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
         * Note that we also need to check that we do not exceed this figure 
after
         * including columns from inherited relations.
         */
-       if (list_length(schema) > MaxHeapAttributeNumber)
+       if (list_length(columns) > MaxHeapAttributeNumber)
                ereport(ERROR,
                                (errcode(ERRCODE_TOO_MANY_COLUMNS),
                                 errmsg("tables can have at most %d columns",
@@ -2382,15 +2382,15 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
         * sense to assume such conflicts are errors.
         *
         * We don't use foreach() here because we have two nested loops over the
-        * schema list, with possible element deletions in the inner one.  If we
+        * columns list, with possible element deletions in the inner one.  If 
we
         * used foreach_delete_current() it could only fix up the state of one 
of
         * the loops, so it seems cleaner to use looping over list indexes for
         * both loops.  Note that any deletion will happen beyond where the 
outer
         * loop is, so its index never needs adjustment.
         */
-       for (int coldefpos = 0; coldefpos < list_length(schema); coldefpos++)
+       for (int coldefpos = 0; coldefpos < list_length(columns); coldefpos++)
        {
-               ColumnDef  *coldef = list_nth_node(ColumnDef, schema, 
coldefpos);
+               ColumnDef  *coldef = list_nth_node(ColumnDef, columns, 
coldefpos);
 
                if (!is_partition && coldef->typeName == NULL)
                {
@@ -2407,9 +2407,9 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                }
 
                /* restpos scans all entries beyond coldef; incr is in loop 
body */
-               for (int restpos = coldefpos + 1; restpos < 
list_length(schema);)
+               for (int restpos = coldefpos + 1; restpos < 
list_length(columns);)
                {
-                       ColumnDef  *restdef = list_nth_node(ColumnDef, schema, 
restpos);
+                       ColumnDef  *restdef = list_nth_node(ColumnDef, columns, 
restpos);
 
                        if (strcmp(coldef->colname, restdef->colname) == 0)
                        {
@@ -2423,7 +2423,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                        coldef->cooked_default = 
restdef->cooked_default;
                                        coldef->constraints = 
restdef->constraints;
                                        coldef->is_from_type = false;
-                                       schema = list_delete_nth_cell(schema, 
restpos);
+                                       columns = list_delete_nth_cell(columns, 
restpos);
                                }
                                else
                                        ereport(ERROR,
@@ -2443,25 +2443,24 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
         */
        if (is_partition)
        {
-               saved_schema = schema;
-               schema = NIL;
+               saved_columns = columns;
+               columns = NIL;
        }
 
        /*
         * Scan the parents left-to-right, and merge their attributes to form a
-        * list of inherited attributes (inhSchema).
+        * list of inherited columns (inh_columns).
         */
        child_attno = 0;
-       foreach(entry, supers)
+       foreach(lc, supers)
        {
-               Oid                     parent = lfirst_oid(entry);
+               Oid                     parent = lfirst_oid(lc);
                Relation        relation;
                TupleDesc       tupleDesc;
                TupleConstr *constr;
                AttrMap    *newattmap;
                List       *inherited_defaults;
                List       *cols_with_defaults;
-               AttrNumber      parent_attno;
                ListCell   *lc1;
                ListCell   *lc2;
 
@@ -2480,8 +2479,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                 * We do not allow partitioned tables and partitions to 
participate in
                 * regular inheritance.
                 */
-               if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
-                       !is_partition)
+               if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && 
!is_partition)
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                         errmsg("cannot inherit from 
partitioned table \"%s\"",
@@ -2552,7 +2550,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                /* We can't process inherited defaults until newattmap is 
complete. */
                inherited_defaults = cols_with_defaults = NIL;
 
-               for (parent_attno = 1; parent_attno <= tupleDesc->natts;
+               for (AttrNumber parent_attno = 1; parent_attno <= 
tupleDesc->natts;
                         parent_attno++)
                {
                        Form_pg_attribute attribute = TupleDescAttr(tupleDesc,
@@ -2570,7 +2568,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                        /*
                         * Does it conflict with some previously inherited 
column?
                         */
-                       exist_attno = findAttrByName(attributeName, inhSchema);
+                       exist_attno = findAttrByName(attributeName, 
inh_columns);
                        if (exist_attno > 0)
                        {
                                Oid                     defTypeId;
@@ -2583,7 +2581,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                ereport(NOTICE,
                                                (errmsg("merging multiple 
inherited definitions of column \"%s\"",
                                                                
attributeName)));
-                               def = (ColumnDef *) list_nth(inhSchema, 
exist_attno - 1);
+                               def = (ColumnDef *) list_nth(inh_columns, 
exist_attno - 1);
 
                                /*
                                 * Must have the same type and typmod
@@ -2686,7 +2684,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                if 
(CompressionMethodIsValid(attribute->attcompression))
                                        def->compression =
                                                
pstrdup(GetCompressionMethodName(attribute->attcompression));
-                               inhSchema = lappend(inhSchema, def);
+                               inh_columns = lappend(inh_columns, def);
                                newattmap->attnums[parent_attno - 1] = 
++child_attno;
                        }
 
@@ -2760,7 +2758,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                         * If we already had a default from some prior parent, 
check to
                         * see if they are the same.  If so, no problem; if 
not, mark the
                         * column as having a bogus default.  Below, we will 
complain if
-                        * the bogus default isn't overridden by the child 
schema.
+                        * the bogus default isn't overridden by the child 
columns.
                         */
                        Assert(def->raw_default == NULL);
                        if (def->cooked_default == NULL)
@@ -2780,9 +2778,8 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                if (constr && constr->num_check > 0)
                {
                        ConstrCheck *check = constr->check;
-                       int                     i;
 
-                       for (i = 0; i < constr->num_check; i++)
+                       for (int i = 0; i < constr->num_check; i++)
                        {
                                char       *name = check[i].ccname;
                                Node       *expr;
@@ -2826,27 +2823,27 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
        }
 
        /*
-        * If we had no inherited attributes, the result schema is just the
+        * If we had no inherited attributes, the result columns are just the
         * explicitly declared columns.  Otherwise, we need to merge the 
declared
-        * columns into the inherited schema list.  Although, we never have any
+        * columns into the inherited column list.  Although, we never have any
         * explicitly declared columns if the table is a partition.
         */
-       if (inhSchema != NIL)
+       if (inh_columns != NIL)
        {
-               int                     schema_attno = 0;
+               int                     newcol_attno = 0;
 
-               foreach(entry, schema)
+               foreach(lc, columns)
                {
-                       ColumnDef  *newdef = lfirst(entry);
+                       ColumnDef  *newdef = lfirst(lc);
                        char       *attributeName = newdef->colname;
                        int                     exist_attno;
 
-                       schema_attno++;
+                       newcol_attno++;
 
                        /*
                         * Does it conflict with some previously inherited 
column?
                         */
-                       exist_attno = findAttrByName(attributeName, inhSchema);
+                       exist_attno = findAttrByName(attributeName, 
inh_columns);
                        if (exist_attno > 0)
                        {
                                ColumnDef  *def;
@@ -2866,7 +2863,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                /*
                                 * Yes, try to merge the two column definitions.
                                 */
-                               if (exist_attno == schema_attno)
+                               if (exist_attno == newcol_attno)
                                        ereport(NOTICE,
                                                        (errmsg("merging column 
\"%s\" with inherited definition",
                                                                        
attributeName)));
@@ -2874,7 +2871,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                                        ereport(NOTICE,
                                                        (errmsg("moving and 
merging column \"%s\" with inherited definition", attributeName),
                                                         
errdetail("User-specified column moved to the position of the inherited 
column.")));
-                               def = (ColumnDef *) list_nth(inhSchema, 
exist_attno - 1);
+                               def = (ColumnDef *) list_nth(inh_columns, 
exist_attno - 1);
 
                                /*
                                 * Must have the same type and typmod
@@ -2999,19 +2996,19 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                        else
                        {
                                /*
-                                * No, attach new column to result schema
+                                * No, attach new column to result columns
                                 */
-                               inhSchema = lappend(inhSchema, newdef);
+                               inh_columns = lappend(inh_columns, newdef);
                        }
                }
 
-               schema = inhSchema;
+               columns = inh_columns;
 
                /*
                 * Check that we haven't exceeded the legal # of columns after 
merging
                 * in inherited columns.
                 */
-               if (list_length(schema) > MaxHeapAttributeNumber)
+               if (list_length(columns) > MaxHeapAttributeNumber)
                        ereport(ERROR,
                                        (errcode(ERRCODE_TOO_MANY_COLUMNS),
                                         errmsg("tables can have at most %d 
columns",
@@ -3026,13 +3023,13 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
         */
        if (is_partition)
        {
-               foreach(entry, saved_schema)
+               foreach(lc, saved_columns)
                {
-                       ColumnDef  *restdef = lfirst(entry);
+                       ColumnDef  *restdef = lfirst(lc);
                        bool            found = false;
                        ListCell   *l;
 
-                       foreach(l, schema)
+                       foreach(l, columns)
                        {
                                ColumnDef  *coldef = lfirst(l);
 
@@ -3105,9 +3102,9 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
         */
        if (have_bogus_defaults)
        {
-               foreach(entry, schema)
+               foreach(lc, columns)
                {
-                       ColumnDef  *def = lfirst(entry);
+                       ColumnDef  *def = lfirst(lc);
 
                        if (def->cooked_default == &bogus_marker)
                        {
@@ -3128,7 +3125,7 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
        }
 
        *supconstr = constraints;
-       return schema;
+       return columns;
 }
 
 
@@ -3282,22 +3279,20 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
 }
 
 /*
- * Look for an existing schema entry with the given name.
+ * Look for an existing column entry with the given name.
  *
- * Returns the index (starting with 1) if attribute already exists in schema,
+ * Returns the index (starting with 1) if attribute already exists in columns,
  * 0 if it doesn't.
  */
 static int
-findAttrByName(const char *attributeName, List *schema)
+findAttrByName(const char *attributeName, const List *columns)
 {
-       ListCell   *s;
+       ListCell   *lc;
        int                     i = 1;
 
-       foreach(s, schema)
+       foreach(lc, columns)
        {
-               ColumnDef  *def = lfirst(s);
-
-               if (strcmp(attributeName, def->colname) == 0)
+               if (strcmp(attributeName, lfirst_node(ColumnDef, lc)->colname) 
== 0)
                        return i;
 
                i++;
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index b4286cf922..f6cc28a661 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -147,8 +147,8 @@ extern void TupleDescInitEntryCollation(TupleDesc desc,
                                                                                
AttrNumber attributeNumber,
                                                                                
Oid collationid);
 
-extern TupleDesc BuildDescForRelation(List *schema);
+extern TupleDesc BuildDescForRelation(const List *columns);
 
-extern TupleDesc BuildDescFromLists(List *names, List *types, List *typmods, 
List *collations);
+extern TupleDesc BuildDescFromLists(const List *names, const List *types, 
const List *typmods, const List *collations);
 
 #endif                                                 /* TUPDESC_H */
-- 
2.41.0

From 255e6a3e40ef8ad0194f5b2c233e4a60bea4de1d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 09:55:56 +0200
Subject: [PATCH 08/17] Improve some catalog documentation

Point out that typstorage and attstorage are never '\0', even for
fixed-length types.  This is different from attcompression.  For this
reason, some of the handling of these columns in tablecmds.c etc. is
different.  (catalogs.sgml already contained this information in an
indirect way.)
---
 src/include/catalog/pg_attribute.h | 10 +++++-----
 src/include/catalog/pg_type.h      |  3 +++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/include/catalog/pg_attribute.h 
b/src/include/catalog/pg_attribute.h
index f8b4861b94..a821bb1665 100644
--- a/src/include/catalog/pg_attribute.h
+++ b/src/include/catalog/pg_attribute.h
@@ -108,11 +108,11 @@ CATALOG(pg_attribute,1249,AttributeRelationId) 
BKI_BOOTSTRAP BKI_ROWTYPE_OID(75,
         */
        char            attalign;
 
-       /*----------
-        * attstorage tells for VARLENA attributes, what the heap access
-        * methods can do to it if a given tuple doesn't fit into a page.
-        * Possible values are as for pg_type.typstorage (see TYPSTORAGE 
macros).
-        *----------
+       /*
+        * attstorage tells for VARLENA attributes, what the heap access methods
+        * can do to it if a given tuple doesn't fit into a page.  Possible 
values
+        * are as for pg_type.typstorage (see TYPSTORAGE macros).  This is never
+        * '\0', even for fixed-length types.
         */
        char            attstorage;
 
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index 519e570c8c..e0a86354ff 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -187,6 +187,9 @@ CATALOG(pg_type,1247,TypeRelationId) BKI_BOOTSTRAP 
BKI_ROWTYPE_OID(71,TypeRelati
         *
         * Note that 'm' fields can also be moved out to secondary storage,
         * but only as a last resort ('e' and 'x' fields are moved first).
+        *
+        * For types that are not variable-length (that is, typlen != -1), this
+        * must be set to 'p'.
         * ----------------
         */
        char            typstorage BKI_DEFAULT(p) BKI_ARRAY_DEFAULT(x);
-- 
2.41.0

From eea7f17ebddd1635c0dab90ef7e3e818300b6069 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 11:24:05 +0200
Subject: [PATCH 09/17] Remove useless if condition

This is useless because these fields are not set anywhere before, so
we can assign them unconditionally.  This also makes this more
consistent with ATExecAddColumn().
---
 src/backend/commands/tablecmds.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 031b2dc423..5400d37b5e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -927,11 +927,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                        attr->atthasdef = true;
                }
 
-               if (colDef->identity)
-                       attr->attidentity = colDef->identity;
-
-               if (colDef->generated)
-                       attr->attgenerated = colDef->generated;
+               attr->attidentity = colDef->identity;
+               attr->attgenerated = colDef->generated;
 
                if (colDef->compression)
                        attr->attcompression = 
GetAttributeCompression(attr->atttypid,
-- 
2.41.0

From bc02f8c2ec75b989c894b71b860b3e7ce2cc7029 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 11:30:05 +0200
Subject: [PATCH 10/17] Remove useless if condition

We can call GetAttributeCompression() with a NULL argument.  It
handles that internally already.  This change makes all the callers of
GetAttributeCompression() uniform.
---
 src/backend/commands/tablecmds.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5400d37b5e..4e6310886f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -929,11 +929,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 
                attr->attidentity = colDef->identity;
                attr->attgenerated = colDef->generated;
-
-               if (colDef->compression)
-                       attr->attcompression = 
GetAttributeCompression(attr->atttypid,
-                                                                               
                                   colDef->compression);
-
+               attr->attcompression = GetAttributeCompression(attr->atttypid, 
colDef->compression);
                if (colDef->storage_name)
                        attr->attstorage = GetAttributeStorage(attr->atttypid, 
colDef->storage_name);
        }
-- 
2.41.0

From 2eda6bc9897d0995a5112e2851c51daf0c35656e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 14 Jun 2023 17:51:31 +0200
Subject: [PATCH 11/17] Refactor ATExecAddColumn() to use
 BuildDescForRelation()

BuildDescForRelation() has all the knowledge for converting a
ColumnDef into pg_attribute/tuple descriptor.  ATExecAddColumn() can
make use of that, instead of duplicating all that logic.  We just pass
a one-element list of ColumnDef and we get back exactly the data
structure we need.  Note that we don't even need to touch
BuildDescForRelation() to make this work.
---
 src/backend/commands/tablecmds.c | 97 ++++++++++----------------------
 1 file changed, 31 insertions(+), 66 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4e6310886f..cee4f0186c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6755,22 +6755,15 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        Relation        pgclass,
                                attrdesc;
        HeapTuple       reltup;
-       FormData_pg_attribute attribute;
+       Form_pg_attribute attribute;
        int                     newattnum;
        char            relkind;
-       HeapTuple       typeTuple;
-       Oid                     typeOid;
-       int32           typmod;
-       Oid                     collOid;
-       Form_pg_type tform;
        Expr       *defval;
        List       *children;
        ListCell   *child;
        AlterTableCmd *childcmd;
-       AclResult       aclresult;
        ObjectAddress address;
        TupleDesc       tupdesc;
-       FormData_pg_attribute *aattr[] = {&attribute};
 
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
@@ -6892,58 +6885,30 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
                                 errmsg("tables can have at most %d columns",
                                                MaxHeapAttributeNumber)));
 
-       typeTuple = typenameType(NULL, colDef->typeName, &typmod);
-       tform = (Form_pg_type) GETSTRUCT(typeTuple);
-       typeOid = tform->oid;
-
-       aclresult = object_aclcheck(TypeRelationId, typeOid, GetUserId(), 
ACL_USAGE);
-       if (aclresult != ACLCHECK_OK)
-               aclcheck_error_type(aclresult, typeOid);
+       /*
+        * Construct new attribute's pg_attribute entry.
+        */
+       tupdesc = BuildDescForRelation(list_make1(colDef));
 
-       collOid = GetColumnDefCollation(NULL, colDef, typeOid);
+       attribute = TupleDescAttr(tupdesc, 0);
 
-       /* make sure datatype is legal for a column */
-       CheckAttributeType(colDef->colname, typeOid, collOid,
-                                          list_make1_oid(rel->rd_rel->reltype),
-                                          0);
+       /* Fix up attribute number */
+       attribute->attnum = newattnum;
 
        /*
-        * Construct new attribute's pg_attribute entry.  (Variable-length 
fields
-        * are handled by InsertPgAttributeTuples().)
+        * Additional fields not handled by BuildDescForRelation() (mirrors
+        * DefineRelation())
         */
-       attribute.attrelid = myrelid;
-       namestrcpy(&(attribute.attname), colDef->colname);
-       attribute.atttypid = typeOid;
-       attribute.attstattarget = -1;
-       attribute.attlen = tform->typlen;
-       attribute.attnum = newattnum;
-       if (list_length(colDef->typeName->arrayBounds) > PG_INT16_MAX)
-               ereport(ERROR,
-                               errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                               errmsg("too many array dimensions"));
-       attribute.attndims = list_length(colDef->typeName->arrayBounds);
-       attribute.atttypmod = typmod;
-       attribute.attbyval = tform->typbyval;
-       attribute.attalign = tform->typalign;
+       attribute->attidentity = colDef->identity;
+       attribute->attgenerated = colDef->generated;
+       attribute->attcompression = 
GetAttributeCompression(attribute->atttypid, colDef->compression);
        if (colDef->storage_name)
-               attribute.attstorage = GetAttributeStorage(typeOid, 
colDef->storage_name);
-       else
-               attribute.attstorage = tform->typstorage;
-       attribute.attcompression = GetAttributeCompression(typeOid,
-                                                                               
                           colDef->compression);
-       attribute.attnotnull = colDef->is_not_null;
-       attribute.atthasdef = false;
-       attribute.atthasmissing = false;
-       attribute.attidentity = colDef->identity;
-       attribute.attgenerated = colDef->generated;
-       attribute.attisdropped = false;
-       attribute.attislocal = colDef->is_local;
-       attribute.attinhcount = colDef->inhcount;
-       attribute.attcollation = collOid;
+               attribute->attstorage = 
GetAttributeStorage(attribute->atttypid, colDef->storage_name);
 
-       ReleaseSysCache(typeTuple);
-
-       tupdesc = CreateTupleDesc(lengthof(aattr), (FormData_pg_attribute **) 
&aattr);
+       /* make sure datatype is legal for a column */
+       CheckAttributeType(NameStr(attribute->attname), attribute->atttypid, 
attribute->attcollation,
+                                          list_make1_oid(rel->rd_rel->reltype),
+                                          0);
 
        InsertPgAttributeTuples(attrdesc, tupdesc, myrelid, NULL, NULL);
 
@@ -6974,7 +6939,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
                RawColumnDefault *rawEnt;
 
                rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault));
-               rawEnt->attnum = attribute.attnum;
+               rawEnt->attnum = attribute->attnum;
                rawEnt->raw_default = copyObject(colDef->raw_default);
 
                /*
@@ -7048,7 +7013,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
                        NextValueExpr *nve = makeNode(NextValueExpr);
 
                        nve->seqid = RangeVarGetRelid(colDef->identitySequence, 
NoLock, false);
-                       nve->typeId = typeOid;
+                       nve->typeId = attribute->atttypid;
 
                        defval = (Expr *) nve;
 
@@ -7056,23 +7021,23 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
                        tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
                }
                else
-                       defval = (Expr *) build_column_default(rel, 
attribute.attnum);
+                       defval = (Expr *) build_column_default(rel, 
attribute->attnum);
 
-               if (!defval && DomainHasConstraints(typeOid))
+               if (!defval && DomainHasConstraints(attribute->atttypid))
                {
                        Oid                     baseTypeId;
                        int32           baseTypeMod;
                        Oid                     baseTypeColl;
 
-                       baseTypeMod = typmod;
-                       baseTypeId = getBaseTypeAndTypmod(typeOid, 
&baseTypeMod);
+                       baseTypeMod = attribute->atttypmod;
+                       baseTypeId = getBaseTypeAndTypmod(attribute->atttypid, 
&baseTypeMod);
                        baseTypeColl = get_typcollation(baseTypeId);
                        defval = (Expr *) makeNullConst(baseTypeId, 
baseTypeMod, baseTypeColl);
                        defval = (Expr *) coerce_to_target_type(NULL,
                                                                                
                        (Node *) defval,
                                                                                
                        baseTypeId,
-                                                                               
                        typeOid,
-                                                                               
                        typmod,
+                                                                               
                        attribute->atttypid,
+                                                                               
                        attribute->atttypmod,
                                                                                
                        COERCION_ASSIGNMENT,
                                                                                
                        COERCE_IMPLICIT_CAST,
                                                                                
                        -1);
@@ -7085,17 +7050,17 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
                        NewColumnValue *newval;
 
                        newval = (NewColumnValue *) 
palloc0(sizeof(NewColumnValue));
-                       newval->attnum = attribute.attnum;
+                       newval->attnum = attribute->attnum;
                        newval->expr = expression_planner(defval);
                        newval->is_generated = (colDef->generated != '\0');
 
                        tab->newvals = lappend(tab->newvals, newval);
                }
 
-               if (DomainHasConstraints(typeOid))
+               if (DomainHasConstraints(attribute->atttypid))
                        tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
 
-               if (!TupleDescAttr(rel->rd_att, attribute.attnum - 
1)->atthasmissing)
+               if (!TupleDescAttr(rel->rd_att, attribute->attnum - 
1)->atthasmissing)
                {
                        /*
                         * If the new column is NOT NULL, and there is no 
missing value,
@@ -7108,8 +7073,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        /*
         * Add needed dependency entries for the new column.
         */
-       add_column_datatype_dependency(myrelid, newattnum, attribute.atttypid);
-       add_column_collation_dependency(myrelid, newattnum, 
attribute.attcollation);
+       add_column_datatype_dependency(myrelid, newattnum, attribute->atttypid);
+       add_column_collation_dependency(myrelid, newattnum, 
attribute->attcollation);
 
        /*
         * Propagate to children as appropriate.  Unlike most other ALTER
-- 
2.41.0

From 1d513672df3c01a731d54de0da68a7f3e1c0d289 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 28 Jun 2023 17:17:22 +0200
Subject: [PATCH 12/17] Push attidentity and attgenerated handling into
 BuildDescForRelation()

Previously, this was handled by the callers separately, but it can be
trivially moved into BuildDescForRelation() so that it is handled in a
central place.
---
 src/backend/access/common/tupdesc.c | 2 ++
 src/backend/commands/tablecmds.c    | 4 ----
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/common/tupdesc.c 
b/src/backend/access/common/tupdesc.c
index 253d6c86f8..bc1b67e6c9 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -856,6 +856,8 @@ BuildDescForRelation(const List *columns)
                has_not_null |= entry->is_not_null;
                att->attislocal = entry->is_local;
                att->attinhcount = entry->inhcount;
+               att->attidentity = entry->identity;
+               att->attgenerated = entry->generated;
        }
 
        if (has_not_null)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cee4f0186c..06e4527dd1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -927,8 +927,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                        attr->atthasdef = true;
                }
 
-               attr->attidentity = colDef->identity;
-               attr->attgenerated = colDef->generated;
                attr->attcompression = GetAttributeCompression(attr->atttypid, 
colDef->compression);
                if (colDef->storage_name)
                        attr->attstorage = GetAttributeStorage(attr->atttypid, 
colDef->storage_name);
@@ -6899,8 +6897,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
         * Additional fields not handled by BuildDescForRelation() (mirrors
         * DefineRelation())
         */
-       attribute->attidentity = colDef->identity;
-       attribute->attgenerated = colDef->generated;
        attribute->attcompression = 
GetAttributeCompression(attribute->atttypid, colDef->compression);
        if (colDef->storage_name)
                attribute->attstorage = 
GetAttributeStorage(attribute->atttypid, colDef->storage_name);
-- 
2.41.0

From 8a81f53b6cd043ed923b521d2917292f6726df64 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 28 Jun 2023 17:16:49 +0200
Subject: [PATCH 13/17] Move BuildDescForRelation() from tupdesc.c to
 tablecmds.c

BuildDescForRelation() main job is to convert ColumnDef lists to
pg_attribute/tuple descriptor arrays, which is really mostly an
internal subroutine of DefineRelation() and some related functions,
which is more the remit of tablecmds.c and doesn't have much to do
with the basic tuple descriptor interfaces in tupdesc.c.  This is also
supported by observing the header includes we can remove in tupdesc.c.
By moving it over, we can also (in the future) make
BuildDescForRelation() use more internals of tablecmds.c that are not
sensible to be exposed in tupdesc.c.
---
 src/backend/access/common/tupdesc.c | 109 +---------------------------
 src/backend/commands/tablecmds.c    | 102 ++++++++++++++++++++++++++
 src/include/access/tupdesc.h        |   2 -
 src/include/commands/tablecmds.h    |   2 +
 4 files changed, 105 insertions(+), 110 deletions(-)

diff --git a/src/backend/access/common/tupdesc.c 
b/src/backend/access/common/tupdesc.c
index bc1b67e6c9..5d85831339 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -25,9 +25,6 @@
 #include "catalog/pg_collation.h"
 #include "catalog/pg_type.h"
 #include "common/hashfn.h"
-#include "miscadmin.h"
-#include "parser/parse_type.h"
-#include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
 #include "utils/resowner_private.h"
@@ -778,109 +775,6 @@ TupleDescInitEntryCollation(TupleDesc desc,
        TupleDescAttr(desc, attributeNumber - 1)->attcollation = collationid;
 }
 
-
-/*
- * BuildDescForRelation
- *
- * Given a list of ColumnDef nodes, build a TupleDesc.
- *
- * Note: tdtypeid will need to be filled in later on.
- */
-TupleDesc
-BuildDescForRelation(const List *columns)
-{
-       int                     natts;
-       AttrNumber      attnum;
-       ListCell   *l;
-       TupleDesc       desc;
-       bool            has_not_null;
-       char       *attname;
-       Oid                     atttypid;
-       int32           atttypmod;
-       Oid                     attcollation;
-       int                     attdim;
-
-       /*
-        * allocate a new tuple descriptor
-        */
-       natts = list_length(columns);
-       desc = CreateTemplateTupleDesc(natts);
-       has_not_null = false;
-
-       attnum = 0;
-
-       foreach(l, columns)
-       {
-               ColumnDef  *entry = lfirst(l);
-               AclResult       aclresult;
-               Form_pg_attribute att;
-
-               /*
-                * for each entry in the list, get the name and type 
information from
-                * the list and have TupleDescInitEntry fill in the attribute
-                * information we need.
-                */
-               attnum++;
-
-               attname = entry->colname;
-               typenameTypeIdAndMod(NULL, entry->typeName, &atttypid, 
&atttypmod);
-
-               aclresult = object_aclcheck(TypeRelationId, atttypid, 
GetUserId(), ACL_USAGE);
-               if (aclresult != ACLCHECK_OK)
-                       aclcheck_error_type(aclresult, atttypid);
-
-               attcollation = GetColumnDefCollation(NULL, entry, atttypid);
-               attdim = list_length(entry->typeName->arrayBounds);
-               if (attdim > PG_INT16_MAX)
-                       ereport(ERROR,
-                                       errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                                       errmsg("too many array dimensions"));
-
-               if (entry->typeName->setof)
-                       ereport(ERROR,
-                                       
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-                                        errmsg("column \"%s\" cannot be 
declared SETOF",
-                                                       attname)));
-
-               TupleDescInitEntry(desc, attnum, attname,
-                                                  atttypid, atttypmod, attdim);
-               att = TupleDescAttr(desc, attnum - 1);
-
-               /* Override TupleDescInitEntry's settings as requested */
-               TupleDescInitEntryCollation(desc, attnum, attcollation);
-               if (entry->storage)
-                       att->attstorage = entry->storage;
-
-               /* Fill in additional stuff not handled by TupleDescInitEntry */
-               att->attnotnull = entry->is_not_null;
-               has_not_null |= entry->is_not_null;
-               att->attislocal = entry->is_local;
-               att->attinhcount = entry->inhcount;
-               att->attidentity = entry->identity;
-               att->attgenerated = entry->generated;
-       }
-
-       if (has_not_null)
-       {
-               TupleConstr *constr = (TupleConstr *) 
palloc0(sizeof(TupleConstr));
-
-               constr->has_not_null = true;
-               constr->has_generated_stored = false;
-               constr->defval = NULL;
-               constr->missing = NULL;
-               constr->num_defval = 0;
-               constr->check = NULL;
-               constr->num_check = 0;
-               desc->constr = constr;
-       }
-       else
-       {
-               desc->constr = NULL;
-       }
-
-       return desc;
-}
-
 /*
  * BuildDescFromLists
  *
@@ -889,8 +783,7 @@ BuildDescForRelation(const List *columns)
  *
  * No constraints are generated.
  *
- * This is essentially a cut-down version of BuildDescForRelation for use
- * with functions returning RECORD.
+ * This is for use with functions returning RECORD.
  */
 TupleDesc
 BuildDescFromLists(const List *names, const List *types, const List *typmods, 
const List *collations)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 06e4527dd1..2760b8b111 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1252,6 +1252,108 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid 
ownerId,
        return address;
 }
 
+/*
+ * BuildDescForRelation
+ *
+ * Given a list of ColumnDef nodes, build a TupleDesc.
+ *
+ * Note: tdtypeid will need to be filled in later on.
+ */
+TupleDesc
+BuildDescForRelation(const List *columns)
+{
+       int                     natts;
+       AttrNumber      attnum;
+       ListCell   *l;
+       TupleDesc       desc;
+       bool            has_not_null;
+       char       *attname;
+       Oid                     atttypid;
+       int32           atttypmod;
+       Oid                     attcollation;
+       int                     attdim;
+
+       /*
+        * allocate a new tuple descriptor
+        */
+       natts = list_length(columns);
+       desc = CreateTemplateTupleDesc(natts);
+       has_not_null = false;
+
+       attnum = 0;
+
+       foreach(l, columns)
+       {
+               ColumnDef  *entry = lfirst(l);
+               AclResult       aclresult;
+               Form_pg_attribute att;
+
+               /*
+                * for each entry in the list, get the name and type 
information from
+                * the list and have TupleDescInitEntry fill in the attribute
+                * information we need.
+                */
+               attnum++;
+
+               attname = entry->colname;
+               typenameTypeIdAndMod(NULL, entry->typeName, &atttypid, 
&atttypmod);
+
+               aclresult = object_aclcheck(TypeRelationId, atttypid, 
GetUserId(), ACL_USAGE);
+               if (aclresult != ACLCHECK_OK)
+                       aclcheck_error_type(aclresult, atttypid);
+
+               attcollation = GetColumnDefCollation(NULL, entry, atttypid);
+               attdim = list_length(entry->typeName->arrayBounds);
+               if (attdim > PG_INT16_MAX)
+                       ereport(ERROR,
+                                       errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                       errmsg("too many array dimensions"));
+
+               if (entry->typeName->setof)
+                       ereport(ERROR,
+                                       
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+                                        errmsg("column \"%s\" cannot be 
declared SETOF",
+                                                       attname)));
+
+               TupleDescInitEntry(desc, attnum, attname,
+                                                  atttypid, atttypmod, attdim);
+               att = TupleDescAttr(desc, attnum - 1);
+
+               /* Override TupleDescInitEntry's settings as requested */
+               TupleDescInitEntryCollation(desc, attnum, attcollation);
+               if (entry->storage)
+                       att->attstorage = entry->storage;
+
+               /* Fill in additional stuff not handled by TupleDescInitEntry */
+               att->attnotnull = entry->is_not_null;
+               has_not_null |= entry->is_not_null;
+               att->attislocal = entry->is_local;
+               att->attinhcount = entry->inhcount;
+               att->attidentity = entry->identity;
+               att->attgenerated = entry->generated;
+       }
+
+       if (has_not_null)
+       {
+               TupleConstr *constr = (TupleConstr *) 
palloc0(sizeof(TupleConstr));
+
+               constr->has_not_null = true;
+               constr->has_generated_stored = false;
+               constr->defval = NULL;
+               constr->missing = NULL;
+               constr->num_defval = 0;
+               constr->check = NULL;
+               constr->num_check = 0;
+               desc->constr = constr;
+       }
+       else
+       {
+               desc->constr = NULL;
+       }
+
+       return desc;
+}
+
 /*
  * Emit the right error or warning message for a "DROP" command issued on a
  * non-existent relation
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index f6cc28a661..f61c7cc784 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -147,8 +147,6 @@ extern void TupleDescInitEntryCollation(TupleDesc desc,
                                                                                
AttrNumber attributeNumber,
                                                                                
Oid collationid);
 
-extern TupleDesc BuildDescForRelation(const List *columns);
-
 extern TupleDesc BuildDescFromLists(const List *names, const List *types, 
const List *typmods, const List *collations);
 
 #endif                                                 /* TUPDESC_H */
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 250d89ff88..8a8a9cd0ae 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -27,6 +27,8 @@ struct AlterTableUtilityContext;      /* avoid including 
tcop/utility.h here */
 extern ObjectAddress DefineRelation(CreateStmt *stmt, char relkind, Oid 
ownerId,
                                                                        
ObjectAddress *typaddress, const char *queryString);
 
+extern TupleDesc BuildDescForRelation(const List *columns);
+
 extern void RemoveRelations(DropStmt *drop);
 
 extern Oid     AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE 
lockmode);
-- 
2.41.0

From dbbf9580358c18d78a2a11492b166818e3c19eec Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 16 Jun 2023 12:10:23 +0200
Subject: [PATCH 14/17] Push attcompression and attstorage handling into
 BuildDescForRelation()

This was previously handled by the callers but it can be moved into a
common place.
---
 src/backend/commands/tablecmds.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2760b8b111..c1be6be826 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -926,10 +926,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                        cookedDefaults = lappend(cookedDefaults, cooked);
                        attr->atthasdef = true;
                }
-
-               attr->attcompression = GetAttributeCompression(attr->atttypid, 
colDef->compression);
-               if (colDef->storage_name)
-                       attr->attstorage = GetAttributeStorage(attr->atttypid, 
colDef->storage_name);
        }
 
        /*
@@ -1321,8 +1317,6 @@ BuildDescForRelation(const List *columns)
 
                /* Override TupleDescInitEntry's settings as requested */
                TupleDescInitEntryCollation(desc, attnum, attcollation);
-               if (entry->storage)
-                       att->attstorage = entry->storage;
 
                /* Fill in additional stuff not handled by TupleDescInitEntry */
                att->attnotnull = entry->is_not_null;
@@ -1331,6 +1325,11 @@ BuildDescForRelation(const List *columns)
                att->attinhcount = entry->inhcount;
                att->attidentity = entry->identity;
                att->attgenerated = entry->generated;
+               att->attcompression = GetAttributeCompression(att->atttypid, 
entry->compression);
+               if (entry->storage)
+                       att->attstorage = entry->storage;
+               else if (entry->storage_name)
+                       att->attstorage = GetAttributeStorage(att->atttypid, 
entry->storage_name);
        }
 
        if (has_not_null)
@@ -6995,14 +6994,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        /* Fix up attribute number */
        attribute->attnum = newattnum;
 
-       /*
-        * Additional fields not handled by BuildDescForRelation() (mirrors
-        * DefineRelation())
-        */
-       attribute->attcompression = 
GetAttributeCompression(attribute->atttypid, colDef->compression);
-       if (colDef->storage_name)
-               attribute->attstorage = 
GetAttributeStorage(attribute->atttypid, colDef->storage_name);
-
        /* make sure datatype is legal for a column */
        CheckAttributeType(NameStr(attribute->attname), attribute->atttypid, 
attribute->attcollation,
                                           list_make1_oid(rel->rd_rel->reltype),
-- 
2.41.0

From ec1d40af2d1b890970c416df7a411b4c0af5c288 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 22 Jun 2023 15:21:17 +0200
Subject: [PATCH 15/17] Add TupleDescGetDefault()

This unifies some repetitive code.

Note: I didn't push the "not found" error message into the new
function, even though all existing callers would be able to make use
of it.  Using the existing error handling as-is would probably require
exposing the Relation type via tupdesc.h, which doesn't seem
desirable.
---
 src/backend/access/common/tupdesc.c  | 25 +++++++++++++++++++++++++
 src/backend/commands/tablecmds.c     | 17 ++---------------
 src/backend/parser/parse_utilcmd.c   | 13 ++-----------
 src/backend/rewrite/rewriteHandler.c | 16 +---------------
 src/include/access/tupdesc.h         |  2 ++
 5 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/src/backend/access/common/tupdesc.c 
b/src/backend/access/common/tupdesc.c
index 5d85831339..d119cfafb5 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -822,3 +822,28 @@ BuildDescFromLists(const List *names, const List *types, 
const List *typmods, co
 
        return desc;
 }
+
+/*
+ * Get default expression (or NULL if none) for the given attribute number.
+ */
+Node *
+TupleDescGetDefault(TupleDesc tupdesc, AttrNumber attnum)
+{
+       Node       *result = NULL;
+
+       if (tupdesc->constr)
+       {
+               AttrDefault *attrdef = tupdesc->constr->defval;
+
+               for (int i = 0; i < tupdesc->constr->num_defval; i++)
+               {
+                       if (attrdef[i].adnum == attnum)
+                       {
+                               result = stringToNode(attrdef[i].adbin);
+                               break;
+                       }
+               }
+       }
+
+       return result;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c1be6be826..bb0fb8b9af 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2785,22 +2785,9 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                         */
                        if (attribute->atthasdef)
                        {
-                               Node       *this_default = NULL;
+                               Node       *this_default;
 
-                               /* Find default in constraint structure */
-                               if (constr != NULL)
-                               {
-                                       AttrDefault *attrdef = constr->defval;
-
-                                       for (int i = 0; i < constr->num_defval; 
i++)
-                                       {
-                                               if (attrdef[i].adnum == 
parent_attno)
-                                               {
-                                                       this_default = 
stringToNode(attrdef[i].adbin);
-                                                       break;
-                                               }
-                                       }
-                               }
+                               this_default = TupleDescGetDefault(tupleDesc, 
parent_attno);
                                if (this_default == NULL)
                                        elog(ERROR, "default expression not 
found for attribute %d of relation \"%s\"",
                                                 parent_attno, 
RelationGetRelationName(relation));
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index 53420306ec..9f80ac40f2 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1248,20 +1248,11 @@ expandTableLikeClause(RangeVar *heapRel, 
TableLikeClause *table_like_clause)
                                 (table_like_clause->options & 
CREATE_TABLE_LIKE_GENERATED) :
                                 (table_like_clause->options & 
CREATE_TABLE_LIKE_DEFAULTS)))
                        {
-                               Node       *this_default = NULL;
-                               AttrDefault *attrdef = constr->defval;
+                               Node       *this_default;
                                AlterTableCmd *atsubcmd;
                                bool            found_whole_row;
 
-                               /* Find default in constraint structure */
-                               for (int i = 0; i < constr->num_defval; i++)
-                               {
-                                       if (attrdef[i].adnum == parent_attno)
-                                       {
-                                               this_default = 
stringToNode(attrdef[i].adbin);
-                                               break;
-                                       }
-                               }
+                               this_default = TupleDescGetDefault(tupleDesc, 
parent_attno);
                                if (this_default == NULL)
                                        elog(ERROR, "default expression not 
found for attribute %d of relation \"%s\"",
                                                 parent_attno, 
RelationGetRelationName(relation));
diff --git a/src/backend/rewrite/rewriteHandler.c 
b/src/backend/rewrite/rewriteHandler.c
index b486ab559a..41a362310a 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1246,21 +1246,7 @@ build_column_default(Relation rel, int attrno)
         */
        if (att_tup->atthasdef)
        {
-               if (rd_att->constr && rd_att->constr->num_defval > 0)
-               {
-                       AttrDefault *defval = rd_att->constr->defval;
-                       int                     ndef = 
rd_att->constr->num_defval;
-
-                       while (--ndef >= 0)
-                       {
-                               if (attrno == defval[ndef].adnum)
-                               {
-                                       /* Found it, convert string 
representation to node tree. */
-                                       expr = stringToNode(defval[ndef].adbin);
-                                       break;
-                               }
-                       }
-               }
+               expr = TupleDescGetDefault(rd_att, attrno);
                if (expr == NULL)
                        elog(ERROR, "default expression not found for attribute 
%d of relation \"%s\"",
                                 attrno, RelationGetRelationName(rel));
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index f61c7cc784..d833d5f2e1 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -149,4 +149,6 @@ extern void TupleDescInitEntryCollation(TupleDesc desc,
 
 extern TupleDesc BuildDescFromLists(const List *names, const List *types, 
const List *typmods, const List *collations);
 
+extern Node *TupleDescGetDefault(TupleDesc tupdesc, AttrNumber attnum);
+
 #endif                                                 /* TUPDESC_H */
-- 
2.41.0

From 0896393c4794574b5bf476fc493842297b47848a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 22 Jun 2023 20:17:56 +0200
Subject: [PATCH 16/17] MergeAttributes: convert pg_attribute back to ColumnDef
 before comparing

MergeAttributes() has a loop to merge multiple inheritance parents
into a column column definition.  The merged-so-far definition is
stored in a ColumnDef node.  If we have to merge columns from multiple
inheritance parents (if the name matches), then we have to check
whether various column properties (type, collation, etc.) match.  The
current code extracts the pg_attribute value of the
currently-considered inheritance parent and compares it against the
merge-so-far ColumnDef value.  If the currently considered column
doesn't match any previously inherited column, we make a new ColumnDef
node from the pg_attribute information and add it to the column list.

This patch rearranges this so that we create the ColumnDef node first
in either case.  Then the code that checks the column properties for
compatibility compares ColumnDef against ColumnDef (instead of
ColumnDef against pg_attribute).  This matches the code more symmetric
and easier to follow.  Also, later in MergeAttributes(), there is a
similar but separate loop that merges the new local column definition
with the combination of the inheritance parents established in the
first loop.  That comparison is already ColumnDef-vs-ColumnDef.  With
this change, but of these can use similar looking logic.  (A future
project might be to extract these two sets of code into a common
routine that encodes all the knowledge of whether two column
definitions are compatible.  But this isn't currently straightforward
because we want to give different error messages in the two cases.)
Furthermore, by avoiding the use of Form_pg_attribute here, we make it
easier to make changes in the pg_attribute layout without having to
worry about the local needs of tablecmds.c.

Because MergeAttributes() is hugely long, it's sometimes hard to know
where in the function you are currently looking.  To help with that, I
also renamed some variables to make it clearer where you are currently
looking.  The first look is "prev" vs. "new", the second loop is "inh"
vs. "new".
---
 src/backend/commands/tablecmds.c | 179 ++++++++++++++++---------------
 1 file changed, 93 insertions(+), 86 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bb0fb8b9af..ac7452bcff 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2649,7 +2649,8 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                                                                
                                parent_attno - 1);
                        char       *attributeName = NameStr(attribute->attname);
                        int                     exist_attno;
-                       ColumnDef  *def;
+                       ColumnDef  *newdef;
+                       ColumnDef  *savedef;
 
                        /*
                         * Ignore dropped columns in the parent.
@@ -2658,14 +2659,30 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                continue;               /* leave 
newattmap->attnums entry as zero */
 
                        /*
-                        * Does it conflict with some previously inherited 
column?
+                        * Create new column definition
+                        */
+                       newdef = makeColumnDef(attributeName, 
attribute->atttypid, attribute->atttypmod, attribute->attcollation);
+                       newdef->is_not_null = attribute->attnotnull;
+                       newdef->storage = attribute->attstorage;
+                       newdef->generated = attribute->attgenerated;
+                       if (CompressionMethodIsValid(attribute->attcompression))
+                               newdef->compression =
+                                       
pstrdup(GetCompressionMethodName(attribute->attcompression));
+
+                       /*
+                        * Does it match some previously considered column from 
another
+                        * parent?
                         */
                        exist_attno = findAttrByName(attributeName, 
inh_columns);
                        if (exist_attno > 0)
                        {
-                               Oid                     defTypeId;
-                               int32           deftypmod;
-                               Oid                     defCollId;
+                               ColumnDef  *prevdef;
+                               Oid                     prevtypeid,
+                                                       newtypeid;
+                               int32           prevtypmod,
+                                                       newtypmod;
+                               Oid                     prevcollid,
+                                                       newcollid;
 
                                /*
                                 * Yes, try to merge the two column definitions.
@@ -2673,78 +2690,71 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                ereport(NOTICE,
                                                (errmsg("merging multiple 
inherited definitions of column \"%s\"",
                                                                
attributeName)));
-                               def = (ColumnDef *) list_nth(inh_columns, 
exist_attno - 1);
+                               prevdef = list_nth_node(ColumnDef, inh_columns, 
exist_attno - 1);
 
                                /*
                                 * Must have the same type and typmod
                                 */
-                               typenameTypeIdAndMod(NULL, def->typeName, 
&defTypeId, &deftypmod);
-                               if (defTypeId != attribute->atttypid ||
-                                       deftypmod != attribute->atttypmod)
+                               typenameTypeIdAndMod(NULL, prevdef->typeName, 
&prevtypeid, &prevtypmod);
+                               typenameTypeIdAndMod(NULL, newdef->typeName, 
&newtypeid, &newtypmod);
+                               if (prevtypeid != newtypeid || prevtypmod != 
newtypmod)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                         errmsg("inherited 
column \"%s\" has a type conflict",
                                                                        
attributeName),
                                                         errdetail("%s versus 
%s",
-                                                                          
format_type_with_typemod(defTypeId,
-                                                                               
                                                deftypmod),
-                                                                          
format_type_with_typemod(attribute->atttypid,
-                                                                               
                                                attribute->atttypmod))));
+                                                                          
format_type_with_typemod(prevtypeid, prevtypmod),
+                                                                          
format_type_with_typemod(newtypeid, newtypmod))));
 
                                /*
                                 * Must have the same collation
                                 */
-                               defCollId = GetColumnDefCollation(NULL, def, 
defTypeId);
-                               if (defCollId != attribute->attcollation)
+                               prevcollid = GetColumnDefCollation(NULL, 
prevdef, prevtypeid);
+                               newcollid = GetColumnDefCollation(NULL, newdef, 
newtypeid);
+                               if (prevcollid != newcollid)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_COLLATION_MISMATCH),
                                                         errmsg("inherited 
column \"%s\" has a collation conflict",
                                                                        
attributeName),
                                                         errdetail("\"%s\" 
versus \"%s\"",
-                                                                          
get_collation_name(defCollId),
-                                                                          
get_collation_name(attribute->attcollation))));
+                                                                          
get_collation_name(prevcollid),
+                                                                          
get_collation_name(newcollid))));
 
                                /*
                                 * Copy/check storage parameter
                                 */
-                               if (def->storage == 0)
-                                       def->storage = attribute->attstorage;
-                               else if (def->storage != attribute->attstorage)
+                               if (prevdef->storage == 0)
+                                       prevdef->storage = newdef->storage;
+                               else if (prevdef->storage != newdef->storage)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                         errmsg("inherited 
column \"%s\" has a storage parameter conflict",
                                                                        
attributeName),
                                                         errdetail("%s versus 
%s",
-                                                                          
storage_name(def->storage),
-                                                                          
storage_name(attribute->attstorage))));
+                                                                          
storage_name(prevdef->storage),
+                                                                          
storage_name(newdef->storage))));
 
                                /*
                                 * Copy/check compression parameter
                                 */
-                               if 
(CompressionMethodIsValid(attribute->attcompression))
-                               {
-                                       const char *compression =
-                                               
GetCompressionMethodName(attribute->attcompression);
-
-                                       if (def->compression == NULL)
-                                               def->compression = 
pstrdup(compression);
-                                       else if (strcmp(def->compression, 
compression) != 0)
-                                               ereport(ERROR,
-                                                               
(errcode(ERRCODE_DATATYPE_MISMATCH),
-                                                                errmsg("column 
\"%s\" has a compression method conflict",
-                                                                               
attributeName),
-                                                                errdetail("%s 
versus %s", def->compression, compression)));
-                               }
+                               if (prevdef->compression == NULL)
+                                       prevdef->compression = 
newdef->compression;
+                               else if (strcmp(prevdef->compression, 
newdef->compression) != 0)
+                                       ereport(ERROR,
+                                                       
(errcode(ERRCODE_DATATYPE_MISMATCH),
+                                                        errmsg("column \"%s\" 
has a compression method conflict",
+                                                                       
attributeName),
+                                                        errdetail("%s versus 
%s", prevdef->compression, newdef->compression)));
 
                                /*
                                 * Merge of NOT NULL constraints = OR 'em 
together
                                 */
-                               def->is_not_null |= attribute->attnotnull;
+                               prevdef->is_not_null |= newdef->is_not_null;
 
                                /*
                                 * Check for GENERATED conflicts
                                 */
-                               if (def->generated != attribute->attgenerated)
+                               if (prevdef->generated != newdef->generated)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                         errmsg("inherited 
column \"%s\" has a generation conflict",
@@ -2754,30 +2764,29 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                 * Default and other constraints are handled 
below
                                 */
 
-                               def->inhcount++;
-                               if (def->inhcount < 0)
+                               prevdef->inhcount++;
+                               if (prevdef->inhcount < 0)
                                        ereport(ERROR,
                                                        
errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                                                        errmsg("too many 
inheritance parents"));
 
                                newattmap->attnums[parent_attno - 1] = 
exist_attno;
+
+                               /* remember for default processing below */
+                               savedef = prevdef;
                        }
                        else
                        {
                                /*
                                 * No, create a new inherited column
                                 */
-                               def = makeColumnDef(attributeName, 
attribute->atttypid, attribute->atttypmod, attribute->attcollation);
-                               def->inhcount = 1;
-                               def->is_local = false;
-                               def->is_not_null = attribute->attnotnull;
-                               def->storage = attribute->attstorage;
-                               def->generated = attribute->attgenerated;
-                               if 
(CompressionMethodIsValid(attribute->attcompression))
-                                       def->compression =
-                                               
pstrdup(GetCompressionMethodName(attribute->attcompression));
-                               inh_columns = lappend(inh_columns, def);
+                               newdef->inhcount = 1;
+                               newdef->is_local = false;
+                               inh_columns = lappend(inh_columns, newdef);
                                newattmap->attnums[parent_attno - 1] = 
++child_attno;
+
+                               /* remember for default processing below */
+                               savedef = newdef;
                        }
 
                        /*
@@ -2799,7 +2808,7 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                 * all the inherited default expressions for 
the moment.
                                 */
                                inherited_defaults = 
lappend(inherited_defaults, this_default);
-                               cols_with_defaults = 
lappend(cols_with_defaults, def);
+                               cols_with_defaults = 
lappend(cols_with_defaults, savedef);
                        }
                }
 
@@ -2920,17 +2929,17 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                        newcol_attno++;
 
                        /*
-                        * Does it conflict with some previously inherited 
column?
+                        * Does it match some inherited column?
                         */
                        exist_attno = findAttrByName(attributeName, 
inh_columns);
                        if (exist_attno > 0)
                        {
-                               ColumnDef  *def;
-                               Oid                     defTypeId,
-                                                       newTypeId;
-                               int32           deftypmod,
+                               ColumnDef  *inhdef;
+                               Oid                     inhtypeid,
+                                                       newtypeid;
+                               int32           inhtypmod,
                                                        newtypmod;
-                               Oid                     defcollid,
+                               Oid                     inhcollid,
                                                        newcollid;
 
                                /*
@@ -2950,77 +2959,75 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                        ereport(NOTICE,
                                                        (errmsg("moving and 
merging column \"%s\" with inherited definition", attributeName),
                                                         
errdetail("User-specified column moved to the position of the inherited 
column.")));
-                               def = (ColumnDef *) list_nth(inh_columns, 
exist_attno - 1);
+                               inhdef = list_nth_node(ColumnDef, inh_columns, 
exist_attno - 1);
 
                                /*
                                 * Must have the same type and typmod
                                 */
-                               typenameTypeIdAndMod(NULL, def->typeName, 
&defTypeId, &deftypmod);
-                               typenameTypeIdAndMod(NULL, newdef->typeName, 
&newTypeId, &newtypmod);
-                               if (defTypeId != newTypeId || deftypmod != 
newtypmod)
+                               typenameTypeIdAndMod(NULL, inhdef->typeName, 
&inhtypeid, &inhtypmod);
+                               typenameTypeIdAndMod(NULL, newdef->typeName, 
&newtypeid, &newtypmod);
+                               if (inhtypeid != newtypeid || inhtypmod != 
newtypmod)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                         errmsg("column \"%s\" 
has a type conflict",
                                                                        
attributeName),
                                                         errdetail("%s versus 
%s",
-                                                                          
format_type_with_typemod(defTypeId,
-                                                                               
                                                deftypmod),
-                                                                          
format_type_with_typemod(newTypeId,
-                                                                               
                                                newtypmod))));
+                                                                          
format_type_with_typemod(inhtypeid, inhtypmod),
+                                                                          
format_type_with_typemod(newtypeid, newtypmod))));
 
                                /*
                                 * Must have the same collation
                                 */
-                               defcollid = GetColumnDefCollation(NULL, def, 
defTypeId);
-                               newcollid = GetColumnDefCollation(NULL, newdef, 
newTypeId);
-                               if (defcollid != newcollid)
+                               inhcollid = GetColumnDefCollation(NULL, inhdef, 
inhtypeid);
+                               newcollid = GetColumnDefCollation(NULL, newdef, 
newtypeid);
+                               if (inhcollid != newcollid)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_COLLATION_MISMATCH),
                                                         errmsg("column \"%s\" 
has a collation conflict",
                                                                        
attributeName),
                                                         errdetail("\"%s\" 
versus \"%s\"",
-                                                                          
get_collation_name(defcollid),
+                                                                          
get_collation_name(inhcollid),
                                                                           
get_collation_name(newcollid))));
 
                                /*
                                 * Identity is never inherited.  The new column 
can have an
                                 * identity definition, so we always just take 
that one.
                                 */
-                               def->identity = newdef->identity;
+                               inhdef->identity = newdef->identity;
 
                                /*
                                 * Copy storage parameter
                                 */
-                               if (def->storage == 0)
-                                       def->storage = newdef->storage;
-                               else if (newdef->storage != 0 && def->storage 
!= newdef->storage)
+                               if (inhdef->storage == 0)
+                                       inhdef->storage = newdef->storage;
+                               else if (newdef->storage != 0 && 
inhdef->storage != newdef->storage)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                         errmsg("column \"%s\" 
has a storage parameter conflict",
                                                                        
attributeName),
                                                         errdetail("%s versus 
%s",
-                                                                          
storage_name(def->storage),
+                                                                          
storage_name(inhdef->storage),
                                                                           
storage_name(newdef->storage))));
 
                                /*
                                 * Copy compression parameter
                                 */
-                               if (def->compression == NULL)
-                                       def->compression = newdef->compression;
+                               if (inhdef->compression == NULL)
+                                       inhdef->compression = 
newdef->compression;
                                else if (newdef->compression != NULL)
                                {
-                                       if (strcmp(def->compression, 
newdef->compression) != 0)
+                                       if (strcmp(inhdef->compression, 
newdef->compression) != 0)
                                                ereport(ERROR,
                                                                
(errcode(ERRCODE_DATATYPE_MISMATCH),
                                                                 errmsg("column 
\"%s\" has a compression method conflict",
                                                                                
attributeName),
-                                                                errdetail("%s 
versus %s", def->compression, newdef->compression)));
+                                                                errdetail("%s 
versus %s", inhdef->compression, newdef->compression)));
                                }
 
                                /*
                                 * Merge of NOT NULL constraints = OR 'em 
together
                                 */
-                               def->is_not_null |= newdef->is_not_null;
+                               inhdef->is_not_null |= newdef->is_not_null;
 
                                /*
                                 * Check for conflicts related to generated 
columns.
@@ -3037,18 +3044,18 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                 * it results in being able to override the 
generation
                                 * expression via UPDATEs through the parent.)
                                 */
-                               if (def->generated)
+                               if (inhdef->generated)
                                {
                                        if (newdef->raw_default && 
!newdef->generated)
                                                ereport(ERROR,
                                                                
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
                                                                 errmsg("column 
\"%s\" inherits from generated column but specifies default",
-                                                                               
def->colname)));
+                                                                               
inhdef->colname)));
                                        if (newdef->identity)
                                                ereport(ERROR,
                                                                
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
                                                                 errmsg("column 
\"%s\" inherits from generated column but specifies identity",
-                                                                               
def->colname)));
+                                                                               
inhdef->colname)));
                                }
                                else
                                {
@@ -3056,7 +3063,7 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                                ereport(ERROR,
                                                                
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
                                                                 errmsg("child 
column \"%s\" specifies generation expression",
-                                                                               
def->colname),
+                                                                               
inhdef->colname),
                                                                 errhint("A 
child table column cannot be generated unless its parent column is.")));
                                }
 
@@ -3065,12 +3072,12 @@ MergeAttributes(List *columns, const List *supers, char 
relpersistence,
                                 */
                                if (newdef->raw_default != NULL)
                                {
-                                       def->raw_default = newdef->raw_default;
-                                       def->cooked_default = 
newdef->cooked_default;
+                                       inhdef->raw_default = 
newdef->raw_default;
+                                       inhdef->cooked_default = 
newdef->cooked_default;
                                }
 
                                /* Mark the column as locally defined */
-                               def->is_local = true;
+                               inhdef->is_local = true;
                        }
                        else
                        {
-- 
2.41.0

From 87ed2951471e3fe6dc1bc318e1dd8e3ea4eab95f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 27 Jun 2023 11:58:51 +0200
Subject: [PATCH 17/17] Add some const decorations

---
 src/backend/commands/tablecmds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac7452bcff..0476110372 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -636,7 +636,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, 
Relation partIdx,
                                                                  Relation 
partitionTbl);
 static List *GetParentedForeignKeyRefs(Relation partition);
 static void ATDetachCheckNoForeignKeyRefs(Relation partition);
-static char GetAttributeCompression(Oid atttypid, char *compression);
+static char GetAttributeCompression(Oid atttypid, const char *compression);
 static char GetAttributeStorage(Oid atttypid, const char *storagemode);
 
 
@@ -19310,7 +19310,7 @@ ATDetachCheckNoForeignKeyRefs(Relation partition)
  * resolve column compression specification to compression method.
  */
 static char
-GetAttributeCompression(Oid atttypid, char *compression)
+GetAttributeCompression(Oid atttypid, const char *compression)
 {
        char            cmethod;
 
-- 
2.41.0

Reply via email to