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. */
PG_TRY();
***************
*** 2663,2669 **** postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
" attnotnull, "
" pg_get_expr(adbin, adrelid), "
" collname, "
! " collnsp.nspname "
"FROM pg_class c "
" JOIN pg_namespace n ON "
" relnamespace = n.oid "
--- 2669,2677 ----
" attnotnull, "
" pg_get_expr(adbin, adrelid), "
" collname, "
! " collnsp.nspname, "
! " c.oid, "
! " relchecks "
"FROM pg_class c "
" JOIN pg_namespace n ON "
" relnamespace = n.oid "
***************
*** 2683,2689 **** postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
" format_type(atttypid, atttypmod), "
" attnotnull, "
" pg_get_expr(adbin, adrelid), "
! " NULL, NULL "
"FROM pg_class c "
" JOIN pg_namespace n ON "
" relnamespace = n.oid "
--- 2691,2699 ----
" format_type(atttypid, atttypmod), "
" attnotnull, "
" pg_get_expr(adbin, adrelid), "
! " NULL, NULL, "
! " c.oid, "
! " relchecks "
"FROM pg_class c "
" JOIN pg_namespace n ON "
" relnamespace = n.oid "
***************
*** 2803,2808 **** postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
--- 2813,2860 ----
while (++i < numrows &&
strcmp(PQgetvalue(res, i, 0), tablename) == 0);
+ /* Add all CHECK constraints if needed */
+ if (import_check)
+ {
+ char *tableoid = PQgetvalue(res, i - 1, 7);
+ bool hascheck = (atoi(PQgetvalue(res, i - 1, 8)) > 0);
+
+ if (hascheck)
+ {
+ int numchecks;
+ int row;
+
+ /* Fetch all CHECK constraint data */
+ resetStringInfo(&buf2);
+ appendStringInfo(&buf2,
+ "SELECT conname, "
+ " pg_get_constraintdef(r.oid) "
+ "FROM pg_constraint r "
+ "WHERE conrelid = '%s' AND contype = 'c' "
+ "ORDER BY conname",
+ tableoid);
+
+ /* Fetch the data */
+ res2 = PQexec(conn, buf2.data);
+ if (PQresultStatus(res2) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res2, conn, false, buf2.data);
+
+ /* Process results */
+ numchecks = PQntuples(res2);
+ for (row = 0; row < numchecks; row++)
+ {
+ char *condef = PQgetvalue(res2, row, 1);
+
+ appendStringInfoString(&buf, ",\n");
+ appendStringInfo(&buf, " %s", condef);
+ }
+
+ /* Clean up */
+ PQclear(res2);
+ res2 = NULL;
+ }
+ }
+
/*
* Add server name and table-level options. We specify remote
* schema and table name as options (the latter to ensure that
***************
*** 2829,2834 **** postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
--- 2881,2888 ----
{
if (res)
PQclear(res);
+ if (res2)
+ PQclear(res2);
PG_RE_THROW();
}
PG_END_TRY();
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 785,790 **** CREATE TABLE import_source.t2 (c1 int default 42, c2 varchar NULL, c3 text colla
--- 785,791 ----
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;
***************
*** 801,807 **** IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
\d import_dest2.*
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
\d import_dest3.*
--- 802,808 ----
\d import_dest2.*
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
\d import_dest3.*
*** a/doc/src/sgml/postgres-fdw.sgml
--- b/doc/src/sgml/postgres-fdw.sgml
***************
*** 349,360 ****
</para>
</listitem>
</varlistentry>
</variablelist>
<para>
! Note that constraints other than <literal>NOT NULL</> will never be
! imported from the remote tables, since <productname>PostgreSQL</>
! 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>
--- 349,371 ----
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>import_check</literal></term>
+ <listitem>
+ <para>
+ This option controls whether column and table <literal>CHECK</>
+ constraints are included in the definitions of foreign tables imported
+ from a foreign server. The default is <literal>true</>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<para>
! Note that constraints other than <literal>NOT NULL</> and
! <literal>CHECK</> will never be imported from the remote tables, since
! <productname>PostgreSQL</> 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>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
