Re: [HACKERS] foreign table creation and NOT VALID check constraints

2017-08-05 Thread Robert Haas 
On Thu, Aug 3, 2017 at 9:29 PM, Amit Langote
 wrote:
> Thanks for committing the code changes.
>
> About the documentation changes, it seems that the only places where any
> description of NOT VALID appears is ALTER TABLE, ALTER FOREIGN TABLE, and
> ALTER DOMAIN references pages.  Even if the CREATE (FOREIGN) TABLE syntax
> allows it, NOT VALID does not appear in the syntax synopsis, so it seems
> kind of a hidden feature.  Maybe, we should fix that first (if at all).

Yeah.  If somebody wants to do a general survey of how NOT VALID is
documented and improve that, cool.

-- 
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] foreign table creation and NOT VALID check constraints

2017-08-03 Thread Amit Langote 
On 2017/08/04 2:13, Robert Haas wrote:
> On Thu, Aug 3, 2017 at 12:35 AM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> On Wed, Aug 2, 2017 at 9:41 PM, Amit Langote
>>>  wrote:
 Attached is a patch.  I think this could be considered a bug-fix,
 backpatchable to 9.6 which introduced this behavior change [1].
>>
>>> I could go either way on that.  It's not inconceivable somebody could
>>> be unhappy about seeing this behavior change in a minor release.
>>
>> FWIW, I vote with the camp that this is a clear bug and needs to be
>> fixed.  9.6 broke a behavior that could be relied on before that.
>> We do not normally hesitate to fix regressions in minor releases.
>>
>> (That's not a vote for the patch as submitted; I haven't reviewed it.
>> But we need to fix this.)
> 
> OK.  I'm going to commit and back-patch the substantive fix with a
> comment change, but I'm not going to include Amit's documentation
> changes for now because I'm not sure they are going to be sufficiently
> clear.  There's not a lot of context for them where he put them.

Thanks for committing the code changes.

About the documentation changes, it seems that the only places where any
description of NOT VALID appears is ALTER TABLE, ALTER FOREIGN TABLE, and
ALTER DOMAIN references pages.  Even if the CREATE (FOREIGN) TABLE syntax
allows it, NOT VALID does not appear in the syntax synopsis, so it seems
kind of a hidden feature.  Maybe, we should fix that first (if at all).

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] foreign table creation and NOT VALID check constraints

2017-08-03 Thread Robert Haas 
On Thu, Aug 3, 2017 at 12:35 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Wed, Aug 2, 2017 at 9:41 PM, Amit Langote
>>  wrote:
>>> Attached is a patch.  I think this could be considered a bug-fix,
>>> backpatchable to 9.6 which introduced this behavior change [1].
>
>> I could go either way on that.  It's not inconceivable somebody could
>> be unhappy about seeing this behavior change in a minor release.
>
> FWIW, I vote with the camp that this is a clear bug and needs to be
> fixed.  9.6 broke a behavior that could be relied on before that.
> We do not normally hesitate to fix regressions in minor releases.
>
> (That's not a vote for the patch as submitted; I haven't reviewed it.
> But we need to fix this.)

OK.  I'm going to commit and back-patch the substantive fix with a
comment change, but I'm not going to include Amit's documentation
changes for now because I'm not sure they are going to be sufficiently
clear.  There's not a lot of context for them where he put them.

-- 
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] foreign table creation and NOT VALID check constraints

2017-08-02 Thread Tom Lane 
Robert Haas  writes:
> On Wed, Aug 2, 2017 at 9:41 PM, Amit Langote
>  wrote:
>> Attached is a patch.  I think this could be considered a bug-fix,
>> backpatchable to 9.6 which introduced this behavior change [1].

> I could go either way on that.  It's not inconceivable somebody could
> be unhappy about seeing this behavior change in a minor release.

FWIW, I vote with the camp that this is a clear bug and needs to be
fixed.  9.6 broke a behavior that could be relied on before that.
We do not normally hesitate to fix regressions in minor releases.

(That's not a vote for the patch as submitted; I haven't reviewed it.
But we need to fix this.)

regards, tom lane


-- 
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] foreign table creation and NOT VALID check constraints

2017-08-02 Thread Robert Haas 
On Wed, Aug 2, 2017 at 9:41 PM, Amit Langote
 wrote:
