From 970c05057593c2f5919a69b43fd917c4fa86f51c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 30 Oct 2020 17:00:59 -0400 Subject: [PATCH] Fix assertion failure in check_new_partition_bound(). Commit 6b2c4e59d was overly confident about not being able to see a negative cmpval result from partition_range_bsearch(). Adjust the code to cope with that. Report and patch by Amul Sul; some additional cosmetic changes by me Discussion: https://postgr.es/m/CAAJ_b97WCO=EyVA7fKzc86kKfojHXLU04_zs7-7+yVzm=-1QkQ@mail.gmail.com --- src/backend/partitioning/partbounds.c | 40 +++++++++++----------- src/test/regress/expected/create_table.out | 4 +++ src/test/regress/sql/create_table.sql | 1 + 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 66c42b5898..a4f97c10fe 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -2703,10 +2703,10 @@ add_merged_range_bounds(int partnatts, FmgrInfo *partsupfuncs, prev_ub.lower = false; /* - * We pass to partition_rbound_cmp() lower1 as false to prevent it - * from considering the last upper bound to be smaller than the lower - * bound of the merged partition when the values of the two range - * bounds compare equal. + * We pass lower1 = false to partition_rbound_cmp() to prevent it from + * considering the last upper bound to be smaller than the lower bound + * of the merged partition when the values of the two range bounds + * compare equal. */ cmpval = partition_rbound_cmp(partnatts, partsupfuncs, partcollations, merged_lb->datums, merged_lb->kind, @@ -2978,16 +2978,19 @@ check_new_partition_bound(char *relname, Relation parent, /* * First check if the resulting range would be empty with - * specified lower and upper bounds + * specified lower and upper bounds. partition_rbound_cmp + * cannot return zero here, since the lower-bound flags are + * different. */ cmpval = partition_rbound_cmp(key->partnatts, key->partsupfunc, key->partcollation, lower->datums, lower->kind, true, upper); - if (cmpval >= 0) + Assert(cmpval != 0); + if (cmpval > 0) { - /* Fetch the problematic key from the lower datums list. */ + /* Point to problematic key in the lower datums list. */ PartitionRangeDatum *datum = list_nth(spec->lowerdatums, cmpval - 1); @@ -3057,11 +3060,11 @@ check_new_partition_bound(char *relname, Relation parent, if (cmpval < 0) { /* - * Fetch the problematic key from the upper + * Point to problematic key in the upper * datums list. */ PartitionRangeDatum *datum = - list_nth(spec->upperdatums, -cmpval - 1); + list_nth(spec->upperdatums, Abs(cmpval) - 1); /* * The new partition overlaps with the @@ -3083,15 +3086,11 @@ check_new_partition_bound(char *relname, Relation parent, PartitionRangeDatum *datum; /* - * Fetch the problematic key from the lower datums - * list. Given the way partition_range_bsearch() - * works, the new lower bound is certainly >= the - * bound at offset. If the bound matches exactly, we - * flag the 1st key. + * Point to problematic key in the lower datums list; + * if we have equality, point to the first one. */ - Assert(cmpval >= 0); datum = cmpval == 0 ? linitial(spec->lowerdatums) : - list_nth(spec->lowerdatums, cmpval - 1); + list_nth(spec->lowerdatums, Abs(cmpval) - 1); overlap = true; overlap_location = datum->location; with = boundinfo->indexes[offset + 1]; @@ -3393,13 +3392,14 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc, else if (kind1[i] > kind2[i]) return colnum; else if (kind1[i] != PARTITION_RANGE_DATUM_VALUE) - + { /* * The column bounds are both MINVALUE or both MAXVALUE. No later * columns should be considered, but we still need to compare * whether they are upper or lower bounds. */ break; + } cmpval = DatumGetInt32(FunctionCall2Coll(&partsupfunc[i], partcollation[i], @@ -3692,9 +3692,9 @@ qsort_partition_rbound_cmp(const void *a, const void *b, void *arg) PartitionRangeBound *b2 = (*(PartitionRangeBound *const *) b); PartitionKey key = (PartitionKey) arg; - return partition_rbound_cmp(key->partnatts, key->partsupfunc, - key->partcollation, b1->datums, b1->kind, - b1->lower, b2); + return compare_range_bounds(key->partnatts, key->partsupfunc, + key->partcollation, + b1, b2); } /* diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 1fc266dd65..ed8c01b8de 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -856,6 +856,10 @@ ERROR: partition "fail_part" would overlap partition "part0" LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) ... ^ CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10); +CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1); +ERROR: partition "fail_part" would overlap partition "part0" +LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1)... + ^ CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (maxvalue); ERROR: partition "fail_part" would overlap partition "part1" LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (max... diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index cee822aa8b..d257679ba6 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -687,6 +687,7 @@ CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (1); CREATE TABLE part0 PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (1); CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (2); CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10); +CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1); CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (maxvalue); CREATE TABLE part2 PARTITION OF range_parted2 FOR VALUES FROM (20) TO (30); CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40);