Re: [HACKERS] Missing importing option of postgres_fdw

2015-05-25 Thread Etsuro Fujita
On 2015/05/22 23:50, Tom Lane wrote:
 Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
 I agree with you on that point.  And I think one option for that is that
 IMPORT FOREIGN SCHEMA only imports CHECK constraints of remote tables
 from a remote server that have convalidated = true.  (If doing so, we
 wouldn't need to allow IMPORT FOREIGN SCHEMA to return ALTER FOREIGN
 TABLE statements.)  But I'm not sure that that is a good idea.  ISTM it
 would be better for IMPORT FOREIGN SCHEMA just to imports all CHECK
 constraints of remote tables, reflecting convalidated, and that we leave
 it to the user to do VALIDATE CONSTRAINT for locally defined constraints
 that have convalidated = false when matching constraints are validated
 on the remote server.
 
 It would only be safe to automatically import CHECK constraints if they
 contain no expressions that could evaluate differently on remote and local
 --- IOW, only expressions that would be safe to transmit as remote query
 conditions.  I don't really think we should try to do that at all.
 
 For starters, while it might seem like we could use is_foreign_expr()
 on the conditions, there's no guarantee that the local server accurately
 knows what functions on the foreign server are really safe.  The is safe
 to transmit condition isn't symmetric.
 
 For that matter, there's no guarantee that we could even parse the
 remote's constraint condition text without failing --- it might use SQL
 features we don't have.  (We have that risk already for DEFAULT
 expressions, of course, but those tend to be a lot simpler.)

I missed that point (because I was only thinking to use this in a
sharding environment where all the servers have the same OS and PG).
Thanks for pointing it out!

 So I think worrying about convalidated is pretty pointless.  Really,
 it is up to the user to determine what constraints should be attached
 to the foreign table, and IMPORT FOREIGN SCHEMA can't help much.

Agreed.  So, I'd like to propose to update the docs as above.  Please
find attached a patch.  The existing sentence Checking other types of
constraints is always left to the remote server. sounds to me that
constraints on foreign tables are enforced locally when updating the
foreign tables.  So, I'd also like to propose to remove that sentence.
Maybe I'm missing something though.

I'll change the name and category of this in CF 2015-06, accordingly.

Best regards,
Etsuro Fujita
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 1079140..8e39052 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -361,9 +361,11 @@
 
para
 Note that constraints other than literalNOT NULL/ will never be
-imported from the remote tables, since productnamePostgreSQL/
-does not support any other type of constraint on a foreign table.
-Checking other types of constraints is always left to the remote server.
+imported from the remote tables.  Other than literalNOT NULL/,
+productnamePostgreSQL/ supports literalCHECK/ constraints
+on foreign tables.  However, it is up to you to determine which
+literalCHECK/ constraints on the remote tables should be attached
+to the foreign table definitions.
/para
   /sect3
  /sect2

-- 
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] Missing importing option of postgres_fdw

2015-05-25 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
 On 2015/05/22 23:50, Tom Lane wrote:
 So I think worrying about convalidated is pretty pointless.  Really,
 it is up to the user to determine what constraints should be attached
 to the foreign table, and IMPORT FOREIGN SCHEMA can't help much.

 Agreed.  So, I'd like to propose to update the docs as above.  Please
 find attached a patch.  The existing sentence Checking other types of
 constraints is always left to the remote server. sounds to me that
 constraints on foreign tables are enforced locally when updating the
 foreign tables.  So, I'd also like to propose to remove that sentence.
 Maybe I'm missing something though.

Yeah, I agree this documentation is inadequate; but I think it'd be a good
thing to spell out the reason why there's no support for importing check
constraints.  I committed the attached enlarged version.

regards, tom lane

diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 1079140..14b12e3 100644
*** a/doc/src/sgml/postgres-fdw.sgml
--- b/doc/src/sgml/postgres-fdw.sgml
***
*** 309,316 
  using xref linkend=sql-importforeignschema.  This command creates
  foreign table definitions on the local server that match tables or
  views present on the remote server.  If the remote tables to be imported
! have columns of user-defined data types, the local server must have types
! of the same names.
 /para
  
 para
--- 309,316 
  using xref linkend=sql-importforeignschema.  This command creates
  foreign table definitions on the local server that match tables or
  views present on the remote server.  If the remote tables to be imported