> On 2017/08/02 20:40, Robert Haas wrote:
>> On Wed, Aug 2, 2017 at 3:46 AM, Ashutosh Bapat
>>  wrote:
>>> If the user has specified "not valid" for a constraint on the foreign
>>> table, there is high chance that s/he is aware of the fact that the
>>> remote table that the foreign table points to has some rows which will
>>> violet the constraint. So, +1.
>>
>> +1 from me, too.
>
> Alright, thanks.
>
> Attached is a patch.  I think this could be considered a bug-fix,
> backpatchable to 9.6 which introduced this behavior change [1].

I could go either way on that.  It's not inconceivable somebody could
be unhappy about seeing this behavior change in a minor release.

-- 
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] foreign table creation and NOT VALID check constraints

2017-08-02 Thread Amit Langote 
On 2017/08/02 20:40, Robert Haas wrote:
> On Wed, Aug 2, 2017 at 3:46 AM, Ashutosh Bapat
>  wrote:
>> If the user has specified "not valid" for a constraint on the foreign
>> table, there is high chance that s/he is aware of the fact that the
>> remote table that the foreign table points to has some rows which will
>> violet the constraint. So, +1.
> 
> +1 from me, too.

Alright, thanks.

Attached is a patch.  I think this could be considered a bug-fix,
backpatchable to 9.6 which introduced this behavior change [1].

Thoughts?

Thanks,
Amit

[1]
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f27a6b15e656
From a0967f1a71a65e7802f504002a6dc3dfd1f4a6bb Mon Sep 17 00:00:00 2001
From: amit 
Date: Thu, 3 Aug 2017 10:31:21 +0900
Subject: [PATCH] Honor NOT VALID specification in CREATE FOREIGN TABLE

It should be possible for a user to mark the constraints on foreign
tables as NOT VALID even when creating the table, because the remote
data they point to may contain constraint-violating rows, which the
database has no way to detect and show an error message for.
---
 doc/src/sgml/ref/create_foreign_table.sgml | 7 +++
 doc/src/sgml/ref/create_table.sgml | 7 +++
 src/backend/parser/parse_utilcmd.c | 3 ++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/create_foreign_table.sgml 
b/doc/src/sgml/ref/create_foreign_table.sgml
index 065c982082..72bf37b8b9 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -222,6 +222,13 @@ CHECK ( expression ) [ NO INHERIT ]
   A constraint marked with NO INHERIT will not propagate to
   child tables.
  
