On 2017/11/22 17:42, amul sul wrote: > On Wed, Nov 22, 2017 at 11:38 AM, Amit Langote wrote: >> On 2017/11/22 13:45, Rushabh Lathia wrote: >>> Attaching patch to fix as well as regression test. >> >> Thanks for the patch. About the code, how about do it like the attached >> instead? >> > > Looks good to me, even we can skip the following change in v2 patch: > > 19 @@ -2560,6 +2559,8 @@ get_partition_for_tuple(Relation relation, > Datum *values, bool *isnull) > 20 */ > 21 part_index = > partdesc->boundinfo->indexes[bound_offset + 1]; > 22 } > 23 + else > 24 + part_index = partdesc->boundinfo->default_index; > 25 } > 26 break; > 27 > > default_index will get assign by following code in get_partition_for_tuple() : > > /* > * part_index < 0 means we failed to find a partition of this parent. > * Use the default partition, if there is one. > */ > if (part_index < 0) > part_index = partdesc->boundinfo->default_index;
Good point. Updated patch attached. Thanks, Amit
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 9a44cceb22..3b5df5164a 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -2531,16 +2531,14 @@ get_partition_for_tuple(Relation relation, Datum *values, bool *isnull) /* * No range includes NULL, so this will be accepted by the - * default partition if there is one, and otherwise - * rejected. + * default partition if there is one, otherwise rejected. */ for (i = 0; i < key->partnatts; i++) { - if (isnull[i] && - partition_bound_has_default(partdesc->boundinfo)) + if (isnull[i]) { range_partkey_has_null = true; - part_index = partdesc->boundinfo->default_index; + break; } } diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 9d84ba4658..a0e3746e70 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -642,6 +642,10 @@ create table mcrparted2 partition of mcrparted for values from (10, 6, minvalue) create table mcrparted3 partition of mcrparted for values from (11, 1, 1) to (20, 10, 10); create table mcrparted4 partition of mcrparted for values from (21, minvalue, minvalue) to (30, 20, maxvalue); create table mcrparted5 partition of mcrparted for values from (30, 21, 20) to (maxvalue, maxvalue, maxvalue); +-- null not allowed in range partition +insert into mcrparted values (null, null, null); +ERROR: no partition of relation "mcrparted" found for row +DETAIL: Partition key of the failing row contains (a, abs(b), c) = (null, null, null). -- routed to mcrparted0 insert into mcrparted values (0, 1, 1); insert into mcrparted0 values (0, 1, 1); diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 791817ba50..1c4491a6a2 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -417,6 +417,9 @@ create table mcrparted3 partition of mcrparted for values from (11, 1, 1) to (20 create table mcrparted4 partition of mcrparted for values from (21, minvalue, minvalue) to (30, 20, maxvalue); create table mcrparted5 partition of mcrparted for values from (30, 21, 20) to (maxvalue, maxvalue, maxvalue); +-- null not allowed in range partition +insert into mcrparted values (null, null, null); + -- routed to mcrparted0 insert into mcrparted values (0, 1, 1); insert into mcrparted0 values (0, 1, 1);