! have columns of user-defined data types, the local server must have
! compatible types of the same names.
 /para
  
 para
***
*** 361,369 
  
 para
  Note that constraints other than literalNOT NULL/ will never be
! imported from the remote tables, since productnamePostgreSQL/
! does not support any other type of constraint on a foreign table.
! Checking other types of constraints is always left to the remote server.
 /para
/sect3
   /sect2
--- 361,376 
  
 para
  Note that constraints other than literalNOT NULL/ will never be
! imported from the remote tables.  Although productnamePostgreSQL/
! does support literalCHECK/ constraints on foreign tables, there is no
! provision for importing them automatically, because of the risk that a
! constraint expression could evaluate differently on the local and remote
! servers.  Any such inconsistency in the behavior of a literalCHECK/
! constraint could lead to hard-to-detect errors in query optimization.
! So if you wish to import literalCHECK/ constraints, you must do so
! manually, and you should verify the semantics of each one carefully.
! For more detail about the treatment of literalCHECK/ constraints on
! foreign tables, see xref linkend=sql-createforeigntable.
 /para
/sect3
   /sect2

-- 
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] Missing importing option of postgres_fdw

2015-05-25 Thread Etsuro Fujita

On 2015/05/26 3:15, Tom Lane wrote:

Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:

On 2015/05/22 23:50, Tom Lane wrote:

So I think worrying about convalidated is pretty pointless.  Really,
it is up to the user to determine what constraints should be attached
to the foreign table, and IMPORT FOREIGN SCHEMA can't help much.



Agreed.  So, I'd like to propose to update the docs as above.  Please
find attached a patch.  The existing sentence Checking other types of
constraints is always left to the remote server. sounds to me that
constraints on foreign tables are enforced locally when updating the
foreign tables.  So, I'd also like to propose to remove that sentence.
Maybe I'm missing something though.


Yeah, I agree this documentation is inadequate; but I think it'd be a good
thing to spell out the reason why there's no support for importing check
constraints.  I committed the attached enlarged version.


Thanks!

Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-05-22 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
 I agree with you on that point.  And I think one option for that is that 
 IMPORT FOREIGN SCHEMA only imports CHECK constraints of remote tables 
 from a remote server that have convalidated = true.  (If doing so, we 
 wouldn't need to allow IMPORT FOREIGN SCHEMA to return ALTER FOREIGN 
 TABLE statements.)  But I'm not sure that that is a good idea.  ISTM it 
 would be better for IMPORT FOREIGN SCHEMA just to imports all CHECK 
 constraints of remote tables, reflecting convalidated, and that we leave 
 it to the user to do VALIDATE CONSTRAINT for locally defined constraints 
 that have convalidated = false when matching constraints are validated 
 on the remote server.

It would only be safe to automatically import CHECK constraints if they
contain no expressions that could evaluate differently on remote and local
--- IOW, only expressions that would be safe to transmit as remote query
conditions.  I don't really think we should try to do that at all.

For starters, while it might seem like we could use is_foreign_expr()
on the conditions, there's no guarantee that the local server accurately
knows what functions on the foreign server are really safe.  The is safe
to transmit condition isn't symmetric.

For that matter, there's no guarantee that we could even parse the
remote's constraint condition text without failing --- it might use SQL
features we don't have.  (We have that risk already for DEFAULT
expressions, of course, but those tend to be a lot simpler.)

So I think worrying about convalidated is pretty pointless.  Really,
it is up to the user to determine what constraints should be attached
to the foreign table, and IMPORT FOREIGN SCHEMA can't help much.

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] Missing importing option of postgres_fdw

2015-05-21 Thread Robert Haas
On Mon, May 18, 2015 at 4:03 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 On 2015/05/16 3:32, Robert Haas wrote:
 On Thu, May 14, 2015 at 6:37 AM, Etsuro Fujita
 fujita.ets...@lab.ntt.co.jp wrote:

 On second thought, I noticed that as for this option, we cannot live
 without
 allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements
 because we cannot declare the convalidated information in the CREATE
 FOREIGN
 TABLE statement.  So, I think we shoould also allow it to return ALTER
 FOREIGN TABLE statements.  Am I right?

 Isn't convalidated utterly meaningless for constraints on foreign tables?

 Let me explain.  I think that convalidated would be *essential* for
 accurately performing relation_excluded_by_constraints for foreign tables
 like plain tables; if we didn't have that information, I think we would fail
 to accurately detect whether foreign tables need not be scanned.

