8000 Fix bogus logic for reporting which hash partition conflicts. · postgrespro/postgres@dd2364c · GitHub
[go: up one dir, main page]
Skip to content

Commit dd2364c

Browse files
tglsfdctglsfdc
committed
< 10000 /div>
Fix bogus logic for reporting which hash partition conflicts.
Commit efbfb64 added logic for reporting exactly which existing partition conflicts when complaining that a new hash partition's modulus isn't compatible with the existing ones. However, it misunderstood the partitioning data structure, and would select the wrong partition in some cases, or crash outright due to fetching a bogus table OID in other cases. Per bug #17076 from Alexander Lakhin. Fix by Amit Langote; some further work on the code comments by me. Discussion: https://postgr.es/m/17076-89a16ae835d329b9@postgresql.org
1 parent dc227eb commit dd2364c

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

‎src/backend/partitioning/partbounds.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2838,12 +2838,15 @@ check_new_partition_bound(char *relname, Relation parent,
28382838

28392839
/*
28402840
* Check rule that every modulus must be a factor of the
2841-
* next larger modulus. For example, if you have a bunch
2841+
* next larger modulus. (For example, if you have a bunch
28422842
* of partitions that all have modulus 5, you can add a
28432843
* new partition with modulus 10 or a new partition with
28442844
* modulus 15, but you cannot add both a partition with
28452845
* modulus 10 and a partition with modulus 15, because 10
2846-
* is not a factor of 15.
2846+
* is not a factor of 15.) We need only check the next
2847+
* smaller and next larger existing moduli, relying on
2848+
* previous enforcement of this rule to be sure that the
2849+
* rest are in line.
28472850
*/
28482851

28492852
/*
@@ -2870,15 +2873,16 @@ check_new_partition_bound(char *relname, Relation parent,
28702873
errmsg("every hash partition modulus must be a factor of the next larger modulus"),
28712874
errdetail("The new modulus %d is not a factor of %d, the modulus of existing partition \"%s\".",
28722875
spec->modulus, next_modulus,
2873-
get_rel_name(partdesc->oids[boundinfo->indexes[0]]))));
2876+
get_rel_name(partdesc->oids[0]))));
28742877
}
28752878
else
28762879
{
28772880
int prev_modulus;
28782881

28792882
/*
2880-
* We found the largest modulus less than or equal to
2881-
* ours.
2883+
* We found the largest (modulus, remainder) pair less
2884+
* than or equal to the new one. That modulus must be
2885+
* a divisor of, or equal to, the new modulus.
28822886
*/
28832887
prev_modulus = DatumGetInt32(boundinfo->datums[offset][0]);
28842888

@@ -2889,13 +2893,19 @@ check_new_partition_bound(char *relname, Relation parent,
28892893
errdetail("The new modulus %d is not divisible by %d, the modulus of existing partition \"%s\".",
28902894
spec->modulus,
28912895
prev_modulus,
2892-
get_rel_name(partdesc->oids[boundinfo->indexes[offset]]))));
2896+
get_rel_name(partdesc->oids[offset]))));
28932897

28942898
if (offset + 1 < boundinfo->ndatums)
28952899
{
28962900
int next_modulus;
28972901

2898-
/* Look at the next higher modulus */
2902+
/*
2903+
* Look at the next higher (modulus, remainder)
2904+
* pair. That could have the same modulus and a
2905+
* larger remainder than the new pair, in which
2906+
* case we're good. If it has a larger modulus,
2907+
* the new modulus must divide that one.
2908+
*/
28992909
next_modulus = DatumGetInt32(boundinfo->datums[offset + 1][0]);
29002910

29012911
if (next_modulus % spec->modulus != 0)
@@ -2904,7 +2914,7 @@ check_new_partition_bound(char *relname, Relation parent,
29042914
errmsg("every hash partition modulus must be a factor of the next larger modulus"),
29052915
errdetail("The new modulus %d is not a factor of %d, the modulus of existing partition \"%s\".",
29062916
spec->modulus, next_modulus,
2907-
get_rel_name(partdesc->oids[boundinfo->indexes[offset + 1]]))));
2917+
get_rel_name(partdesc->oids[offset + 1]))));
29082918
}
29092919
}
29102920

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -780,14 +780,20 @@ CREATE TABLE hash_parted (
780780
CREATE TABLE hpart_1 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 0);
781781
CREATE TABLE hpart_2 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 50, REMAINDER 1);
782782
CREATE TABLE hpart_3 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 200, REMAINDER 2);
783+
CREATE TABLE hpart_4 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 3);
783784
-- modulus 25 is factor of modulus of 50 but 10 is not a factor of 25.
784785
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 25, REMAINDER 3);
785786
ERROR: every hash partition modulus must be a factor of the next larger modulus
786-
DETAIL: The new modulus 25 is not divisible by 10, the modulus of existing partition "hpart_1".
787+
DETAIL: The new modulus 25 is not divisible by 10, the modulus of existing partition "hpart_4".
787788
-- previous modulus 50 is factor of 150 but this modulus is not a factor of next modulus 200.
788789
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 150, REMAINDER 3);
789790
ERROR: every hash partition modulus must be a factor of the next larger modulus
790791
DETAIL: The new modulus 150 is not a factor of 200, the modulus of existing partition "hpart_3".
792+
-- overlapping remainders
793+
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 100, REMAINDER 3);
794+
ERROR: partition "fail_part" would overlap partition "hpart_4"
795+
LINE 1: ...BLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODU...
796+
^
791797
-- trying to specify range for the hash partitioned table
792798
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES FROM ('a', 1) TO ('z');
793799
ERROR: invalid bound specification for a hash partition
@@ -1100,7 +1106,7 @@ Number of partitions: 3 (Use \d+ to list them.)
11001106
--------+---------+-----------+----------+---------
11011107
a | integer | | |
11021108
Partition key: HASH (a)
1103-
Number of partitions: 3 (Use \d+ to list them.)
1109+
Number of partitions: 4 (Use \d+ to list them.)
11041110

11051111
-- check that we get the expected partition constraints
11061112
CREATE TABLE range_parted4 (a int, b int, c int) PARTITION BY RANGE (abs(a), abs(b), c);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,10 +631,13 @@ CREATE TABLE hash_parted (
631631
CREATE TABLE hpart_1 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 0);
632632
CREATE TABLE hpart_2 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 50, REMAINDER 1);
633633
CREATE TABLE hpart_3 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 200, REMAINDER 2);
634+
CREATE TABLE hpart_4 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 3);
634635
-- modulus 25 is factor of modulus of 50 but 10 is not a factor of 25.
635636
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 25, REMAINDER 3);
636637
-- previous modulus 50 is factor of 150 but this modulus is not a factor of next modulus 200.
637638
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 150, REMAINDER 3);
639+
-- overlapping remainders
640+
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 100, REMAINDER 3);
638641
-- trying to specify range for the hash partitioned table
639642
CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES FROM ('a', 1) TO ('z');
640643
-- trying to specify list value for the hash partitioned table

0 commit comments

Comments
 (0)
0