Skip to content

Commit 8a8afe2

Browse files
robertmhaasrobertmhaas
committed
Fix some problems in check_new_partition_bound().
Account for the fact that the highest bound less than or equal to the upper bound might be either the lower or the upper bound of the overlapping partition, depending on whether the proposed partition completely contains the existing partition or merely overlaps it. Also, we need not continue searching for even greater bound in partition_bound_bsearch() once we find the first bound that is *equal* to the probe, because we don't have duplicate datums. That spends cycles needlessly. Amit Langote, per a report from Amul Sul. Cosmetic changes by me. Discussion: http://postgr.es/m/CAAJ_b94XgbqVoXMyxxs63CaqWoMS1o2gpHiU0F7yGnJBnvDc_A%40mail.gmail.com
1 parent 05bd889 commit 8a8afe2

File tree

3 files changed

+62
-16
lines changed

3 files changed

+62
-16
lines changed

‎src/backend/catalog/partition.c

+49-15
Original file line numberDiff line numberDiff line change
@@ -741,37 +741,68 @@ check_new_partition_bound(char *relname, Relation parent, Node *bound)
741741
boundinfo->strategy == PARTITION_STRATEGY_RANGE);
742742

743743
/*
744-
* Find the greatest index of a range bound that is less
745-
* than or equal with the new lower bound.
744+
* Firstly, find the greatest range bound that is less
745+
* than or equal to the new lower bound.
746746
*/
747747
off1 = partition_bound_bsearch(key, boundinfo, lower, true,
748748
&equal);
749749

750750
/*
751-
* If equal has been set to true, that means the new lower
752-
* bound is found to be equal with the bound at off1,
753-
* which clearly means an overlap with the partition at
754-
* index off1+1).
755-
*
756-
* Otherwise, check if there is a "gap" that could be
757-
* occupied by the new partition. In case of a gap, the
758-
* new upper bound should not cross past the upper
759-
* boundary of the gap, that is, off2 == off1 should be
760-
* true.
751+
* off1 == -1 means that all existing bounds are greater
752+
* than the new lower bound. In that case and the case
753+
* where no partition is defined between the bounds at
754+
* off1 and off1 + 1, we have a "gap" in the range that
755+
* could be occupied by the new partition. We confirm if
756+
* so by checking whether the new upper bound is confined
757+
* within the gap.
761758
*/
762759
if (!equal && boundinfo->indexes[off1 + 1] < 0)
763760
{
764761
off2 = partition_bound_bsearch(key, boundinfo, upper,
765762
true, &equal);
766763

764+
/*
765+
* If the new upper bound is returned to be equal to
766+
* the bound at off2, the latter must be the upper
767+
* bound of some partition with which the new
768+
* partition clearly overlaps.
769+
*
770+
* Also, if bound at off2 is not same as the one
771+
* returned for the new lower bound (IOW, off1 !=
772+
* off2), then the new partition overlaps at least one
773+
* partition.
774+
*/
767775
if (equal || off1 != off2)
768776
{
769777
overlap = true;
770-
with = boundinfo->indexes[off2 + 1];
778+
779+
/*
780+
* The bound at off2 could be the lower bound of
781+
* the partition with which the new partition
782+
* overlaps. In that case, use the upper bound
783+
* (that is, the bound at off2 + 1) to get the
784+
* index of that partition.
785+
*/
786+
if (boundinfo->indexes[off2] < 0)
787+
with = boundinfo->indexes[off2 + 1];
788+
else
789+
with = boundinfo->indexes[off2];
771790
}
772791
}
773792
else
774793
{
794+
/*
795+
* Equal has been set to true and there is no "gap"
796+
* between the bound at off1 and that at off1 + 1, so
797+
* the new partition will overlap some partition. In
798+
* the former case, the new lower bound is found to be
799+
* equal to the bound at off1, which could only ever
800+
* be true if the latter is the lower bound of some
801+
* partition. It's clear in such a case that the new
802+
* partition overlaps that partition, whose index we
803+
* get using its upper bound (that is, using the bound
804+
* at off1 + 1).
805+
*/
775806
overlap = true;
776807
with = boundinfo->indexes[off1 + 1];
777808
}
@@ -1957,8 +1988,8 @@ partition_bound_cmp(PartitionKey key, PartitionBoundInfo boundinfo,
19571988
}
19581989

19591990
/*
1960-
* Binary search on a collection of partition bounds. Returns greatest index
1961-
* of bound in array boundinfo->datums which is less or equal with *probe.
1991+
* Binary search on a collection of partition bounds. Returns greatest
1992+
* bound in array boundinfo->datums which is less than or equal to *probe
19621993
* If all bounds in the array are greater than *probe, -1 is returned.
19631994
*
19641995
* *probe could either be a partition bound or a Datum array representing
@@ -1990,6 +2021,9 @@ partition_bound_bsearch(PartitionKey key, PartitionBoundInfo boundinfo,
19902021
{
19912022
lo = mid;
19922023
*is_equal = (cmpval == 0);
2024+
2025+
if (*is_equal)
2026+
break;
19932027
}
19942028
else
19952029
hi = mid - 1;

‎src/test/regress/expected/create_table.out

+9-1
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,12 @@ ERROR: partition "fail_part" would overlap partition "part0"
551551
CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10);
552552
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (unbounded);
553553
ERROR: partition "fail_part" would overlap partition "part1"
554+
CREATE TABLE part2 PARTITION OF range_parted2 FOR VALUES FROM (20) TO (30);
555+
CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40);
556+
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (30);
557+
ERROR: partition "fail_part" would overlap partition "part2"
558+
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (50);
559+
ERROR: partition "fail_part" would overlap partition "part3"
554560
-- now check for multi-column range partition key
555561
CREATE TABLE range_parted3 (
556562
a int,
@@ -655,13 +661,15 @@ table part_c depends on table parted
655661
table part_c_1_10 depends on table part_c
656662
HINT: Use DROP ... CASCADE to drop the dependent objects too.
657663
DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3 CASCADE;
658-
NOTICE: drop cascades to 14 other objects
664+
NOTICE: drop cascades to 16 other objects
659665
DETAIL: drop cascades to table part00
660666
drop cascades to table part10
661667
drop cascades to table part11
662668
drop cascades to table part12
663669
drop cascades to table part0
664670
drop cascades to table part1
671+
drop cascades to table part2
672+
drop cascades to table part3
665673
drop cascades to table part_null_z
666674
drop cascades to table part_ab
667675
drop cascades to table part_1

‎src/test/regress/sql/create_table.sql

+4
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,10 @@ CREATE TABLE part0 PARTITION OF range_parted2 FOR VALUES FROM (unbounded) TO (1)
519519
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (unbounded) TO (2);
520520
CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10);
521521
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (unbounded);
522+
CREATE TABLE part2 PARTITION OF range_parted2 FOR VALUES FROM (20) TO (30);
523+
CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40);
524+
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (30);
525+
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (50);
522526

523527
-- now check for multi-column range partition key
524528
CREATE TABLE range_parted3 (

0 commit comments

Comments
 (0)