My point is that any constraint on a foreign table is just something
we HOPE the remote side is enforcing.  Regardless of whether
convalidated is true or false locally, it could have some other value
on the remote side, or the constraint might not exist on the remote
side at all.

-- 
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] Missing importing option of postgres_fdw

2015-05-18 Thread Amit Langote
On 2015-05-18 PM 05:03, Etsuro Fujita wrote:
 On 2015/05/16 3:32, Robert Haas wrote:
 On Thu, May 14, 2015 at 6:37 AM, Etsuro Fujita
 fujita.ets...@lab.ntt.co.jp wrote:
 On second thought, I noticed that as for this option, we cannot live without
 allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements
 because we cannot declare the convalidated information in the CREATE FOREIGN
 TABLE statement.  So, I think we shoould also allow it to return ALTER
 FOREIGN TABLE statements.  Am I right?

 Isn't convalidated utterly meaningless for constraints on foreign tables?
 
 Let me explain.  I think that convalidated would be *essential* for accurately
 performing relation_excluded_by_constraints for foreign tables like plain
 tables; if we didn't have that information, I think we would fail to
 accurately detect whether foreign tables need not be scanned.
 

So, if we decide to reflect remote/accurate convalidated locally by using the
method you propose then would it mean only the foreign tables imported using
IMPORT FOREIGN SCHEMA will have accurate convalidated info? AFAICS, there is
no way to ensure the same for foreign tables created in normal way (CREATE
FOREIGN TABLE) nor is there a way to propagate ALTER TABLE ... VALIDATE
CONSTRAINT on a foreign table to remote side so that convalidated on the local
side really matches the reality (on remote side). Does that cause inconsistent
behavior or am I missing something?

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] Missing importing option of postgres_fdw

2015-05-18 Thread Etsuro Fujita

On 2015/05/16 3:32, Robert Haas wrote:

On Thu, May 14, 2015 at 6:37 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:

On second thought, I noticed that as for this option, we cannot live without
allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements
because we cannot declare the convalidated information in the CREATE FOREIGN
TABLE statement.  So, I think we shoould also allow it to return ALTER
FOREIGN TABLE statements.  Am I right?


Isn't convalidated utterly meaningless for constraints on foreign tables?


Let me explain.  I think that convalidated would be *essential* for 
accurately performing relation_excluded_by_constraints for foreign 
tables like plain tables; if we didn't have that information, I think we 
would fail to accurately detect whether foreign tables need not be scanned.


BTW, I don't know if it's a good idea to import connoinherit from the 
remote and then reflect that information on the local.


Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-05-18 Thread Etsuro Fujita

On 2015/05/18 17:44, Amit Langote wrote:

On 2015-05-18 PM 05:03, Etsuro Fujita wrote:

On 2015/05/16 3:32, Robert Haas wrote:

On Thu, May 14, 2015 at 6:37 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:

On second thought, I noticed that as for this option, we cannot live without
allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements
because we cannot declare the convalidated information in the CREATE FOREIGN
TABLE statement.  So, I think we shoould also allow it to return ALTER
FOREIGN TABLE statements.  Am I right?


Isn't convalidated utterly meaningless for constraints on foreign tables?


Let me explain.  I think that convalidated would be *essential* for accurately
performing relation_excluded_by_constraints for foreign tables like plain
tables; if we didn't have that information, I think we would fail to
accurately detect whether foreign tables need not be scanned.



So, if we decide to reflect remote/accurate convalidated locally by using the
method you propose then would it mean only the foreign tables imported using
IMPORT FOREIGN SCHEMA will have accurate convalidated info? AFAICS, there is
no way to ensure the same for foreign tables created in normal way (CREATE
FOREIGN TABLE) nor is there a way to propagate ALTER TABLE ... VALIDATE
CONSTRAINT on a foreign table to remote side so that convalidated on the local
side really matches the reality (on remote side). Does that cause inconsistent
behavior or am I missing something?


