Oddity in COPY FROM handling of check constraints on partition tables

Started by Etsuro Fujitaalmost 8 years ago8 messages
#1Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
1 attachment(s)

Hi,

While updating the tuple-routing-for-foreign-partitions patch, I noticed
oddity in the COPY FROM handling of check constraints on partition
tables. Here is an example:

postgres=# create table pt (a int, b int) partition by list (a);
CREATE TABLE
postgres=# create table p1 partition of pt for values in (1);
CREATE TABLE
postgres=# alter table p1 add check (b > 0);
ALTER TABLE
postgres=# copy pt from '/home/pgsql/copy_data.csv' (format csv,
delimiter ',');
COPY 1
postgres=# select tableoid::regclass, * from pt;
tableoid | a | b
----------+---+----
p1 | 1 | -1
(1 row)

where the file '/home/pgsql/copy_data.csv' has a single row data

$ cat /home/pgsql/copy_data.csv
1,-1

which violates the constraint on the column b (ie, b > 0), so this
should abort. The reason for that is because CopyFrom looks at the
parent relation's constraints, not the partition's constraints, when
checking the constraint against the input row.

Attached is a patch for fixing this issue.

Best regards,
Etsuro Fujita

Attachments:

copy-from-check-constraint-fix.patchtext/x-diff; name=copy-from-check-constraint-fix.patchDownload
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 2709,2715 **** CopyFrom(CopyState cstate)
  					check_partition_constr = false;
  
  				/* Check the constraints of the tuple */
! 				if (cstate->rel->rd_att->constr || check_partition_constr)
  					ExecConstraints(resultRelInfo, slot, estate, true);
  
  				if (useHeapMultiInsert)
--- 2709,2716 ----
  					check_partition_constr = false;
  
  				/* Check the constraints of the tuple */
! 				if (resultRelInfo->ri_RelationDesc->rd_att->constr ||
! 					check_partition_constr)
  					ExecConstraints(resultRelInfo, slot, estate, true);
  
  				if (useHeapMultiInsert)
#2Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Etsuro Fujita (#1)
Re: Oddity in COPY FROM handling of check constraints on partition tables

Fujita-san,

On 2018/03/27 22:00, Etsuro Fujita wrote:

Hi,

While updating the tuple-routing-for-foreign-partitions patch, I noticed
oddity in the COPY FROM handling of check constraints on partition
tables. Here is an example:

postgres=# create table pt (a int, b int) partition by list (a);
CREATE TABLE
postgres=# create table p1 partition of pt for values in (1);
CREATE TABLE
postgres=# alter table p1 add check (b > 0);
ALTER TABLE
postgres=# copy pt from '/home/pgsql/copy_data.csv' (format csv,
delimiter ',');
COPY 1
postgres=# select tableoid::regclass, * from pt;
tableoid | a | b
----------+---+----
p1 | 1 | -1
(1 row)

where the file '/home/pgsql/copy_data.csv' has a single row data

$ cat /home/pgsql/copy_data.csv
1,-1

which violates the constraint on the column b (ie, b > 0), so this
should abort. The reason for that is because CopyFrom looks at the
parent relation's constraints, not the partition's constraints, when
checking the constraint against the input row.

Good catch, thanks!

Attached is a patch for fixing this issue.

That looks good to me. This one would need to be back-patched to v10.

Thanks,
Amit

#3Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: Amit Langote (#2)
Re: Oddity in COPY FROM handling of check constraints on partition tables

On Wed, Mar 28, 2018 at 6:58 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

which violates the constraint on the column b (ie, b > 0), so this
should abort. The reason for that is because CopyFrom looks at the
parent relation's constraints, not the partition's constraints, when
checking the constraint against the input row.

Good catch, thanks!

+1

Attached is a patch for fixing this issue.

That looks good to me. This one would need to be back-patched to v10.

Thanks. Please add to the next commitfest so that it doesn't get lost.
We can not add this to v11 open items since it isn't a v11 bug
exactly.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

#4Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Amit Langote (#2)
Re: Oddity in COPY FROM handling of check constraints on partition tables

(2018/03/28 10:28), Amit Langote wrote:

Attached is a patch for fixing this issue.

That looks good to me. This one would need to be back-patched to v10.

Thanks for the review!

Best regards,
Etsuro Fujita

#5Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Ashutosh Bapat (#3)
Re: Oddity in COPY FROM handling of check constraints on partition tables

(2018/03/28 18:51), Ashutosh Bapat wrote:

On Wed, Mar 28, 2018 at 6:58 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Attached is a patch for fixing this issue.

That looks good to me. This one would need to be back-patched to v10.

Thanks. Please add to the next commitfest so that it doesn't get lost.
We can not add this to v11 open items since it isn't a v11 bug
exactly.

OK, done.

Best regards,
Etsuro Fujita

#6Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#1)
Re: Oddity in COPY FROM handling of check constraints on partition tables

On Tue, Mar 27, 2018 at 9:00 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Attached is a patch for fixing this issue.

This no longer applies.

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

#7Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Robert Haas (#6)
Re: Oddity in COPY FROM handling of check constraints on partition tables
#8Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#7)
Re: Oddity in COPY FROM handling of check constraints on partition tables

On Wed, May 16, 2018 at 11:30 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

Attached is a patch for fixing this issue.

This no longer applies.

The patch has already been committed by you [1]. Thanks for committing!

Well, that's embarrassing.

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