Thread: [HACKERS] Bug in get_partition_for_tuple

[HACKERS] Bug in get_partition_for_tuple

From
Amit Langote
Date:
Just observed a crash due to thinko in the logic that handles NULL
partition key.  Absence of null-accepting partition in this case should
have caused an error, instead the current code proceeds with comparison
resulting in crash.

create table p (a int, b char) partition by list (b);
create table p1 partition of p for values in ('a');
insert into p values (1);   -- crashes

Attached patch fixes that and adds a test.

Thanks,
Amit

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

Re: [HACKERS] Bug in get_partition_for_tuple

From
Jeevan Ladhe
Date:
Hi Amit,

I was able to reproduce the crash, and with the attached patch the crash goes
away. Also, "make check-world" passes clean.

Patch looks good to me. However, In following comment in your test:

-- check routing error through a list partitioned table when they key is null

I think you want to say:

-- check routing error through a list partitioned table when the key is null


Thanks,
Jeevan Ladhe


On Fri, Mar 10, 2017 at 8:26 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Just observed a crash due to thinko in the logic that handles NULL
partition key.  Absence of null-accepting partition in this case should
have caused an error, instead the current code proceeds with comparison
resulting in crash.

create table p (a int, b char) partition by list (b);
create table p1 partition of p for values in ('a');
insert into p values (1);   -- crashes

Attached patch fixes that and adds a test.

Thanks,
Amit


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Bug in get_partition_for_tuple

From
Amit Langote
Date:
Hi Jeevan,

On 2017/03/13 14:31, Jeevan Ladhe wrote:
> Hi Amit,
> 
> I was able to reproduce the crash, and with the attached patch the crash
> goes
> away. Also, "make check-world" passes clean.
> 
> Patch looks good to me.

Thanks for the review.

> However, In following comment in your test:
> 
> -- check routing error through a list partitioned table when they key is
> null
> 
> I think you want to say:
> 
> -- check routing error through a list partitioned table when the key is null

You're right, fixed that in the attached updated patch.

Thanks,
Amit

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

Re: [HACKERS] Bug in get_partition_for_tuple

From
Amit Langote
Date:
On 2017/03/13 14:41, Amit Langote wrote:
> On 2017/03/13 14:31, Jeevan Ladhe wrote:
>> However, In following comment in your test:
>>
>> -- check routing error through a list partitioned table when they key is
>> null
>>
>> I think you want to say:
>>
>> -- check routing error through a list partitioned table when the key is null
> 
> You're right, fixed that in the attached updated patch.

Added this to the partitioning open items list, to avoid being forgotten
about.

https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items#Partitioning

Thanks,
Amit





Re: Bug in get_partition_for_tuple

From
Robert Haas
Date:
On Wed, Mar 22, 2017 at 4:00 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> You're right, fixed that in the attached updated patch.
>
> Added this to the partitioning open items list, to avoid being forgotten
> about.
>
> https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items#Partitioning

Thanks for adding it there.  I'm making a sweep through that list now,
or at least the parts of it that obvious pertain to my commits.  This
patch looks good, so committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Bug in get_partition_for_tuple

From
Amit Langote
Date:
On 2017/03/27 23:54, Robert Haas wrote:
> On Wed, Mar 22, 2017 at 4:00 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> You're right, fixed that in the attached updated patch.
>>
>> Added this to the partitioning open items list, to avoid being forgotten
>> about.
>>
>> https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items#Partitioning
> 
> Thanks for adding it there.  I'm making a sweep through that list now,
> or at least the parts of it that obvious pertain to my commits.  This
> patch looks good, so committed.

Thanks.

Regards,
Amit