Re: With commit 4e5fe9ad19, range partition missing handling for the NULL partition key

On Wed, Nov 22, 2017 at 11:38 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> 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 <rhaas(at)postgresql(dot)org>
>> 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?
>

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;

Regards,
Amul