From e8993ab2e00db5f8260ef9d9cafe9fbe069d54a2 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Tue, 23 Jan 2024 12:47:46 +0530 Subject: [PATCH 3/4] Separate function to merge a child attribute into matching inherited attribute The logic to merge a child attribute into matching inherited attribute in MergeAttribute() is only applicable to regular inheritance child. The code is isolated and coherent enough that it can be separated into a function of its own. This separation also makes MergeAttribute() more readable by making it easier to follow high level logic without getting entangled into details. Author: Ashutosh Bapat --- src/backend/commands/tablecmds.c | 337 +++++++++++++++++-------------- 1 file changed, 184 insertions(+), 153 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cde524083e..843cc3bff6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -359,6 +359,7 @@ static List *MergeAttributes(List *columns, const List *supers, char relpersiste bool is_partition, List **supconstr, List **supnotnulls); static List *MergeCheckConstraint(List *constraints, const char *name, Node *expr); +static bool MergeChildAttribute(ColumnDef *newdef, int newcol_attno, List *inh_columns); static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispartition); static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel); static void StoreCatalogInheritance(Oid relationId, List *supers, @@ -3066,167 +3067,21 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, foreach(lc, columns) { - ColumnDef *newdef = lfirst(lc); - char *attributeName = newdef->colname; - int exist_attno; + ColumnDef *newdef = lfirst_node(ColumnDef, lc); newcol_attno++; /* - * Does it match some inherited column? + * Partitions have only one parent and have no column definitions + * of their own, so conflict should never occur. */ - exist_attno = findAttrByName(attributeName, inh_columns); - if (exist_attno > 0) - { - ColumnDef *inhdef; - Oid inhtypeid, - newtypeid; - int32 inhtypmod, - newtypmod; - Oid inhcollid, - newcollid; - - /* - * Partitions have only one parent and have no column - * definitions of their own, so conflict should never occur. - */ - Assert(!is_partition); - - /* - * Yes, try to merge the two column definitions. - */ - if (exist_attno == newcol_attno) - ereport(NOTICE, - (errmsg("merging column \"%s\" with inherited definition", - attributeName))); - else - ereport(NOTICE, - (errmsg("moving and merging column \"%s\" with inherited definition", attributeName), - errdetail("User-specified column moved to the position of the inherited column."))); - inhdef = list_nth_node(ColumnDef, inh_columns, exist_attno - 1); - - /* - * Must have the same type and typmod - */ - 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(inhtypeid, inhtypmod), - format_type_with_typemod(newtypeid, newtypmod)))); - - /* - * Must have the same collation - */ - 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(inhcollid), - get_collation_name(newcollid)))); - - /* - * Identity is never inherited. The new column can have an - * identity definition, so we always just take that one. - */ - inhdef->identity = newdef->identity; - - /* - * Copy storage parameter - */ - 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(inhdef->storage), - storage_name(newdef->storage)))); - - /* - * Copy compression parameter - */ - if (inhdef->compression == NULL) - inhdef->compression = newdef->compression; - else if (newdef->compression != NULL) - { - 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", inhdef->compression, newdef->compression))); - } - - /* - * Merge of not-null constraints = OR 'em together - */ - inhdef->is_not_null |= newdef->is_not_null; - - /* - * Check for conflicts related to generated columns. - * - * If the parent column is generated, the child column will be - * made a generated column if it isn't already. If it is a - * generated column, we'll take its generation expression in - * preference to the parent's. We must check that the child - * column doesn't specify a default value or identity, which - * matches the rules for a single column in parse_utilcmd.c. - * - * Conversely, if the parent column is not generated, the - * child column can't be either. (We used to allow that, but - * it results in being able to override the generation - * expression via UPDATEs through the parent.) - */ - 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", - inhdef->colname))); - if (newdef->identity) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_DEFINITION), - errmsg("column \"%s\" inherits from generated column but specifies identity", - inhdef->colname))); - } - else - { - if (newdef->generated) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_DEFINITION), - errmsg("child column \"%s\" specifies generation expression", - inhdef->colname), - errhint("A child table column cannot be generated unless its parent column is."))); - } - - /* - * If new def has a default, override previous default - */ - if (newdef->raw_default != NULL) - { - inhdef->raw_default = newdef->raw_default; - inhdef->cooked_default = newdef->cooked_default; - } + Assert(!is_partition); - /* Mark the column as locally defined */ - inhdef->is_local = true; - } - else + if (!MergeChildAttribute(newdef, newcol_attno, inh_columns)) { /* - * No, attach new column to result columns + * No inherited attribute, attach new column to result + * columns. */ inh_columns = lappend(inh_columns, newdef); } @@ -3419,6 +3274,182 @@ MergeCheckConstraint(List *constraints, const char *name, Node *expr) return lappend(constraints, newcon); } +/* + * MergeChildAttribute + * Merge given child attribute definition into any inherited attribute with + * the same name. + * + * Input arguments: + * 'newdef' is the column/attribute definition from the child table. + * 'newcol_attno' is the attribute number in child table's schema definition + * 'inh_columns' is the list of inherited ColumnDefs. + * + * Return value: + * True if the given attribute is merged into an inherited attribute with the + * same name. False if no matching inherited attribute is found. When returning + * true, matching ColumnDef in 'inh_columns' list is modified. Child + * attribute's ColumnDef remains unchanged. + * + * Notes: + * (1) The attribute is merged according to the rules laid out in the prologue + * of MergeAttributes(). + * (2) If matching inherited attribute exists but the child attribute can not + * be merged into it, the function throws respective errors. + * (3) A partition can not have its own column definitions. Hence this + * function is applicable only to a regular inheritance child. + */ +static bool +MergeChildAttribute(ColumnDef *newdef, int newcol_attno, List *inh_columns) +{ + char *attributeName = newdef->colname; + int exist_attno; + ColumnDef *inhdef; + Oid inhtypeid, + newtypeid; + int32 inhtypmod, + newtypmod; + Oid inhcollid, + newcollid; + + /* + * Does it match some inherited column? + */ + exist_attno = findAttrByName(attributeName, inh_columns); + if (exist_attno <= 0) + return false; + + /* + * Yes, try to merge the two column definitions. + */ + if (exist_attno == newcol_attno) + ereport(NOTICE, + (errmsg("merging column \"%s\" with inherited definition", + attributeName))); + else + ereport(NOTICE, + (errmsg("moving and merging column \"%s\" with inherited definition", attributeName), + errdetail("User-specified column moved to the position of the inherited column."))); + inhdef = list_nth_node(ColumnDef, inh_columns, exist_attno - 1); + + /* + * Must have the same type and typmod + */ + 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(inhtypeid, inhtypmod), + format_type_with_typemod(newtypeid, newtypmod)))); + + /* + * Must have the same collation + */ + 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(inhcollid), + get_collation_name(newcollid)))); + + /* + * Identity is never inherited by a regular inheritance child. Pick + * child's identity definition if there's one. + */ + inhdef->identity = newdef->identity; + + /* + * Copy storage parameter + */ + 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(inhdef->storage), + storage_name(newdef->storage)))); + + /* + * Copy compression parameter + */ + if (inhdef->compression == NULL) + inhdef->compression = newdef->compression; + else if (newdef->compression != NULL) + { + 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", inhdef->compression, newdef->compression))); + } + + /* + * Merge of not-null constraints = OR 'em together + */ + inhdef->is_not_null |= newdef->is_not_null; + + /* + * Check for conflicts related to generated columns. + * + * If the parent column is generated, the child column will be made a + * generated column if it isn't already. If it is a generated column, + * we'll take its generation expression in preference to the parent's. We + * must check that the child column doesn't specify a default value or + * identity, which matches the rules for a single column in + * parse_utilcmd.c. + * + * Conversely, if the parent column is not generated, the child column + * can't be either. (We used to allow that, but it results in being able + * to override the generation expression via UPDATEs through the parent.) + */ + 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", + inhdef->colname))); + if (newdef->identity) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_DEFINITION), + errmsg("column \"%s\" inherits from generated column but specifies identity", + inhdef->colname))); + } + else + { + if (newdef->generated) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_DEFINITION), + errmsg("child column \"%s\" specifies generation expression", + inhdef->colname), + errhint("A child table column cannot be generated unless its parent column is."))); + } + + /* + * If new def has a default, override previous default + */ + if (newdef->raw_default != NULL) + { + inhdef->raw_default = newdef->raw_default; + inhdef->cooked_default = newdef->cooked_default; + } + + /* Mark the column as locally defined */ + inhdef->is_local = true; + + return true; +} /* * StoreCatalogInheritance -- 2.25.1