Hi Rushabh,
On 2017/11/22 13:45, Rushabh Lathia wrote:
> Consider the below test:
>
> CREATE TABLE range_tab(a int, b int) PARTITION BY RANGE(a);
> CREATE TABLE range_tab_p1 PARTITION OF range_tab FOR VALUES FROM (minvalue)
> TO (10);
> CREATE TABLE range_tab_p2 PARTITION OF range_tab FOR VALUES FROM (10) TO
> (20);
> CREATE TABLE range_tab_p3 PARTITION OF range_tab FOR VALUES FROM (20) TO
> (maxvalue);
>
> INSERT INTO range_tab VALUES(NULL, 10);
>
> Above insert should fail with an error "no partition of relation found for
> row".
>
> Looking further I found that, this behaviour is changed after below commit:
>
> commit 4e5fe9ad19e14af360de7970caa8b150436c9dec
> Author: Robert Haas <[email protected]>
> Date: Wed Nov 15 10:23:28 2017 -0500
>
> Centralize executor-related partitioning code.
>
> Some code is moved from partition.c, which has grown very quickly
> lately;
> splitting the executor parts out might help to keep it from getting
> totally out of control. Other code is moved from execMain.c. All is
> moved to a new file execPartition.c. get_partition_for_tuple now has
> a new interface that more clearly separates executor concerns from
> generic concerns.
>
> Amit Langote. A slight comment tweak by me.
>
> Before above commit insert with NULL partition key in the range partition
> was throwing a proper error.
Oops, good catch.
> Attaching patch to fix as well as regression test.
Thanks for the patch. About the code, how about do it like the attached
instead?
Thanks,
Amit
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 9a44cceb22..a87dacc057 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -2536,11 +2536,10 @@ get_partition_for_tuple(Relation relation, Datum
*values, bool *isnull)
*/
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;
}
}
@@ -2560,6 +2559,8 @@ get_partition_for_tuple(Relation relation, Datum *values,
bool *isnull)
*/
part_index =
partdesc->boundinfo->indexes[bound_offset + 1];
}
+ else
+ 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);
