public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: tablecmds.c/MergeAttributes() cleanup
Date: Wed, 6 Dec 2023 09:23:07 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
On 05.10.23 17:49, Peter Eisentraut wrote:
> On 19.09.23 15:11, Peter Eisentraut wrote:
>> 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.
>
> I have committed through 0007, and I'll now close this patch set as
> "Committed", and I will (probably) bring back the rest (especially 0008)
> as part of a different patch set soon.
After playing with this for, well, 2 months, and considering various
other approaches, I would like to bring back the remaining two patches
in unchanged form.
Especially the (now) first patch "Refactor ATExecAddColumn() to use
BuildDescForRelation()" would be very helpful for facilitating further
refactoring in this area, because it avoids having two essentially
duplicate pieces of code responsible for converting a ColumnDef node
into internal form.
One of your (Álvaro's) comments about this earlier was
> Hmm, crazy. I'm not sure I like this, because it seems much too clever.
> The number of lines that are deleted is alluring, though.
>
> Maybe it'd be better to create a separate routine that takes a single
> ColumnDef and returns the Form_pg_attribute element for it; then use
> that in both BuildDescForRelation and ATExecAddColumn.
which was also my thought at the beginning. However, this wouldn't
quite work that way, for several reasons. For instance,
BuildDescForRelation() also needs to keep track of the has_not_null
property across all fields, and so if you split that function up, you
would have to somehow make that an output argument and have the caller
keep track of it. Also, the output of BuildDescForRelation() in
ATExecAddColumn() is passed into InsertPgAttributeTuples(), which
requires a tuple descriptor anyway, so splitting this up into a
per-attribute function would then require ATExecAddColumn() to
re-assemble a tuple descriptor anyway, so this wouldn't save anything.
Also note that ATExecAddColumn() could in theory be enhanced to add more
than one column in one go, so having this code structure in place isn't
inconsistent with that.
The main hackish thing, I suppose, is that we have to fix up the
attribute number after returning from BuildDescForRelation(). I suppose
we could address that by passing in a starting attribute number (or
alternatively maximum existing attribute number) into
BuildDescForRelation(). I think that would be okay; it would probably
be about a wash in terms of code added versus saved.
The (now) second patch is also still of interest to me, but I have since
noticed that I think [0] should be fixed first, but to make that fix
simpler, I would like the first patch from here.
[0]:
https://www.postgresql.org/message-id/flat/24656cec-d6ef-4d15-8b5b-e8dfc9c833a7%40eisentraut.org
From 42615f4c523920c7ae916ba0b1819cc6a5e622b1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 5 Oct 2023 16:17:16 +0200
Subject: [PATCH v4 1/2] 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.
Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
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 6b0a20010e..875cfeaa5a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6965,22 +6965,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)
@@ -7102,59 +7095,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);
@@ -7184,7 +7139,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);
/*
@@ -7258,7 +7213,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;
@@ -7266,23 +7221,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);
@@ -7295,17 +7250,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,
@@ -7318,8 +7273,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
base-commit: 7636725b922c8cd68f21d040f3542d3bce9c68a4
--
2.43.0
From 8fe0e8157d932c33ab06e0eccdfa0e2634e27935 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 5 Oct 2023 16:17:16 +0200
Subject: [PATCH v4 2/2] 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".
Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
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 875cfeaa5a..2cd8aefb35 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2694,7 +2694,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.
@@ -2703,14 +2704,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.
@@ -2718,68 +2735,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
@@ -2813,12 +2823,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",
@@ -2828,34 +2838,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;
/*
@@ -2884,6 +2890,9 @@ MergeAttributes(List *columns, const List *supers, char relpersistence,
nnconstraints = lappend(nnconstraints, nn);
}
+
+ /* remember for default processing below */
+ savedef = newdef;
}
/*
@@ -2905,7 +2914,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);
}
}
@@ -3043,17 +3052,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;
/*
@@ -3073,77 +3082,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.
@@ -3160,18 +3167,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
{
@@ -3179,7 +3186,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.")));
}
@@ -3188,12 +3195,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.43.0
Attachments:
[text/plain] v4-0001-Refactor-ATExecAddColumn-to-use-BuildDescForRelat.patch (7.2K, ../[email protected]/2-v4-0001-Refactor-ATExecAddColumn-to-use-BuildDescForRelat.patch)
download | inline diff:
From 42615f4c523920c7ae916ba0b1819cc6a5e622b1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 5 Oct 2023 16:17:16 +0200
Subject: [PATCH v4 1/2] 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.
Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
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 6b0a20010e..875cfeaa5a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6965,22 +6965,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)
@@ -7102,59 +7095,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);
@@ -7184,7 +7139,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);
/*
@@ -7258,7 +7213,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;
@@ -7266,23 +7221,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);
@@ -7295,17 +7250,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,
@@ -7318,8 +7273,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
base-commit: 7636725b922c8cd68f21d040f3542d3bce9c68a4
--
2.43.0
[text/plain] v4-0002-MergeAttributes-convert-pg_attribute-back-to-Colu.patch (16.6K, ../[email protected]/3-v4-0002-MergeAttributes-convert-pg_attribute-back-to-Colu.patch)
download | inline diff:
From 8fe0e8157d932c33ab06e0eccdfa0e2634e27935 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 5 Oct 2023 16:17:16 +0200
Subject: [PATCH v4 2/2] 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".
Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
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 875cfeaa5a..2cd8aefb35 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2694,7 +2694,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.
@@ -2703,14 +2704,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.
@@ -2718,68 +2735,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
@@ -2813,12 +2823,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",
@@ -2828,34 +2838,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;
/*
@@ -2884,6 +2890,9 @@ MergeAttributes(List *columns, const List *supers, char relpersistence,
nnconstraints = lappend(nnconstraints, nn);
}
+
+ /* remember for default processing below */
+ savedef = newdef;
}
/*
@@ -2905,7 +2914,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);
}
}
@@ -3043,17 +3052,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;
/*
@@ -3073,77 +3082,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.
@@ -3160,18 +3167,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
{
@@ -3179,7 +3186,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.")));
}
@@ -3188,12 +3195,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.43.0
view thread (16+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: tablecmds.c/MergeAttributes() cleanup
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox