Bug in get_partition_for_tuple

Started by Amit Langoteabout 9 years ago6 messageshackers
Jump to latest
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp

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

Attachments:

0001-Fix-a-bug-in-get_partition_for_tuple.patchtext/x-diff; name=0001-Fix-a-bug-in-get_partition_for_tuple.patchDownload+20-4
#2Jeevan Ladhe
jeevan.ladhe@enterprisedb.com
In reply to: Amit Langote (#1)
Re: Bug in get_partition_for_tuple

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

Show quoted text

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

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Jeevan Ladhe (#2)
Re: Bug in get_partition_for_tuple

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

Attachments:

0001-Fix-a-bug-in-get_partition_for_tuple.patchtext/x-diff; name=0001-Fix-a-bug-in-get_partition_for_tuple.patchDownload+20-4
#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#3)
Re: Bug in get_partition_for_tuple

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

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Amit Langote (#4)
Re: Bug in get_partition_for_tuple

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

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

#6Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Robert Haas (#5)
Re: Bug in get_partition_for_tuple

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

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