Re: [HACKERS] Bug in get_partition_for_tuple

2017-03-27 Thread Amit Langote 
On 2017/03/27 23:54, Robert Haas wrote:
> On Wed, Mar 22, 2017 at 4:00 AM, Amit Langote
>  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

Re: [HACKERS] Bug in get_partition_for_tuple

2017-03-27 Thread Robert Haas 
On Wed, Mar 22, 2017 at 4:00 AM, Amit Langote
 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

Re: [HACKERS] Bug in get_partition_for_tuple

2017-03-22 Thread Amit Langote 
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

Re: [HACKERS] Bug in get_partition_for_tuple

2017-03-12 Thread Amit Langote 
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
>From dbdb0f8f5205a3abd4691c00febf196b853df43f Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 10 Mar 2017 11:53:41 +0900
Subject: [PATCH] Fix a bug in get_partition_for_tuple

Handle the NULL partition key more carefully in the case of list
partitioning.
---
 src/backend/catalog/partition.c  | 10 +++---
 src/test/regress/expected/insert.out |  7 +++
 src/test/regress/sql/insert.sql  |  6 ++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index e01ef864f0..a5ba7448eb 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1726,10 +1726,14 @@ get_partition_for_tuple(PartitionDispatch *pd,
 		errmsg("range partition key of row contains null")));
 		}
 
-		if (partdesc->boundinfo->has_null && isnull[0])
-			/* Tuple maps to the null-accepting list partition */
+		/*
+		 * A null partition key is only acceptable if null-accepting list
+		 * partition exists.
+		 */
+		cur_index = -1;
+		if (isnull[0] && partdesc->boundinfo->has_null)
 			cur_index = partdesc->boundinfo->null_index;
-		else
+		else if (!isnull[0])
 		{
 			/* Else bsearch in partdesc->boundinfo */
 			bool		equal = false;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 116854e142..7fafa98212 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -365,6 +365,13 @@ DETAIL:  Failing row contains (1, 2).
 insert into mlparted1 (a, b) values (2, 3);
 ERROR:  new row for relation "mlparted11" violates partition constraint
 DETAIL:  Failing row contains (3, 2).
+-- check routing error through a list partitioned table when the key is null
+create table lparted_nonullpart (a int, b char) partition by list (b);
+create table lparted_nonullpart_a partition of lparted_nonullpart for values in ('a');
+insert into lparted_nonullpart values (1);
+ERROR:  no partition of relation "lparted_nonullpart" found for row
+DETAIL:  Partition key of the failing row contains (b) = (null).
+drop table lparted_nonullpart;
 -- check that RETURNING works correctly with tuple-routing
 alter table mlparted drop constraint check_b;
 create table mlparted12 partition of mlparted1 for values from (5) to (10);
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index c56c3c22f8..f9c00705a2 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -226,6 +226,12 @@ insert into mlparted values (1, 2);
 -- selected by tuple-routing
 insert into mlparted1 (a, b) values (2, 3);
 
+-- check routing error through a list partitioned table when the key is null
+create table lparted_nonullpart (a int, b char) partition by list (b);
+create table lparted_nonullpart_a partition of lparted_nonullpart for values in ('a');
+insert into lparted_nonullpart values (1);
+drop table lparted_nonullpart;
+
 -- check that RETURNING works correctly with tuple-routing
 alter table mlparted drop constraint check_b;
 create table mlparted12 partition of mlparted1 for values from (5) to (10);
-- 
2.11.0


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

2017-03-12 Thread Jeevan Ladhe 
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  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
>
>