We now allow ALTER FOREIGN TABLE ADD CONSTRAINT NOT VALID.  So after 
creating foreign tables in normal way (CREATE FOREIGN TABLE), we can 
manually define CHECK constraints that have convalidated = false by that 
command.


Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-05-18 Thread Amit Langote
On 2015-05-18 PM 06:45, Etsuro Fujita wrote:
 On 2015/05/18 17:44, Amit Langote wrote:
 On 2015-05-18 PM 05:03, Etsuro Fujita wrote:
 Let me explain.  I think that convalidated would be *essential* for 
 accurately
 performing relation_excluded_by_constraints for foreign tables like plain
 tables; if we didn't have that information, I think we would fail to
 accurately detect whether foreign tables need not be scanned.
 
 So, if we decide to reflect remote/accurate convalidated locally by using the
 method you propose then would it mean only the foreign tables imported using
 IMPORT FOREIGN SCHEMA will have accurate convalidated info? AFAICS, there is
 no way to ensure the same for foreign tables created in normal way (CREATE
 FOREIGN TABLE) nor is there a way to propagate ALTER TABLE ... VALIDATE
 CONSTRAINT on a foreign table to remote side so that convalidated on the 
 local
 side really matches the reality (on remote side). Does that cause 
 inconsistent
 behavior or am I missing something?
 
 We now allow ALTER FOREIGN TABLE ADD CONSTRAINT NOT VALID.  So after creating
 foreign tables in normal way (CREATE FOREIGN TABLE), we can manually define
 CHECK constraints that have convalidated = false by that command.
 

Ah, so creating a NOT VALID constraint *prevents* potentially wrong exclusion
of a foreign table at least based on that constraint. Thanks for reminding of
that option.

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] Missing importing option of postgres_fdw

2015-05-15 Thread Robert Haas
On Thu, May 14, 2015 at 6:37 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 On second thought, I noticed that as for this option, we cannot live without
 allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements
 because we cannot declare the convalidated information in the CREATE FOREIGN
 TABLE statement.  So, I think we shoould also allow it to return ALTER
 FOREIGN TABLE statements.  Am I right?

Isn't convalidated utterly meaningless for constraints on foreign tables?

-- 
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] Missing importing option of postgres_fdw

2015-05-14 Thread Etsuro Fujita

On 2015/04/30 2:10, Robert Haas wrote:

On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier
michael.paqu...@gmail.com wrote:

Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
with IMPORT FOREIGN SCHEMA is a different feature than what is
proposed in this patch, aka an option for postgres_fdw and meritates a
discussion on its own because it impacts all the FDWs and not only
postgres_fdw. Now, related to this patch, we could live without
authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
authorize the definition of CHECK constraints.



I agree.  I don't think there's a huge problem with allowing IMPORT
FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it
doesn't really seem to be necessary.  I don't see why we can't just
declare the CHECK constraints in the CREATE FOREIGN TABLE statement
instead of adding more DDL.


On second thought, I noticed that as for this option, we cannot live 
without allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE 
statements because we cannot declare the convalidated information in the 
CREATE FOREIGN TABLE statement.  So, I think we shoould also allow it to 
return ALTER FOREIGN TABLE statements.  Am I right?


Comments welcome.

Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-05-01 Thread Etsuro Fujita

On 2015/04/30 17:15, Etsuro Fujita wrote:

On 2015/04/30 2:10, Robert Haas wrote:

On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier
michael.paqu...@gmail.com wrote:

Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
with IMPORT FOREIGN SCHEMA is a different feature than what is
proposed in this patch, aka an option for postgres_fdw and meritates a
discussion on its own because it impacts all the FDWs and not only
postgres_fdw. Now, related to this patch, we could live without
authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
authorize the definition of CHECK constraints.


I agree.  I don't think there's a huge problem with allowing IMPORT
FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it
doesn't really seem to be necessary.  I don't see why we can't just
declare the CHECK constraints in the CREATE FOREIGN TABLE statement
instead of adding more DDL.


I think that it'd improve the convenience of an FDW developer writing
ImportForeignSchema() to allow it to return ALTER FOREIGN TABLE (and
perhaps DROP FOREIGN TABLE) as well, but I agree that that needs another
discussion.  So I'll leave that as is and update the patch as discussed
above.