+
+ 
+  It is possible to mark the constraint as NOT VALID if it is
+  specified as a table constraint.  If marked as such, the database will
+  not assume that the constraint holds for all the rows in the table until
+  it is validated by using the VALIDATE CONSTRAINT command.
+  See .
 

 
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index e9c2c49533..72de64a03e 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -652,6 +652,13 @@ FROM ( { numeric_literal | PostgreSQL versions before 9.5 did not honor any
   particular firing order for CHECK constraints.)
  
+
+ 
+  Note that even if the constraint is marked as NOT VALID,
+  it is considered validated as the table that is just created cannot
+  contain any data.  In other words, specifying NOT VALID in
+  this case has no effect.
+ 
 

 
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index 9f37f1b920..e54322b460 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -165,6 +165,7 @@ transformCreateStmt(CreateStmt *stmt, const char 
*queryString)
Oid existing_relid;
ParseCallbackState pcbstate;
boollike_found = false;
+   boolis_foreign_table = IsA(stmt, CreateForeignTableStmt);
 
/*
 * We must not scribble on the passed-in CreateStmt, so copy it.  (This 
is
@@ -330,7 +331,7 @@ transformCreateStmt(CreateStmt *stmt, const char 
*queryString)
/*
 * Postprocess check constraints.
 */
-   transformCheckConstraints(, true);
+   transformCheckConstraints(, !is_foreign_table ? true : false);
 
/*
 * Output results.
-- 
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] foreign table creation and NOT VALID check constraints

2017-08-02 Thread Robert Haas 
On Wed, Aug 2, 2017 at 3:46 AM, Ashutosh Bapat
 wrote:
> If the user has specified "not valid" for a constraint on the foreign
> table, there is high chance that s/he is aware of the fact that the
> remote table that the foreign table points to has some rows which will
> violet the constraint. So, +1.

+1 from me, too.

-- 
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] foreign table creation and NOT VALID check constraints

2017-08-02 Thread Ashutosh Bapat 
On Tue, Aug 1, 2017 at 2:41 PM, Amit Langote
 wrote:
> On 2017/08/01 17:54, Simon Riggs wrote:
>> On 1 August 2017 at 08:37, Amit Langote  
>> wrote:
>>> On 2017/08/01 15:22, Simon Riggs wrote:
 On 1 August 2017 at 07:16, Amit Langote  
 wrote:
> In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
> declared NOT VALID valid if created with table."  In retrospect,
> constraints on foreign tables should have been excluded from consideration
> in that commit, because the thinking behind the aforementioned commit
> (that the constraint is trivially validated because the newly created
> table contains no data) does not equally apply to the foreign tables case.
>
> Should we do something about that?

 In what way does it not apply? Do you have a failure case?
>>>
>>> Sorry for not mentioning the details.
>>>
>>> I was thinking that a foreign table starts containing the data of the
>>> remote object it points to the moment it's created (unlike local tables
>>> which contain no data to begin with).  If a user is not sure whether a
>>> particular constraint being created in the same command holds for the
>>> remote data, they may mark it as NOT VALID and hope that the system treats
>>> the constraint as such until such a time that they mark it valid by
>>> running ALTER TABLE VALIDATE CONSTRAINT.  Since the planner is the only
>>> consumer of pg_constraint.convalidated, that means the user expects the
>>> planner to ignore such a constraint.  Since f27a6b15e656, users are no
>>> longer able to expect so.
>>
>> For Foreign Tables, it sounds like an issue. Don't think it exists for
>> normal tables.
>
> Yes.  I was saying in my first email that we should not disregard user's
> request to mark a constraint NOT VALID if the table in question is a
> *foreign table*.
>
> So, it's OK that commit f27a6b15e656 changed things to ignore NOT VALID if
> it's in the following command:
>
> create table foo (a int, constraint check_a check (a > 0) not valid);
>
> But, not OK in the following command:
>
> create foreign table ffoo (
>   a int,
>   constraint check_a check (a > 0) not valid
> ) server loopback options (table_name 'foo');
>

If the user has specified "not valid" for a constraint on the foreign
table, there is high chance that s/he is aware of the fact that the
remote table that the foreign table points to has some rows which will
violet the constraint. So, +1.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] foreign table creation and NOT VALID check constraints

2017-08-01 Thread Amit Langote 
On 2017/08/01 17:54, Simon Riggs wrote:
> On 1 August 2017 at 08:37, Amit Langote  wrote:
>> On 2017/08/01 15:22, Simon Riggs wrote:
>>> On 1 August 2017 at 07:16, Amit Langote  
>>> wrote:
 In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
 declared NOT VALID valid if created with table."  In retrospect,
 constraints on foreign tables should have been excluded from consideration
 in that commit, because the thinking behind the aforementioned commit
 (that the constraint is trivially validated because the newly created
 table contains no data) does not equally apply to the foreign tables case.

 Should we do something about that?
>>>
>>> In what way does it not apply? Do you have a failure case?
>>
>> Sorry for not mentioning the details.
>>
>> I was thinking that a foreign table starts containing the data of the
>> remote object it points to the moment it's created (unlike local tables
>> which contain no data to begin with).  If a user is not sure whether a
>> particular constraint being created in the same command holds for the
>> remote data, they may mark it as NOT VALID and hope that the system treats
>> the constraint as such until such a time that they mark it valid by
>> running ALTER TABLE VALIDATE CONSTRAINT.  Since the planner is the only
>> consumer of pg_constraint.convalidated, that means the user expects the
>> planner to ignore such a constraint.  Since f27a6b15e656, users are no
>> longer able to expect so.
> 
> For Foreign Tables, it sounds like an issue. Don't think it exists for
> normal tables.

Yes.  I was saying in my first email that we should not disregard user's
request to mark a constraint NOT VALID if the table in question is a
*foreign table*.

So, it's OK that commit f27a6b15e656 changed things to ignore NOT VALID if
it's in the following command:

create table foo (a int, constraint check_a check (a > 0) not valid);

But, not OK in the following command:

create foreign table ffoo (
  a int,
  constraint check_a check (a > 0) not valid
) server loopback options (table_name 'foo');

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] foreign table creation and NOT VALID check constraints

2017-08-01 Thread Simon Riggs 
On 1 August 2017 at 08:37, Amit Langote  wrote:
> On 2017/08/01 15:22, Simon Riggs wrote:
>> On 1 August 2017 at 07:16, Amit Langote  
>> wrote:
>>> In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
>>> declared NOT VALID valid if created with table."  In retrospect,
>>> constraints on foreign tables should have been excluded from consideration
>>> in that commit, because the thinking behind the aforementioned commit
>>> (that the constraint is trivially validated because the newly created
>>> table contains no data) does not equally apply to the foreign tables case.
>>>
>>> Should we do something about that?
>>
>> In what way does it not apply? Do you have a failure case?
>
> Sorry for not mentioning the details.
>
> I was thinking that a foreign table starts containing the data of the
> remote object it points to the moment it's created (unlike local tables
> which contain no data to begin with).  If a user is not sure whether a
> particular constraint being created in the same command holds for the
> remote data, they may mark it as NOT VALID and hope that the system treats
> the constraint as such until such a time that they mark it valid by
> running ALTER TABLE VALIDATE CONSTRAINT.  Since the planner is the only
> consumer of pg_constraint.convalidated, that means the user expects the
> planner to ignore such a constraint.  Since f27a6b15e656, users are no
> longer able to expect so.

For Foreign Tables, it sounds like an issue. Don't think it exists for
normal tables.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] foreign table creation and NOT VALID check constraints

2017-08-01 Thread Amit Langote 
On 2017/08/01 15:22, Simon Riggs wrote:
> On 1 August 2017 at 07:16, Amit Langote  wrote:
>> In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
>> declared NOT VALID valid if created with table."  In retrospect,
>> constraints on foreign tables should have been excluded from consideration
>> in that commit, because the thinking behind the aforementioned commit
>> (that the constraint is trivially validated because the newly created
>> table contains no data) does not equally apply to the foreign tables case.
>>
>> Should we do something about that?
> 
> In what way does it not apply? Do you have a failure case?

Sorry for not mentioning the details.

I was thinking that a foreign table starts containing the data of the
remote object it points to the moment it's created (unlike local tables
which contain no data to begin with).  If a user is not sure whether a
particular constraint being created in the same command holds for the
remote data, they may mark it as NOT VALID and hope that the system treats
the constraint as such until such a time that they mark it valid by
running ALTER TABLE VALIDATE CONSTRAINT.  Since the planner is the only
consumer of pg_constraint.convalidated, that means the user expects the
planner to ignore such a constraint.  Since f27a6b15e656, users are no
longer able to expect so.

For example:

create extension postgres_fdw;

create server loopback
  foreign data wrapper postgres_fdw
  options (dbname 'postgres');

create table foo (a) as select 1;

create foreign table ffoo (
  a int,
  constraint check_a check (a = 0) not valid
) server loopback options (table_name 'foo');

set constraint_exclusion to on;

-- constraint check_a will exclude the table
select * from ffoo where a = 1;
 a
---
(0 rows)

explain select * from ffoo where a = 1;
QUERY PLAN
--
 Result  (cost=0.00..0.00 rows=0 width=4)
   One-Time Filter: false
(2 rows)

The above "issue" doesn't occur if the constraint is created
after-the-fact, whereby it's possible to specify NOT VALID and have the
system actually store it in the catalog as such.

alter foreign table ffoo drop constraint check_a;
alter foreign table ffoo add constraint check_a check (a = 0) not valid;

select * from ffoo where a = 1;
 a
---
 1
(1 row)

explain select * from ffoo where a = 1;
 QUERY PLAN
-
 Foreign Scan on ffoo  (cost=100.00..146.86 rows=15 width=4)


Maybe, this is not such a big matter though, because the core system
doesn't actually enforce any constraints of the foreign tables locally
anyway (which is a documented fact), but my concern is the possibility of
some considering this a POLA violation.  Lack of complaints so far perhaps
means nobody did.

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] foreign table creation and NOT VALID check constraints

2017-08-01 Thread Simon Riggs 
On 1 August 2017 at 07:16, Amit Langote  wrote:
> In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
> declared NOT VALID valid if created with table."  In retrospect,
> constraints on foreign tables should have been excluded from consideration
> in that commit, because the thinking behind the aforementioned commit
> (that the constraint is trivially validated because the newly created
> table contains no data) does not equally apply to the foreign tables case.
>
> Should we do something about that?

In what way does it not apply? Do you have a failure case?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] foreign table creation and NOT VALID check constraints

2017-08-01 Thread Amit Langote 
In f27a6b15e656 (9.6 & later), we decided to "Mark CHECK constraints
declared NOT VALID valid if created with table."  In retrospect,
constraints on foreign tables should have been excluded from consideration
in that commit, because the thinking behind the aforementioned commit
(that the constraint is trivially validated because the newly created
table contains no data) does not equally apply to the foreign tables case.

Should we do something about that?

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