On 29.08.23 13:20, Alvaro Herrera wrote:
On 2023-Aug-29, Peter Eisentraut wrote:
@@ -3278,13 +3261,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.
This wording confused me:
"If the constraint doesn't match or conflict with an existing one, a new
constraint is appended to the list."
I first read it as "doesn't match or conflicts with ..." (i.e., the
negation only applied to the first verb, not both) which would have been
surprising (== broken) behavior.
I think it's clearer if you say "doesn't match nor conflict", but I'm
not sure if this is grammatically correct.
Here is an updated version of this patch set. I resolved some conflicts
and addressed this comment of yours. I also dropped the one patch with
some catalog header edits that people didn't seem to particularly like.
The patches that are now 0001 through 0004 were previously reviewed and
just held for the not-null constraint patches, I think, so I'll commit
them in a few days if there are no objections.
Patches 0005 through 0007 are also ready in my opinion, but they haven't
really been reviewed, so this would be something for reviewers to focus
on. (0005 and 0007 are trivial, but they go to together with 0006.)
The remaining 0008 and 0009 were still under discussion and contemplation.
From 28e4dbba35fc3162c13f5896551921896cf30d1c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:34 +0200
Subject: [PATCH v3 1/9] 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 | 123 ++++++++++++-------------------
1 file changed, 48 insertions(+), 75 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a2c671b66..ff001f5ceb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15695,53 +15695,39 @@ 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)));
/*
* If the parent has a not-null constraint that's not
NO INHERIT,
@@ -15749,40 +15735,38 @@ MergeAttributesIntoExisting(Relation child_rel,
Relation parent_rel)
*
* Other constraints are checked elsewhere.
*/
- if (attribute->attnotnull && !childatt->attnotnull)
+ if (parent_att->attnotnull && !child_att->attnotnull)
{
HeapTuple contup;
contup =
findNotNullConstraintAttnum(RelationGetRelid(parent_rel),
-
attribute->attnum);
+
parent_att->attnum);
if (HeapTupleIsValid(contup) &&
!((Form_pg_constraint)
GETSTRUCT(contup))->connoinherit)
ereport(ERROR,
errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("column \"%s\"
in child table must be marked NOT NULL",
-
attributeName));
+
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"));
@@ -15792,10 +15776,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);
@@ -15805,8 +15789,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)));
}
}
@@ -15833,27 +15816,20 @@ 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;
Oid parent_relid = RelationGetRelid(parent_rel);
- 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(parent_relid));
- 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)))
@@ -15877,7 +15853,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)))
@@ -15892,10 +15868,12 @@ MergeConstraintsIntoExisting(Relation child_rel,
Relation parent_rel)
* CHECK constraint are matched by name, NOT NULL ones
by
* attribute number
*/
- if (child_con->contype == CONSTRAINT_CHECK &&
- strcmp(NameStr(parent_con->conname),
- NameStr(child_con->conname)) != 0)
- continue;
+ if (child_con->contype == CONSTRAINT_CHECK)
+ {
+ if (strcmp(NameStr(parent_con->conname),
+ NameStr(child_con->conname))
!= 0)
+ continue;
+ }
else if (child_con->contype == CONSTRAINT_NOTNULL)
{
AttrNumber parent_attno =
extractNotNullColumn(parent_tuple);
@@ -15908,12 +15886,11 @@ MergeConstraintsIntoExisting(Relation child_rel,
Relation parent_rel)
}
if (child_con->contype == CONSTRAINT_CHECK &&
- !constraints_equivalent(parent_tuple,
child_tuple, tuple_desc))
+ !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 CHECK child constraint is "no inherit" then
cannot
@@ -15931,8 +15908,7 @@ MergeConstraintsIntoExisting(Relation child_rel,
Relation parent_rel)
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
@@ -15942,8 +15918,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
@@ -15965,13 +15940,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;
@@ -15998,7 +15973,7 @@ MergeConstraintsIntoExisting(Relation child_rel,
Relation parent_rel)
}
systable_endscan(parent_scan);
- table_close(catalog_relation, RowExclusiveLock);
+ table_close(constraintrel, RowExclusiveLock);
}
/*
@@ -16154,11 +16129,9 @@ RemoveInheritance(Relation child_rel, Relation
parent_rel, bool expect_detached)
List *connames;
List *nncolumns;
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),
@@ -16166,7 +16139,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\"",
@@ -16320,7 +16293,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.42.0
From c5653b746bd8611c7297fb43013ab4f8adee8b40 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:34 +0200
Subject: [PATCH v3 2/9] 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 | 48 +++++++++++++++-----------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ff001f5ceb..b73f4c96a4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -353,7 +353,7 @@ static void RangeVarCallbackForTruncate(const RangeVar
*relation,
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
bool is_partition,
List **supconstr,
List **supnotnulls);
-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,
@@ -2913,24 +2913,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);
}
}
@@ -3277,13 +3260,17 @@ 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 new constraint matches an existing one, then the existing
+ * constraint's inheritance count is updated. If there is a conflict (same
+ * name but different expression), throw an error. If the constraint neither
+ * matches nor conflicts with an existing one, a new constraint is appended to
+ * the list.
*/
-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)
{
@@ -3297,13 +3284,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,
@@ -3312,7 +3299,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.42.0
From b558f35954be5c232d4ff2bb23313d6ce0cceb49 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:34 +0200
Subject: [PATCH v3 3/9] 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 b73f4c96a4..ad398e837d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -350,7 +350,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,
List **supnotnulls);
static List *MergeCheckConstraint(List *constraints, const char *name, Node
*expr);
@@ -361,7 +361,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,
@@ -2307,7 +2307,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.
@@ -2369,17 +2369,17 @@ 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
**supnotnulls)
{
- List *inhSchema = NIL;
+ List *inh_columns = NIL;
List *constraints = NIL;
List *nnconstraints = 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
@@ -2392,7 +2392,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",
@@ -2406,15 +2406,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)
{
@@ -2431,9 +2431,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)
{
@@ -2447,7 +2447,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,
@@ -2467,18 +2467,18 @@ 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;
@@ -2486,7 +2486,6 @@ MergeAttributes(List *schema, List *supers, char
relpersistence,
List *inherited_defaults;
List *cols_with_defaults;
List *nnconstrs;
- AttrNumber parent_attno;
ListCell *lc1;
ListCell *lc2;
Bitmapset *pkattrs;
@@ -2507,8 +2506,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\"",
@@ -2593,7 +2591,7 @@ MergeAttributes(List *schema, List *supers, char
relpersistence,
nncols = bms_add_member(nncols,
((CookedConstraint *) lfirst(lc1))->attnum);
- 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,
@@ -2611,7 +2609,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;
@@ -2624,7 +2622,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
@@ -2761,7 +2759,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;
/*
@@ -2862,7 +2860,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)
@@ -2882,9 +2880,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;
@@ -2945,27 +2942,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;
@@ -2985,7 +2982,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)));
@@ -2993,7 +2990,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
@@ -3118,19 +3115,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",
@@ -3144,13 +3141,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);
@@ -3222,9 +3219,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)
{
@@ -3247,7 +3244,7 @@ MergeAttributes(List *schema, List *supers, char
relpersistence,
*supconstr = constraints;
*supnotnulls = nnconstraints;
- return schema;
+ return columns;
}
@@ -3402,22 +3399,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.42.0
From 0a5593983555e669a9fab8e366c6a36b26ebac14 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 4/9] 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 253d6c86f8..ce2c7bce85 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -927,3 +927,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 ad398e837d..73b8dea81c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2795,22 +2795,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 55c315f0e2..cf0d432ab1 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1358,20 +1358,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 f6cc28a661..ffd2874ee3 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -151,4 +151,6 @@ extern TupleDesc BuildDescForRelation(const List *columns);
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.42.0
From 402174806f36bb1a240f5afbeacad3ffd9292a9a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 5/9] 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 | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/common/tupdesc.c
b/src/backend/access/common/tupdesc.c
index ce2c7bce85..c2e7b14c31 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 73b8dea81c..60ede984e0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -941,8 +941,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);
--
2.42.0
From ad08dbbb459683da6206afb5cdc11164bbb94fc2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 6/9] 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 c2e7b14c31..d119cfafb5 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 60ede984e0..7bd73eb379 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1277,6 +1277,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 ffd2874ee3..d833d5f2e1 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);
extern Node *TupleDescGetDefault(TupleDesc tupdesc, AttrNumber attnum);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 16b6126669..a9c6825601 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.42.0
From 65d2d76d52b049b2e13a16018291225d844cefe0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 7/9] 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 | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7bd73eb379..8820738d91 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -940,10 +940,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);
}
/*
@@ -1346,8 +1342,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;
@@ -1356,6 +1350,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)
--
2.42.0
From 5b8ffd08e58eda3176f6c6caf5b64c74a902cda4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 8/9] 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 | 89 ++++++++------------------------
1 file changed, 22 insertions(+), 67 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8820738d91..d8d7ba904d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6968,22 +6968,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)
@@ -7105,59 +7098,21 @@ 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;
+ /*
+ * Construct new attribute's pg_attribute entry.
+ */
+ tupdesc = BuildDescForRelation(list_make1(colDef));
- aclresult = object_aclcheck(TypeRelationId, typeOid, GetUserId(),
ACL_USAGE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error_type(aclresult, typeOid);
+ attribute = TupleDescAttr(tupdesc, 0);
- collOid = GetColumnDefCollation(NULL, colDef, typeOid);
+ /* Fix up attribute number */
+ attribute->attnum = newattnum;
/* make sure datatype is legal for a column */
- CheckAttributeType(colDef->colname, typeOid, collOid,
+ CheckAttributeType(NameStr(attribute->attname), attribute->atttypid,
attribute->attcollation,
list_make1_oid(rel->rd_rel->reltype),
0);
- /*
- * Construct new attribute's pg_attribute entry. (Variable-length
fields
- * are handled by InsertPgAttributeTuples().)
- */
- 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;
- 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;
-
- ReleaseSysCache(typeTuple);
-
- tupdesc = CreateTupleDesc(lengthof(aattr), (FormData_pg_attribute **)
&aattr);
-
InsertPgAttributeTuples(attrdesc, tupdesc, myrelid, NULL, NULL);
table_close(attrdesc, RowExclusiveLock);
@@ -7187,7 +7142,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);
/*
@@ -7261,7 +7216,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;
@@ -7269,23 +7224,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);
@@ -7298,17 +7253,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,
@@ -7321,8 +7276,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.42.0
From a1000260efb736f2c1e6c161a1475676228a8be8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 12 Jul 2023 16:12:35 +0200
Subject: [PATCH v3 9/9] 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 | 181 ++++++++++++++++---------------
1 file changed, 94 insertions(+), 87 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d8d7ba904d..9b9b5ad5e8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2697,7 +2697,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.
@@ -2706,14 +2707,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->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.
@@ -2721,68 +2738,61 @@ 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)));
/*
* In regular inheritance, columns in the
parent's primary key
@@ -2816,12 +2826,12 @@ MergeAttributes(List *columns, const List *supers, char
relpersistence,
if (bms_is_member(parent_attno, nncols) ||
bms_is_member(parent_attno -
FirstLowInvalidHeapAttributeNumber,
pkattrs))
- def->is_not_null = true;
+ newdef->is_not_null = true;
/*
* 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",
@@ -2831,34 +2841,30 @@ 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;
+ newdef->inhcount = 1;
+ newdef->is_local = false;
/* mark attnotnull if parent has it and it's
not NO INHERIT */
if (bms_is_member(parent_attno, nncols) ||
bms_is_member(parent_attno -
FirstLowInvalidHeapAttributeNumber,
pkattrs))
- def->is_not_null = true;
- 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->is_not_null = true;
+ inh_columns = lappend(inh_columns, newdef);
newattmap->attnums[parent_attno - 1] =
++child_attno;
/*
@@ -2887,6 +2893,9 @@ MergeAttributes(List *columns, const List *supers, char
relpersistence,
nnconstraints = lappend(nnconstraints,
nn);
}
+
+ /* remember for default processing below */
+ savedef = newdef;
}
/*
@@ -2908,7 +2917,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);
}
}
@@ -3046,17 +3055,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;
/*
@@ -3076,77 +3085,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.
@@ -3163,18 +3170,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
{
@@ -3182,7 +3189,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.")));
}
@@ -3191,12 +3198,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.42.0