Done.  Please find attached an updated version of the patch.

Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***
*** 3415,3420  CREATE TABLE import_source.t2 (c1 int default 42, c2 varchar NULL, c3 text colla
--- 3415,3421 
  CREATE TYPE typ1 AS (m1 int, m2 varchar);
  CREATE TABLE import_source.t3 (c1 timestamptz default now(), c2 typ1);
  CREATE TABLE import_source.x 4 (c1 float8, C 2 text, c3 varchar(42));
+ ALTER TABLE import_source.x 4 ADD CHECK (c1 = 0);
  CREATE TABLE import_source.x 5 (c1 float8);
  ALTER TABLE import_source.x 5 DROP COLUMN c1;
  CREATE SCHEMA import_dest1;
***
*** 3462,3467  FDW Options: (schema_name 'import_source', table_name 't3')
--- 3463,3470 
   c1 | double precision  |   | (column_name 'c1')
   C 2| text  |   | (column_name 'C 2')
   c3 | character varying(42) |   | (column_name 'c3')
+ Check constraints:
+ x 4_c1_check CHECK (c1 = 0::double precision)
  Server: loopback
  FDW Options: (schema_name 'import_source', table_name 'x 4')
  
***
*** 3518,3523  FDW Options: (schema_name 'import_source', table_name 't3')
--- 3521,3528 
   c1 | double precision  |   | (column_name 'c1')
   C 2| text  |   | (column_name 'C 2')
   c3 | character varying(42) |   | (column_name 'c3')
+ Check constraints:
+ x 4_c1_check CHECK (c1 = 0::double precision)
  Server: loopback
  FDW Options: (schema_name 'import_source', table_name 'x 4')
  
***
*** 3529,3535  FDW Options: (schema_name 'import_source', table_name 'x 5')
  
  CREATE SCHEMA import_dest3;
  IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
!   OPTIONS (import_collate 'false', import_not_null 'false');
  \det+ import_dest3
   List of foreign tables
  Schema| Table |  Server  |   FDW Options   | Description 
--- 3534,3540 
  
  CREATE SCHEMA import_dest3;
  IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
!   OPTIONS (import_collate 'false', import_not_null 'false', import_check 'false');
  \det+ import_dest3
   List of foreign tables
  Schema| Table |  Server  |   FDW Options   | Description 
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***
*** 2584,2594  postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
--- 2584,2597 
  	bool		import_collate = true;
  	bool		import_default = false;
  	bool		import_not_null = true;
+ 	bool		import_check = true;
  	ForeignServer *server;
  	UserMapping *mapping;
  	PGconn	   *conn;
  	StringInfoData buf;
+ 	StringInfoData buf2;
  	PGresult   *volatile res = NULL;
+ 	PGresult   *volatile res2 = NULL;
  	int			numrows,
  i;
  	ListCell   *lc;
***
*** 2604,2609  postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
--- 2607,2614 
  			import_default = defGetBoolean(def);
  		else if (strcmp(def-defname, import_not_null) == 0)
  			import_not_null = defGetBoolean(def);
+ 		else if (strcmp(def-defname, import_check) == 0)
+ 			import_check = defGetBoolean(def);
  		else
  			ereport(ERROR,
  	(errcode(ERRCODE_FDW_INVALID_OPTION_NAME),
***
*** 2624,2629  postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
--- 2629,2635 
  
  	/* Create workspace for strings */
  	initStringInfo(buf);
+ 	initStringInfo(buf2);
  
  	/* In what follows, do not risk leaking any PGresults. */
 

Re: [HACKERS] Missing importing option of postgres_fdw

2015-04-30 Thread Etsuro Fujita

On 2015/04/30 2:10, Robert Haas wrote:

On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier
michael.paqu...@gmail.com wrote:

Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
with IMPORT FOREIGN SCHEMA is a different feature than what is
proposed in this patch, aka an option for postgres_fdw and meritates a
discussion on its own because it impacts all the FDWs and not only
postgres_fdw. Now, related to this patch, we could live without
authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
authorize the definition of CHECK constraints.


I agree.  I don't think there's a huge problem with allowing IMPORT
FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it
doesn't really seem to be necessary.  I don't see why we can't just
declare the CHECK constraints in the CREATE FOREIGN TABLE statement
instead of adding more DDL.


I think that it'd improve the convenience of an FDW developer writing 
ImportForeignSchema() to allow it to return ALTER FOREIGN TABLE (and 
perhaps DROP FOREIGN TABLE) as well, but I agree that that needs another 
discussion.  So I'll leave that as is and update the patch as discussed 
above.


Thanks for the comments!

Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-04-29 Thread Robert Haas
On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
 with IMPORT FOREIGN SCHEMA is a different feature than what is
 proposed in this patch, aka an option for postgres_fdw and meritates a
 discussion on its own because it impacts all the FDWs and not only
 postgres_fdw. Now, related to this patch, we could live without
 authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
 authorize the definition of CHECK constraints.

I agree.  I don't think there's a huge problem with allowing IMPORT
FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it
doesn't really seem to be necessary.  I don't see why we can't just
declare the CHECK constraints in the CREATE FOREIGN TABLE statement
instead of adding more DDL.

-- 
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] Missing importing option of postgres_fdw

2015-04-27 Thread Michael Paquier
On Mon, Apr 27, 2015 at 3:15 PM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 Hi,

 I noticed that there is no postgres_fdw option to control whether check
 constraints on remote tables are included in the definitions of foreign
 tables imported from a remote PG server when performing IMPORT FOREIGN
 SCHEMA, while we now allow check constraints on foreign tables.

 Attached is a patch for that.  I'll add this to the next CF.

I guess that the addition of this option makes sense, but I think that
this patch is doing it wrong by using ALTER FOREIGN TABLE and by
changing the queries authorized in ImportForeignSchema(). The point of
IMPORT FOREIGN SCHEMA is to authorize only CREATE FOREIGN TABLE and
not other types of queries, not to mention that CREATE FOREIGN TABLE
supports the definitions of constraints at table and column-level.
Logically, this patch should just create diffs with postgres_fdw and
nothing else.
Regards,
-- 
Michael


-- 
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] Missing importing option of postgres_fdw

2015-04-27 Thread Etsuro Fujita

On 2015/04/27 15:51, Michael Paquier wrote:

On Mon, Apr 27, 2015 at 3:15 PM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:

I noticed that there is no postgres_fdw option to control whether check
constraints on remote tables are included in the definitions of foreign
tables imported from a remote PG server when performing IMPORT FOREIGN
SCHEMA, while we now allow check constraints on foreign tables.



I guess that the addition of this option makes sense, but I think that
this patch is doing it wrong by using ALTER FOREIGN TABLE and by
changing the queries authorized in ImportForeignSchema(). The point of
IMPORT FOREIGN SCHEMA is to authorize only CREATE FOREIGN TABLE and
not other types of queries, not to mention that CREATE FOREIGN TABLE
supports the definitions of constraints at table and column-level.


I don't think so.  IMO, I think it'd be more convenient and flexible to 
allow ALTER FOREIGN TABLE for creating foreign table definitions during 
ImportForeignSchema().


Thanks for the comment!

Best regards,
Etsuro Fujita


--
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] Missing importing option of postgres_fdw

