From a7fe5ef75aaee54a2573232d0d27f81f3496efdd Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 15 Jun 2017 19:22:31 +0900 Subject: [PATCH 2/2] Teach ATExecAttachPartition to skip validation in more cases In cases where the table being attached is a partitioned table and the table itself does not have constraints that would allow validation on the whole table to be skipped, we can still skip the validations of individual partitions if they each happen to have the requisite constraints. Per an idea of Robert Haas', with code refactoring suggestions from Ashutosh Bapat. --- src/backend/commands/tablecmds.c | 209 ++++++++++++++++++++++----------------- 1 file changed, 117 insertions(+), 92 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 981b7ae902..b998dff35d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -473,6 +473,8 @@ static void CreateInheritance(Relation child_rel, Relation parent_rel); static void RemoveInheritance(Relation child_rel, Relation parent_rel); static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd); +static bool skipPartConstraintValidation(Relation partrel, + List *partConstraint); static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name); @@ -13413,9 +13415,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) Relation attachRel, catalog; List *attachRel_children; - TupleConstr *attachRel_constr; - List *partConstraint, - *existConstraint; + List *partConstraint; SysScanDesc scan; ScanKeyData skey; AttrNumber attno; @@ -13591,29 +13591,124 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) rel); /* - * Check if we can do away with having to scan the table being attached to - * validate the partition constraint, by *proving* that the existing - * constraints of the table *imply* the partition predicate. We include - * the table's check constraints and NOT NULL constraints in the list of - * clauses passed to predicate_implied_by(). - * - * There is a case in which we cannot rely on just the result of the - * proof. + * Based on the table's existing constraints, determine if we can skip the + * partition constraint validation scan. + */ + if (skipPartConstraintValidation(attachRel, partConstraint)) + { + ereport(INFO, + (errmsg("partition constraint for table \"%s\" is implied by existing constraints", + RelationGetRelationName(attachRel)))); + skip_validate = true; + } + + /* + * Set up to have the table be scanned to validate the partition + * constraint (see partConstraint above). If it's a partitioned table, we + * instead schedule its leaf partitions to be scanned. */ - attachRel_constr = tupleDesc->constr; - existConstraint = NIL; - if (attachRel_constr != NULL) + if (!skip_validate) { - int num_check = attachRel_constr->num_check; + ListCell *lc; + + /* + * We already collected the list of partitions, including the table + * named in the command itself, which should appear at the head of the + * list. + */ + Assert(list_length(attachRel_children) >= 1 && + linitial_oid(attachRel_children) == RelationGetRelid(attachRel)); + + foreach(lc, attachRel_children) + { + AlteredTableInfo *tab; + Oid part_relid = lfirst_oid(lc); + Relation part_rel; + List *my_partconstr; + + /* Skip the original table if it's partitioned. */ + if (part_relid == RelationGetRelid(attachRel) && + attachRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + continue; + + /* Lock already taken */ + part_rel = heap_open(part_relid, NoLock); + + /* + * Skip if it's a partitioned table. Only RELKIND_RELATION + * relations (ie, leaf partitions) need to be scanned. + */ + if (part_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + heap_close(part_rel, NoLock); + continue; + } + + /* + * Can we skip validating this partition? We already concluded + * negatively for attachRel. + */ + my_partconstr = partConstraint; + if (part_relid != RelationGetRelid(attachRel)) + { + /* + * Adjust the constraint that we constructed above for the + * table being attached so that it matches this partition's + * attribute numbers. + */ + my_partconstr = map_partition_varattnos(partConstraint, 1, + part_rel, + attachRel); + if (skipPartConstraintValidation(part_rel, my_partconstr)) + { + heap_close(part_rel, NoLock); + continue; + } + } + + /* Nope. So grab a work queue entry. */ + tab = ATGetQueueEntry(wqueue, part_rel); + tab->partition_constraint = (Expr *) linitial(my_partconstr); + + /* keep our lock until commit */ + heap_close(part_rel, NoLock); + } + } + + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(attachRel)); + + /* keep our lock until commit */ + heap_close(attachRel, NoLock); + + return address; +} + +/* + * skipPartConstraintValidation + * Can we skip partition constraint validation? + * + * This basically returns if the partrel's existing constraints, which + * includes its check constraints and column-level NOT NULL constraints, + * imply the partition constraint as described in partConstraint. + */ +static bool +skipPartConstraintValidation(Relation partrel, List *partConstraint) +{ + List *existConstraint = NIL; + TupleConstr *constr = RelationGetDescr(partrel)->constr; + + if (constr != NULL) + { + int num_check = constr->num_check; int i; - if (attachRel_constr->has_not_null) + if (constr->has_not_null) { - int natts = attachRel->rd_att->natts; + int natts = partrel->rd_att->natts; for (i = 1; i <= natts; i++) { - Form_pg_attribute att = attachRel->rd_att->attrs[i - 1]; + Form_pg_attribute att = partrel->rd_att->attrs[i - 1]; if (att->attnotnull && !att->attisdropped) { @@ -13647,10 +13742,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) * If this constraint hasn't been fully validated yet, we must * ignore it here. */ - if (!attachRel_constr->check[i].ccvalid) + if (!constr->check[i].ccvalid) continue; - cexpr = stringToNode(attachRel_constr->check[i].ccbin); + cexpr = stringToNode(constr->check[i].ccbin); /* * Run each expression through const-simplification and @@ -13669,80 +13764,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) /* And away we go ... */ if (predicate_implied_by(partConstraint, existConstraint, true)) - skip_validate = true; - } - - /* It's safe to skip the validation scan after all */ - if (skip_validate) - ereport(INFO, - (errmsg("partition constraint for table \"%s\" is implied by existing constraints", - RelationGetRelationName(attachRel)))); - - /* - * Set up to have the table be scanned to validate the partition - * constraint (see partConstraint above). If it's a partitioned table, we - * instead schedule its leaf partitions to be scanned. - */ - if (!skip_validate) - { - ListCell *lc; - - /* - * We already collected the list of partitions, including the table - * named in the command itself, which should appear at the head of the - * list. - */ - Assert(list_length(attachRel_children) >= 1 && - linitial_oid(attachRel_children) == RelationGetRelid(attachRel)); - - foreach(lc, attachRel_children) - { - AlteredTableInfo *tab; - Oid part_relid = lfirst_oid(lc); - Relation part_rel; - Expr *constr; - - /* Skip the original table if it's partitioned. */ - if (part_relid == RelationGetRelid(attachRel) && - attachRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - continue; - - /* Lock already taken */ - part_rel = heap_open(part_relid, NoLock); - - /* - * Skip if it's a partitioned table. Only RELKIND_RELATION - * relations (ie, leaf partitions) need to be scanned. - */ - if (part_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - { - heap_close(part_rel, NoLock); - continue; - } - - /* Grab a work queue entry */ - tab = ATGetQueueEntry(wqueue, part_rel); - - /* - * Adjust the constraint that we constructed above for the table - * being attached so that it matches this partition's attribute - * numbers. - */ - constr = linitial(partConstraint); - tab->partition_constraint = (Expr *) - map_partition_varattnos((List *) constr, 1, - part_rel, attachRel); - /* keep our lock until commit */ - heap_close(part_rel, NoLock); - } + return true; } - ObjectAddressSet(address, RelationRelationId, RelationGetRelid(attachRel)); - - /* keep our lock until commit */ - heap_close(attachRel, NoLock); - - return address; + return false; } /* -- 2.11.0