From 3487d63069a2ba7cb205ded31be235bcf08fc0fa Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 29 Jun 2017 16:39:07 +0900 Subject: [PATCH 1/2] Simplify code that checks range partition overlap While making the logic a sightly easier to reason about, a latent bug in the logic was fixed in this simplification process, whereby false positives would occur (partitions that don't actually overlap would be concluded to overlap). --- src/backend/catalog/partition.c | 64 ++++++++++-------------------- src/test/regress/expected/create_table.out | 2 +- 2 files changed, 23 insertions(+), 43 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index f8c55b1fe7..457b2fef66 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -753,68 +753,48 @@ check_new_partition_bound(char *relname, Relation parent, boundinfo->strategy == PARTITION_STRATEGY_RANGE); /* - * Firstly, find the greatest range bound that is less - * than or equal to the new lower bound. + * Find the greatest range bound among those of the + * existing partitions that is less than or equal to the + * new lower bound. */ off1 = partition_bound_bsearch(key, boundinfo, lower, true, &equal); /* - * off1 == -1 means that all existing bounds are greater - * than the new lower bound. In that case and the case - * where no partition is defined between the bounds at - * off1 and off1 + 1, we have a "gap" in the range that - * could be occupied by the new partition. We confirm if - * so by checking whether the new upper bound is confined - * within the gap. + * If there is a "gap" in the range immediately on the + * right of the returned bound (one at off1, which the new + * lower bound is greater than or equal to), then the + * new partition could occupy the same. Only if its upper + * bound is also confined within the gap. */ - if (!equal && boundinfo->indexes[off1 + 1] < 0) + if (boundinfo->indexes[off1 + 1] < 0) { + equal = false; off2 = partition_bound_bsearch(key, boundinfo, upper, true, &equal); - /* - * If the new upper bound is returned to be equal to - * the bound at off2, the latter must be the upper - * bound of some partition with which the new - * partition clearly overlaps. - * - * Also, if bound at off2 is not same as the one - * returned for the new lower bound (IOW, off1 != - * off2), then the new partition overlaps at least one - * partition. + * If the new upper bound turns out to be crossing + * over the available gap, then the new partition + * overlaps with the partition(s) on the right. + * However, because a partition's upper bound can be + * equal to the lower bound of the partition + * immediately on the the right, discount that case. */ - if (equal || off1 != off2) + if (off2 > off1 + 1 || ((off2 == off1 + 1) && !equal)) { overlap = true; /* - * The bound at off2 could be the lower bound of - * the partition with which the new partition - * overlaps. In that case, use the upper bound - * (that is, the bound at off2 + 1) to get the - * index of that partition. + * Although the new upper bound might have crossed + * over multiple partitions on the right, we only + * ever report the immediately adjacent one. */ - if (boundinfo->indexes[off2] < 0) - with = boundinfo->indexes[off2 + 1]; - else - with = boundinfo->indexes[off2]; + with = boundinfo->indexes[off1 + 2]; } } else { - /* - * Equal has been set to true and there is no "gap" - * between the bound at off1 and that at off1 + 1, so - * the new partition will overlap some partition. In - * the former case, the new lower bound is found to be - * equal to the bound at off1, which could only ever - * be true if the latter is the lower bound of some - * partition. It's clear in such a case that the new - * partition overlaps that partition, whose index we - * get using its upper bound (that is, using the bound - * at off1 + 1). - */ + /* There is no gap. */ overlap = true; with = boundinfo->indexes[off1 + 1]; } diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index fb8745be04..b6f794e1c2 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -589,7 +589,7 @@ CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40); CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (30); ERROR: partition "fail_part" would overlap partition "part2" CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (50); -ERROR: partition "fail_part" would overlap partition "part3" +ERROR: partition "fail_part" would overlap partition "part2" -- now check for multi-column range partition key CREATE TABLE range_parted3 ( a int, -- 2.11.0