2015-04-27 Thread Michael Paquier
On Mon, Apr 27, 2015 at 4:20 PM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 On 2015/04/27 15:51, Michael Paquier wrote:

 On Mon, Apr 27, 2015 at 3:15 PM, Etsuro Fujita
 fujita.ets...@lab.ntt.co.jp wrote:

 I noticed that there is no postgres_fdw option to control whether check
 constraints on remote tables are included in the definitions of foreign
 tables imported from a remote PG server when performing IMPORT FOREIGN
 SCHEMA, while we now allow check constraints on foreign tables.


 I guess that the addition of this option makes sense, but I think that
 this patch is doing it wrong by using ALTER FOREIGN TABLE and by
 changing the queries authorized in ImportForeignSchema(). The point of
 IMPORT FOREIGN SCHEMA is to authorize only CREATE FOREIGN TABLE and
 not other types of queries, not to mention that CREATE FOREIGN TABLE
 supports the definitions of constraints at table and column-level.


 I don't think so.  IMO, I think it'd be more convenient and flexible to
 allow ALTER FOREIGN TABLE for creating foreign table definitions during
 ImportForeignSchema().

Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
with IMPORT FOREIGN SCHEMA is a different feature than what is
proposed in this patch, aka an option for postgres_fdw and meritates a
discussion on its own because it impacts all the FDWs and not only
postgres_fdw. Now, related to this patch, we could live without
authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
authorize the definition of CHECK constraints. Also, I imagine that it
would be more natural to combine the fetch of the constraint
expression in the existing query with a join on conrelid so as to not
replicate the logic that your patch is doing with
FDW_IMPORT_SCHEMA_LIMIT_TO and FDW_IMPORT_SCHEMA_EXCEPT that reuses
the same logic to re-build the [ NOT ] IN clause.
-- 
Michael


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