diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 7da2058..0d64adf 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -113,6 +113,16 @@ typedef struct PartitionRangeBound bool lower; /* this is the lower (vs upper) bound */ } PartitionRangeBound; +/* + * List of these elements is prepared while traversing a partition tree, + * so as to get a consistent order of partitions. + */ +typedef struct ParentChild +{ + Oid reloid; + Relation parent; /* Parent relation of reloid */ +} ParentChild; + static int32 qsort_partition_list_value_cmp(const void *a, const void *b, void *arg); static int32 qsort_partition_rbound_cmp(const void *a, const void *b, @@ -148,6 +158,8 @@ static int partition_bound_bsearch(PartitionKey key, PartitionBoundInfo boundinfo, void *probe, bool probe_is_bound, bool *is_equal); +static List *append_rel_partition_oids(List *rel_list, Relation rel); + /* * RelationBuildPartitionDesc * Form rel's partition descriptor @@ -999,20 +1011,9 @@ get_partition_qual_relid(Oid relid) return result; } -/* - * Append OIDs of rel's partitions to the list 'partoids' and for each OID, - * append pointer rel to the list 'parents'. - */ -#define APPEND_REL_PARTITION_OIDS(rel, partoids, parents) \ - do\ - {\ - int i;\ - for (i = 0; i < (rel)->rd_partdesc->nparts; i++)\ - {\ - (partoids) = lappend_oid((partoids), (rel)->rd_partdesc->oids[i]);\ - (parents) = lappend((parents), (rel));\ - }\ - } while(0) +#ifdef DEBUG_PRINT_OIDS +static void print_oids(List *oid_list); +#endif /* * RelationGetPartitionDispatchInfo @@ -1026,11 +1027,13 @@ PartitionDispatch * RelationGetPartitionDispatchInfo(Relation rel, int lockmode, int *num_parted, List **leaf_part_oids) { + PartitionWalker walker; PartitionDispatchData **pd; - List *all_parts = NIL, - *all_parents = NIL, - *parted_rels, + Relation partrel; + Relation parent; + List *parted_rels, *parted_rel_parents; + List *inhOIDs; ListCell *lc1, *lc2; int i, @@ -1041,21 +1044,28 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode, * Lock partitions and make a list of the partitioned ones to prepare * their PartitionDispatch objects below. * - * Cannot use find_all_inheritors() here, because then the order of OIDs - * in parted_rels list would be unknown, which does not help, because we - * assign indexes within individual PartitionDispatch in an order that is - * predetermined (determined by the order of OIDs in individual partition - * descriptors). + * Must call find_all_inheritors() here so as to lock the partitions in a + * consistent order (by oid values) to prevent deadlocks. But we assign + * indexes within individual PartitionDispatch in a different order + * (determined by the order of OIDs in individual partition descriptors). + * So, rather than using the oids returned by find_all_inheritors(), we + * generate canonically ordered oids using partition walker. */ + inhOIDs = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL); + list_free(inhOIDs); + + partition_walker_init(&walker, rel); + parent = NULL; *num_parted = 1; parted_rels = list_make1(rel); /* Root partitioned table has no parent, so NULL for parent */ parted_rel_parents = list_make1(NULL); - APPEND_REL_PARTITION_OIDS(rel, all_parts, all_parents); - forboth(lc1, all_parts, lc2, all_parents) + + /* Go to the next partition */ + partrel = partition_walker_next(&walker, &parent); + + for (; partrel != NULL; partrel = partition_walker_next(&walker, &parent)) { - Relation partrel = heap_open(lfirst_oid(lc1), lockmode); - Relation parent = lfirst(lc2); PartitionDesc partdesc = RelationGetPartitionDesc(partrel); /* @@ -1067,7 +1077,6 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode, (*num_parted)++; parted_rels = lappend(parted_rels, partrel); parted_rel_parents = lappend(parted_rel_parents, parent); - APPEND_REL_PARTITION_OIDS(partrel, all_parts, all_parents); } else heap_close(partrel, NoLock); @@ -1171,6 +1180,10 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode, offset += m; } +#ifdef DEBUG_PRINT_OIDS + print_oids(*leaf_part_oids); +#endif + return pd; } @@ -2331,3 +2344,100 @@ partition_bound_bsearch(PartitionKey key, PartitionBoundInfo boundinfo, return lo; } + +/* + * partition_walker_init + * + * Using the passed partitioned relation, expand it into its partitions using + * its partition descriptor, and make a partition rel list out of those. The + * rel passed in itself is not kept part of the partition list. The caller + * should handle the first rel separately before calling this function. + */ +void +partition_walker_init(PartitionWalker *walker, Relation rel) +{ + memset(walker, 0, sizeof(PartitionWalker)); + + if (RelationGetPartitionDesc(rel)) + walker->rels_list = append_rel_partition_oids(walker->rels_list, rel); + + /* Assign the first one as the current partition cell */ + walker->cur_cell = list_head(walker->rels_list); +} + +/* + * partition_walker_next + * + * Get the next partition in the partition tree. + * At the same time, if the partition is a partitioned table, append its + * children at the end, so that the next time we can traverse through these. + */ +Relation +partition_walker_next(PartitionWalker *walker, Relation *parent) +{ + ParentChild *pc; + Relation partrel; + + if (walker->cur_cell == NULL) + return NULL; + + pc = (ParentChild *) lfirst(walker->cur_cell); + if (parent) + *parent = pc->parent; + + /* Open partrel without locking; find_all_inheritors() has locked it */ + partrel = heap_open(pc->reloid, NoLock); + + /* + * Append the children of partrel to the same list that we are + * iterating on. + */ + if (RelationGetPartitionDesc(partrel)) + walker->rels_list = append_rel_partition_oids(walker->rels_list, + partrel); + + /* Bump the cur_cell here at the end, because above, we modify the list */ + walker->cur_cell = lnext(walker->cur_cell); + + return partrel; +} + +/* + * append_rel_partition_oids + * + * Append OIDs of rel's partitions to the list 'rel_list' and for each OID, + * also store parent rel. + */ +static +List *append_rel_partition_oids(List *rel_list, Relation rel) +{ + int i; + PartitionDescData *partdesc = RelationGetPartitionDesc(rel); + + Assert(partdesc); + + for (i = 0; i < partdesc->nparts; i++) + { + ParentChild *pc = palloc(sizeof(ParentChild)); + pc->parent = rel; + pc->reloid = rel->rd_partdesc->oids[i]; + rel_list = lappend(rel_list, pc); + } + return rel_list; +} + +#ifdef DEBUG_PRINT_OIDS +static void +print_oids(List *oid_list) +{ + ListCell *cell; + StringInfoData oids_str; + + initStringInfo(&oids_str); + foreach(cell, oid_list) + { + appendStringInfo(&oids_str, "%s ", get_rel_name(lfirst_oid(cell))); + } + elog(NOTICE, "leaf oids: %s", oids_str.data); +} +#endif diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index cf46b74..e9856c4 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -33,6 +33,7 @@ #include "access/heapam.h" #include "access/htup_details.h" #include "access/sysattr.h" +#include "catalog/partition.h" #include "catalog/pg_inherits_fn.h" #include "catalog/pg_type.h" #include "miscadmin.h" @@ -99,6 +100,8 @@ static List *generate_append_tlist(List *colTypes, List *colCollations, static List *generate_setop_grouplist(SetOperationStmt *op, List *targetlist); static void expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti); +static Relation get_next_child(Relation oldrelation, ListCell **cell, + PartitionWalker *walker, bool is_partitioned); static void make_inh_translation_list(Relation oldrelation, Relation newrelation, Index newvarno, @@ -1370,12 +1373,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) Oid parentOID; PlanRowMark *oldrc; Relation oldrelation; + Relation newrelation; LOCKMODE lockmode; List *inhOIDs; List *appinfos; - ListCell *l; + ListCell *oids_cell; bool need_append; + bool is_partitioned_resultrel; PartitionedChildRelInfo *pcinfo; + PartitionWalker walker; List *partitioned_child_rels = NIL; /* Does RT entry allow inheritance? */ @@ -1446,23 +1452,54 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) */ oldrelation = heap_open(parentOID, NoLock); + /* + * Remember whether it is a result relation and it is partitioned. We need + * to decide the ordering of result rels based on this. + */ + is_partitioned_resultrel = + (oldrelation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE + && rti == parse->resultRelation); + /* Scan the inheritance set and expand it */ appinfos = NIL; need_append = false; - foreach(l, inhOIDs) + newrelation = oldrelation; + + /* For non-partitioned result-rels, open the first child from inhOIDs */ + if (!is_partitioned_resultrel) + { + oids_cell = list_head(inhOIDs); + newrelation = get_next_child(oldrelation, &oids_cell, &walker, + is_partitioned_resultrel); + } + else { - Oid childOID = lfirst_oid(l); - Relation newrelation; + /* + * For partitioned resultrels, we don't need the inhOIDs list itself, + * because we anyways traverse the tree in canonical order; but we do + * want to lock all the children in a consistent order (see + * find_inheritance_children), so as to avoid unnecessary deadlocks. + * Hence, the call to find_all_inheritors() above. The aim is to + * generate the appinfos in canonical order so that the result rels, + * if generated later, are in the same order as those of the leaf + * partitions that are maintained during insert/update tuple routing. + * Maintaining same order would speed up searching for a given leaf + * partition in these result rels. + */ + list_free(inhOIDs); + inhOIDs = NIL; + partition_walker_init(&walker, oldrelation); + } + + for (; newrelation != NULL; + newrelation = get_next_child(oldrelation, &oids_cell, &walker, + is_partitioned_resultrel)) + { + Oid childOID = RelationGetRelid(newrelation); RangeTblEntry *childrte; Index childRTindex; AppendRelInfo *appinfo; - /* Open rel if needed; we already have required locks */ - if (childOID != parentOID) - newrelation = heap_open(childOID, NoLock); - else - newrelation = oldrelation; - /* * It is possible that the parent table has children that are temp * tables of other backends. We cannot safely access such tables @@ -1575,6 +1612,10 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) heap_close(oldrelation, NoLock); +#ifdef DEBUG_PRINT_OIDS + print_oids(appinfos, parse->rtable); +#endif + /* * If all the children were temp tables or a partitioned parent did not * have any leaf partitions, pretend it's a non-inheritance situation; we @@ -1612,6 +1653,45 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) } /* + * Get the next child in an inheritance tree. + * + * This function is called to traverse two different types of lists. If it's a + * list containing partitions, is_partitioned is true, and 'walker' is valid. + * Otherwise, 'cell' points to a position in the list of inheritance children. + * For partitions walker, the partition traversal is done using canonical + * ordering. Whereas, for inheritence children, list is already prepared, and + * is ordered depending upon the pg_inherit scan. + * + * oldrelation is the root relation in the inheritence tree. This is unused in + * case of is_partitioned=true. + */ +static Relation +get_next_child(Relation oldrelation, ListCell **cell, PartitionWalker *walker, + bool is_partitioned) +{ + if (is_partitioned) + return partition_walker_next(walker, NULL); + else + { + Oid childOID; + + if (!*cell) + return NULL; /* We are done with the list */ + + childOID = lfirst_oid(*cell); + + /* Prepare to get the next child. */ + *cell = lnext(*cell); + + /* If it's the root relation, it is already open */ + if (childOID != RelationGetRelid(oldrelation)) + return heap_open(childOID, NoLock); + else + return oldrelation; + } +} + +/* * make_inh_translation_list * Build the list of translations from parent Vars to child Vars for * an inheritance child. @@ -2161,3 +2241,21 @@ adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node *node, /* Now translate for this child */ return adjust_appendrel_attrs(root, node, appinfo); } + +#ifdef DEBUG_PRINT_OIDS +static void +print_oids(List *oid_list, List *rtable) +{ + ListCell *cell; + StringInfoData oids_str; + + initStringInfo(&oids_str); + foreach(cell, oid_list) + { + AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(cell); + RangeTblEntry *childrte = (RangeTblEntry *) list_nth(rtable, appinfo->child_relid-1); + appendStringInfo(&oids_str, "%s ", get_rel_name(childrte->relid)); + } + elog(NOTICE, "expanded oids: %s", oids_str.data); +} +#endif diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index f10879a..2662850 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -68,6 +68,12 @@ typedef struct PartitionDispatchData int *indexes; } PartitionDispatchData; +typedef struct PartitionWalker +{ + List *rels_list; + ListCell *cur_cell; +} PartitionWalker; + typedef struct PartitionDispatchData *PartitionDispatch; extern void RelationBuildPartitionDesc(Relation relation); @@ -84,6 +90,10 @@ extern List *map_partition_varattnos(List *expr, int target_varno, extern List *RelationGetPartitionQual(Relation rel); extern Expr *get_partition_qual_relid(Oid relid); +extern void partition_walker_init(PartitionWalker *walker, Relation rel); +extern Relation partition_walker_next(PartitionWalker *walker, + Relation *parent); + /* For tuple routing */ extern PartitionDispatch *RelationGetPartitionDispatchInfo(Relation rel, int lockmode, int *num_parted,