From 092828a7d7274b48bf33c88863a97585bff80ebb Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Tue, 23 Jan 2024 17:00:43 +0530 Subject: [PATCH 4/4] Separate function to merge next parent attribute Partition inherit from only a single parent. The logic to merge an attribute from the next parent into the corresponding attribute inherited from previous parents in MergeAttribute() is only applicable to regular inheritance children. This code is isolated enough that it can be separate into a function by itself. This separation makes MergeAttributes() more readable making it easy to follow high level logic without getting entangled into details. This separation revealed that the code handling NOT NULL constraints is duplicated in blocks merging the attribute definition incrementally. Deduplicate that code. Author: Ashutosh Bapat --- src/backend/commands/tablecmds.c | 352 ++++++++++++++++--------------- 1 file changed, 178 insertions(+), 174 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 843cc3bff6..3f0066b435 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -360,6 +360,8 @@ static List *MergeAttributes(List *columns, const List *supers, char relpersiste List **supnotnulls); static List *MergeCheckConstraint(List *constraints, const char *name, Node *expr); static bool MergeChildAttribute(ColumnDef *newdef, int newcol_attno, List *inh_columns); +static ColumnDef *MergeInheritedAttribute(ColumnDef *newdef, int parent_attno, + List *inh_columns, AttrMap *newattmap); 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, @@ -2704,9 +2706,8 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, Form_pg_attribute attribute = TupleDescAttr(tupleDesc, parent_attno - 1); char *attributeName = NameStr(attribute->attname); - int exist_attno; ColumnDef *newdef; - ColumnDef *savedef; + ColumnDef *mergedef; /* * Ignore dropped columns in the parent. @@ -2727,194 +2728,64 @@ MergeAttributes(List *columns, const List *supers, char relpersistence, /* * Regular inheritance children are independent enough not to - * inherit identity columns. But partitions are integral part of - * a partitioned table and inherit identity column. + * inherit identity columns. But partitions are integral part of a + * partitioned table and inherit identity column. */ if (is_partition) newdef->identity = attribute->attidentity; + mergedef = MergeInheritedAttribute(newdef, parent_attno, + inh_columns, newattmap); + /* - * Does it match some previously considered column from another - * parent? + * Partitions have only one parent, so conflict should never + * occur. */ - exist_attno = findAttrByName(attributeName, inh_columns); - if (exist_attno > 0) - { - ColumnDef *prevdef; - Oid prevtypeid, - newtypeid; - int32 prevtypmod, - newtypmod; - Oid prevcollid, - 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. - */ - ereport(NOTICE, - (errmsg("merging multiple inherited definitions of column \"%s\"", - attributeName))); - prevdef = list_nth_node(ColumnDef, inh_columns, exist_attno - 1); - - /* - * Must have the same type and typmod - */ - 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(prevtypeid, prevtypmod), - format_type_with_typemod(newtypeid, newtypmod)))); - - /* - * Must have the same collation - */ - 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(prevcollid), - get_collation_name(newcollid)))); + Assert(mergedef == NULL || !is_partition); - /* - * Copy/check storage parameter - */ - 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(prevdef->storage), - storage_name(newdef->storage)))); - - /* - * Copy/check compression parameter - */ - 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 - * get an extra not-null constraint. - */ - if (bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber, - pkattrs)) - { - CookedConstraint *nn; - - nn = palloc(sizeof(CookedConstraint)); - nn->contype = CONSTR_NOTNULL; - nn->conoid = InvalidOid; - nn->name = NULL; - nn->attnum = exist_attno; - nn->expr = NULL; - nn->skip_validation = false; - nn->is_local = false; - nn->inhcount = 1; - nn->is_no_inherit = false; - - nnconstraints = lappend(nnconstraints, nn); - } - - /* - * 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)) - prevdef->is_not_null = true; - - /* - * Check for GENERATED conflicts - */ - if (prevdef->generated != newdef->generated) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("inherited column \"%s\" has a generation conflict", - attributeName))); - - /* - * Default and other constraints are handled below - */ - - 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 + if (mergedef == NULL) { /* * No, create a new inherited column */ 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)) - newdef->is_not_null = true; inh_columns = lappend(inh_columns, newdef); newattmap->attnums[parent_attno - 1] = ++child_attno; - /* - * In regular inheritance, columns in the parent's primary key - * get an extra not-null constraint. Partitioning doesn't - * need this, because the PK itself is going to be cloned to - * the partition. - */ - if (!is_partition && - bms_is_member(parent_attno - - FirstLowInvalidHeapAttributeNumber, - pkattrs)) - { - CookedConstraint *nn; - - nn = palloc(sizeof(CookedConstraint)); - nn->contype = CONSTR_NOTNULL; - nn->conoid = InvalidOid; - nn->name = NULL; - nn->attnum = newattmap->attnums[parent_attno - 1]; - nn->expr = NULL; - nn->skip_validation = false; - nn->is_local = false; - nn->inhcount = 1; - nn->is_no_inherit = false; - - nnconstraints = lappend(nnconstraints, nn); - } + mergedef = newdef; + } + + /* 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)) + mergedef->is_not_null = true; - /* remember for default processing below */ - savedef = newdef; + /* + * In regular inheritance, columns in the parent's primary key get + * an extra not-null constraint. Partitioning doesn't need this, + * because the PK itself is going to be cloned to the partition. + */ + if (!is_partition && + bms_is_member(parent_attno - + FirstLowInvalidHeapAttributeNumber, + pkattrs)) + { + CookedConstraint *nn; + + nn = palloc(sizeof(CookedConstraint)); + nn->contype = CONSTR_NOTNULL; + nn->conoid = InvalidOid; + nn->name = NULL; + nn->attnum = newattmap->attnums[parent_attno - 1]; + nn->expr = NULL; + nn->skip_validation = false; + nn->is_local = false; + nn->inhcount = 1; + nn->is_no_inherit = false; + + nnconstraints = lappend(nnconstraints, nn); } /* @@ -2936,7 +2807,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, savedef); + cols_with_defaults = lappend(cols_with_defaults, mergedef); } } @@ -3451,6 +3322,139 @@ MergeChildAttribute(ColumnDef *newdef, int newcol_attno, List *inh_columns) return true; } +/* + * MergeInheritedAttribute + * Merge given parent attribute definition into any attribute, with + * the same name, inherited from the previous parents. + * + * Input arguments: + * 'newdef' is the new parent column/attribute definition to be merged. + * 'parent_attno' is the attribute number in parent table's schema definition + * 'inh_columns' is the list of previously inherited ColumnDefs. + * 'newattmap' is attribute map. + * + * Return value: + * True if the given attribute is merged into a previously 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. + * 'newattmap' is updated with matching inherited attribute's position. New + * 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 new attribute can not + * be merged into it, the function throws respective errors. + * (3) A partition inherits from only a single parent. Hence this + * function is applicable only to a regular inheritance. + */ +static ColumnDef * +MergeInheritedAttribute(ColumnDef *newdef, int parent_attno, List *inh_columns, + AttrMap *newattmap) +{ + char *attributeName = newdef->colname; + int exist_attno; + ColumnDef *prevdef; + Oid prevtypeid, + newtypeid; + int32 prevtypmod, + newtypmod; + Oid prevcollid, + newcollid; + + /* + * Does it match some previously considered column from another parent? + */ + exist_attno = findAttrByName(attributeName, inh_columns); + if (exist_attno <= 0) + return NULL; + + /* + * Yes, try to merge the two column definitions. + */ + ereport(NOTICE, + (errmsg("merging multiple inherited definitions of column \"%s\"", + attributeName))); + prevdef = list_nth_node(ColumnDef, inh_columns, exist_attno - 1); + + /* + * Must have the same type and typmod + */ + 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(prevtypeid, prevtypmod), + format_type_with_typemod(newtypeid, newtypmod)))); + + /* + * Must have the same collation + */ + 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(prevcollid), + get_collation_name(newcollid)))); + + /* + * Copy/check storage parameter + */ + 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(prevdef->storage), + storage_name(newdef->storage)))); + + /* + * Copy/check compression parameter + */ + 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))); + + /* + * Check for GENERATED conflicts + */ + if (prevdef->generated != newdef->generated) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("inherited column \"%s\" has a generation conflict", + attributeName))); + + /* + * Default and other constraints are handled by the caller. + */ + + 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; + + return prevdef; +} + /* * StoreCatalogInheritance * Updates the system catalogs with proper inheritance information. -- 2.25.1