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);

Reply via email to