(2014/11/12 20:04), Ashutosh Bapat wrote:
I reviewed fdw-chk-3 patch. Here are my comments

Thanks for the review!

Tests
-------
1. The tests added in file_fdw module look good. We should add tests for
CREATE TABLE with CHECK CONSTRAINT also.

Agreed.  I added the tests, and also changed the proposed tests a bit.

2.  For postgres_fdw we need tests to check the behaviour in case the
constraints mismatch between the remote table and its local foreign
table declaration in case of INSERT, UPDATE and SELECT.

Done.

3. In the testcases for postgres_fdw it seems that you have forgotten to
add statement after SET constraint_exclusion to 'partition'

I added the statement.

4. In test foreign_data there are changes to fix the diffs caused by
these changes like below
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
-ERROR:  "ft1" is not a table
+ERROR:  constraint "no_const" of relation "ft1" does not exist
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
-ERROR:  "ft1" is not a table
+NOTICE:  constraint "no_const" of relation "ft1" does not exist, skipping
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check;
-ERROR:  "ft1" is not a table
+ERROR:  constraint "ft1_c1_check" of relation "ft1" does not exist

Earlier when constraints were not supported for FOREIGN TABLE, these
tests made sure the same functionality. So, even though the
corresponding constraints were not created on the table (in fact it
didn't allow the creation as well). Now that the constraints are
allowed, I think the tests for "no_const" (without IF EXISTS) and
"ft1_c1_check" are duplicating the same testcase. May be we should
review this set of statement in the light of new functionality.

Agreed. I removed the "DROP CONSTRAINT ft1_c1_check" test, and added a new test for ALTER CONSTRAINT.

Code and implementation
----------------------------------
The usage of NO INHERIT and NOT VALID with CONSTRAINT on foreign table
is blocked, but corresponding documentation entry doesn't mention so.
Since foreign tables can not be inherited NO INHERIT option isn't
applicable to foreign tables and the constraints on the foreign tables
are declarative, hence NOT VALID option is also not applicable. So, I
agree with what the code is doing, but that should be reflected in
documentation with this explanation.
Rest of the code modifies the condition checks for CHECK CONSTRAINTs on
foreign tables, and it looks good to me.

Done.

Other changes:
* Modified one error message that I added in AddRelationNewConstraints, to match the other message there.
* Revised other docs a little bit.

Attached is an updated version of the patch.

Thanks,

Best regards,
Etsuro Fujita
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
***************
*** 62,68 **** CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'csv', null '
  CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
  
  CREATE FOREIGN TABLE agg_text (
! 	a	int2,
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/agg.data', delimiter '	', null '\N');
--- 62,68 ----
  CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
  
  CREATE FOREIGN TABLE agg_text (
! 	a	int2 CHECK (a >= 0),
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/agg.data', delimiter '	', null '\N');
***************
*** 72,82 **** CREATE FOREIGN TABLE agg_csv (
--- 72,84 ----
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.csv', header 'true', delimiter ';', quote '@', escape '"', null '');
+ ALTER FOREIGN TABLE agg_csv ADD CHECK (a >= 0);
  CREATE FOREIGN TABLE agg_bad (
  	a	int2,
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
+ ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);
  
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
***************
*** 134,139 **** DELETE FROM agg_csv WHERE a = 100;
--- 136,153 ----
  -- but this should be ignored
  SELECT * FROM agg_csv FOR UPDATE;
  
+ -- constraint exclusion tests
+ \t on
+ EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a < 0;
+ \t off
+ SELECT * FROM agg_csv WHERE a < 0;
+ SET constraint_exclusion = 'on';
+ \t on
+ EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a < 0;
+ \t off
+ SELECT * FROM agg_csv WHERE a < 0;
+ SET constraint_exclusion = 'partition';
+ 
  -- privilege tests
  SET ROLE file_fdw_superuser;
  SELECT * FROM agg_text ORDER BY a;
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
***************
*** 78,84 **** ERROR:  COPY null representation cannot use newline or carriage return
  CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
  ERROR:  filename is required for file_fdw foreign tables
  CREATE FOREIGN TABLE agg_text (
! 	a	int2,
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/agg.data', delimiter '	', null '\N');
--- 78,84 ----
  CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
  ERROR:  filename is required for file_fdw foreign tables
  CREATE FOREIGN TABLE agg_text (
! 	a	int2 CHECK (a >= 0),
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/agg.data', delimiter '	', null '\N');
***************
*** 88,98 **** CREATE FOREIGN TABLE agg_csv (
--- 88,100 ----
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.csv', header 'true', delimiter ';', quote '@', escape '"', null '');
+ ALTER FOREIGN TABLE agg_csv ADD CHECK (a >= 0);
  CREATE FOREIGN TABLE agg_bad (
  	a	int2,
  	b	float4
  ) SERVER file_server
  OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null '');
+ ALTER FOREIGN TABLE agg_bad ADD CHECK (a >= 0);
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
      word1 text OPTIONS (force_not_null 'true'),
***************
*** 219,224 **** SELECT * FROM agg_csv FOR UPDATE;
--- 221,254 ----
    42 |  324.78
  (3 rows)
  
+ -- constraint exclusion tests
+ \t on
+ EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a < 0;
+  Foreign Scan on public.agg_csv
+    Output: a, b
+    Filter: (agg_csv.a < 0)
+    Foreign File: @abs_srcdir@/data/agg.csv
+ 
+ \t off
+ SELECT * FROM agg_csv WHERE a < 0;
+  a | b 
+ ---+---
+ (0 rows)
+ 
+ SET constraint_exclusion = 'on';
+ \t on
+ EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a < 0;
+  Result
+    Output: a, b
+    One-Time Filter: false
+ 
+ \t off
+ SELECT * FROM agg_csv WHERE a < 0;
+  a | b 
+ ---+---
+ (0 rows)
+ 
+ SET constraint_exclusion = 'partition';
  -- privilege tests
  SET ROLE file_fdw_superuser;
  SELECT * FROM agg_text ORDER BY a;
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 2488,2493 **** select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1;
--- 2488,2578 ----
  (13 rows)
  
  -- ===================================================================
+ -- test check constraints
+ -- ===================================================================
+ -- Consistent check constraints provide consistent results
+ ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c2positive CHECK (c2 >= 0);
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 < 0;
+                             QUERY PLAN                             
+ -------------------------------------------------------------------
+  Aggregate
+    Output: count(*)
+    ->  Foreign Scan on public.ft1
+          Remote SQL: SELECT NULL FROM "S 1"."T 1" WHERE ((c2 < 0))
+ (4 rows)
+ 
+ SELECT count(*) FROM ft1 WHERE c2 < 0;
+  count 
+ -------
+      0
+ (1 row)
+ 
+ SET constraint_exclusion = 'on';
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 < 0;
+            QUERY PLAN           
+ --------------------------------
+  Aggregate
+    Output: count(*)
+    ->  Result
+          One-Time Filter: false
+ (4 rows)
+ 
+ SELECT count(*) FROM ft1 WHERE c2 < 0;
+  count 
+ -------
+      0
+ (1 row)
+ 
+ SET constraint_exclusion = 'partition';
+ -- Can throw errors on remote side during update
+ INSERT INTO ft1(c1, c2) VALUES(1111, -2);  -- c2positive
+ ERROR:  new row for relation "T 1" violates check constraint "c2positive"
+ DETAIL:  Failing row contains (1111, -2, null, null, null, null, ft1       , null).
+ CONTEXT:  Remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+ UPDATE ft1 SET c2 = -c2 WHERE c1 = 1;  -- c2positive
+ ERROR:  new row for relation "T 1" violates check constraint "c2positive"
+ DETAIL:  Failing row contains (1, -1, 00001_trig_update, 1970-01-02 08:00:00+00, 1970-01-02 00:00:00, 1, 1         , foo).
+ CONTEXT:  Remote SQL command: UPDATE "S 1"."T 1" SET c2 = $2 WHERE ctid = $1
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c2positive;
+ -- But inconsistent check constraints provide inconsistent results
+ ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c2negative CHECK (c2 < 0);
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 >= 0;
+                              QUERY PLAN                             
+ --------------------------------------------------------------------
+  Aggregate
+    Output: count(*)
+    ->  Foreign Scan on public.ft1
+          Remote SQL: SELECT NULL FROM "S 1"."T 1" WHERE ((c2 >= 0))
+ (4 rows)
+ 
+ SELECT count(*) FROM ft1 WHERE c2 >= 0;
+  count 
+ -------
+    821
+ (1 row)
+ 
+ SET constraint_exclusion = 'on';
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 >= 0;
+            QUERY PLAN           
+ --------------------------------
+  Aggregate
+    Output: count(*)
+    ->  Result
+          One-Time Filter: false
+ (4 rows)
+ 
+ SELECT count(*) FROM ft1 WHERE c2 >= 0;
+  count 
+ -------
+      0
+ (1 row)
+ 
+ SET constraint_exclusion = 'partition';
+ -- Can't throw errors on remote side during update
+ INSERT INTO ft1(c1, c2) VALUES(1111, 2);
+ UPDATE ft1 SET c2 = c2 + 1 WHERE c1 = 1;
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c2negative;
+ -- ===================================================================
  -- test serial columns (ie, sequence-based defaults)
  -- ===================================================================
  create table loc1 (f1 serial, f2 text);
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 387,392 **** select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1;
--- 387,422 ----
  select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1;
  
  -- ===================================================================
+ -- test check constraints
+ -- ===================================================================
+ 
+ -- Consistent check constraints provide consistent results
+ ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c2positive CHECK (c2 >= 0);
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 < 0;
+ SELECT count(*) FROM ft1 WHERE c2 < 0;
+ SET constraint_exclusion = 'on';
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 < 0;
+ SELECT count(*) FROM ft1 WHERE c2 < 0;
+ SET constraint_exclusion = 'partition';
+ -- Can throw errors on remote side during update
+ INSERT INTO ft1(c1, c2) VALUES(1111, -2);  -- c2positive
+ UPDATE ft1 SET c2 = -c2 WHERE c1 = 1;  -- c2positive
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c2positive;
+ 
+ -- But inconsistent check constraints provide inconsistent results
+ ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c2negative CHECK (c2 < 0);
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 >= 0;
+ SELECT count(*) FROM ft1 WHERE c2 >= 0;
+ SET constraint_exclusion = 'on';
+ EXPLAIN (VERBOSE, COSTS false) SELECT count(*) FROM ft1 WHERE c2 >= 0;
+ SELECT count(*) FROM ft1 WHERE c2 >= 0;
+ SET constraint_exclusion = 'partition';
+ -- Can't throw errors on remote side during update
+ INSERT INTO ft1(c1, c2) VALUES(1111, 2);
+ UPDATE ft1 SET c2 = c2 + 1 WHERE c1 = 1;
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c2negative;
+ 
+ -- ===================================================================
  -- test serial columns (ie, sequence-based defaults)
  -- ===================================================================
  create table loc1 (f1 serial, f2 text);
*** a/doc/src/sgml/ref/alter_foreign_table.sgml
--- b/doc/src/sgml/ref/alter_foreign_table.sgml
***************
*** 42,47 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceab
--- 42,49 ----
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> SET ( <replaceable class="PARAMETER">attribute_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> RESET ( <replaceable class="PARAMETER">attribute_option</replaceable> [, ... ] )
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> OPTIONS ( [ ADD | SET | DROP ] <replaceable class="PARAMETER">option</replaceable> ['<replaceable class="PARAMETER">value</replaceable>'] [, ... ])
+     ADD <replaceable class="PARAMETER">table_constraint</replaceable>
+     DROP CONSTRAINT [ IF EXISTS ]  <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ]
      DISABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
      ENABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
      ENABLE REPLICA TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable>
***************
*** 152,157 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceab
--- 154,188 ----
      </listitem>
     </varlistentry>
  
+      <varlistentry>
+       <term><literal>ADD <replaceable class="PARAMETER">table_constraint</replaceable></literal></term>
+       <listitem>
+        <para>
+         This form adds a new constraint to a foreign table, using the same syntax as
+         <xref linkend="SQL-CREATEFOREIGNTABLE">.
+         Unlike the case when adding a constraint to a regular table, nothing happens
+         to the underlying storage: this action simply declares that
+         some new constraint holds for all rows in the foreign table.
+        </para>
+ 
+        <para>
+         Note that constraints on foreign tables cannot be marked
+         <literal>NOT VALID</> since those constraints are simply declarative.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
+       <term><literal>DROP CONSTRAINT [ IF EXISTS ]</literal></term>
+       <listitem>
+        <para>
+         This form drops the specified constraint on a foreign table.
+         If <literal>IF EXISTS</literal> is specified and the constraint
+         does not exist, no error is thrown. In this case a notice is issued instead.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
     <varlistentry>
      <term><literal>DISABLE</literal>/<literal>ENABLE [ REPLICA | ALWAYS ] TRIGGER</literal></term>
      <listitem>
***************
*** 284,289 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceab
--- 315,338 ----
        </listitem>
       </varlistentry>
  
+        <varlistentry>
+         <term><replaceable class="PARAMETER">table_constraint</replaceable></term>
+         <listitem>
+          <para>
+           New table constraint for the table.
+          </para>
+         </listitem>
+        </varlistentry>
+ 
+        <varlistentry>
+         <term><replaceable class="PARAMETER">constraint_name</replaceable></term>
+         <listitem>
+          <para>
+           Name of an existing constraint to drop.
+          </para>
+         </listitem>
+        </varlistentry>
+ 
       <varlistentry>
        <term><literal>CASCADE</literal></term>
        <listitem>
***************
*** 365,374 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceab
     <para>
      Consistency with the foreign server is not checked when a column is added
      or removed with <literal>ADD COLUMN</literal> or
!     <literal>DROP COLUMN</literal>, a <literal>NOT NULL</> constraint is
!     added, or a column type is changed with <literal>SET DATA TYPE</>.  It is
!     the user's responsibility to ensure that the table definition matches the
!     remote side.
     </para>
  
     <para>
--- 414,423 ----
     <para>
      Consistency with the foreign server is not checked when a column is added
      or removed with <literal>ADD COLUMN</literal> or
!     <literal>DROP COLUMN</literal>, a <literal>NOT NULL</> or <literal>CHECK</> 
!     constraint is added, or a column type is changed with <literal>SET DATA TYPE</>.
!     It is the user's responsibility to ensure that the table definition matches
!     the remote side.
     </para>
  
     <para>
*** a/doc/src/sgml/ref/create_foreign_table.sgml
--- b/doc/src/sgml/ref/create_foreign_table.sgml
***************
*** 19,25 ****
   <refsynopsisdiv>
  <synopsis>
  CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name</replaceable> ( [
!     <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ OPTIONS ( <replaceable class="PARAMETER">option</replaceable> '<replaceable class="PARAMETER">value</replaceable>' [, ... ] ) ] [ COLLATE <replaceable>collation</replaceable> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
      [, ... ]
  ] )
    SERVER <replaceable class="parameter">server_name</replaceable>
--- 19,26 ----
   <refsynopsisdiv>
  <synopsis>
  CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name</replaceable> ( [
!   { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ OPTIONS ( <replaceable class="PARAMETER">option</replaceable> '<replaceable class="PARAMETER">value</replaceable>' [, ... ] ) ] [ COLLATE <replaceable>collation</replaceable> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
!     | <replaceable>table_constraint</replaceable> }
      [, ... ]
  ] )
    SERVER <replaceable class="parameter">server_name</replaceable>
***************
*** 30,36 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
--- 31,43 ----
  [ CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ]
  { NOT NULL |
    NULL |
+   CHECK ( <replaceable class="PARAMETER">expression</replaceable> ) |
    DEFAULT <replaceable>default_expr</replaceable> }
+ 
+ <phrase>and <replaceable class="PARAMETER">table_constraint</replaceable> is:</phrase>
+ 
+ [ CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ]
+ { CHECK ( <replaceable class="PARAMETER">expression</replaceable> ) }
  </synopsis>
   </refsynopsisdiv>
  
***************
*** 138,143 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
--- 145,176 ----
     </varlistentry>
  
     <varlistentry>
+     <term><literal>CHECK ( <replaceable class="PARAMETER">expression</replaceable> )</literal></term>
+     <listitem>
+      <para>
+       The <literal>CHECK</> clause specifies an expression producing a
+       Boolean result which each row must satisfy.
+       A check constraint specified as a column constraint should
+       reference that column's value only, while an expression
+       appearing in a table constraint can reference multiple columns.
+      </para>
+ 
+      <para>
+       Currently, <literal>CHECK</literal> expressions cannot contain
+       subqueries nor refer to variables other than columns of the
+       current row.  The system column <literal>tableoid</literal>
+       may be referenced, but not any other system column.
+      </para>
+ 
+      <para>
+       Note that check constraints on foreign tables cannot be marked
+       <literal>NO INHERIT</> since those tables are not allowd to be
+       inherited.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
      <term><literal>DEFAULT
      <replaceable>default_expr</replaceable></literal></term>
      <listitem>
***************
*** 187,192 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
--- 220,235 ----
  
   </refsect1>
  
+  <refsect1>
+   <title>Notes</title>
+ 
+    <para>
+     Constraints on foreign tables are not enforced on insert or update.
+     Those definitions simply declare the constraints hold for all rows
+     in the foreign tables.  It is the user's responsibility to ensure
+     that those definitions match the remote side.
+    </para>
+  </refsect1>
  
   <refsect1 id="SQL-CREATEFOREIGNTABLE-examples">
    <title>Examples</title>
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
***************
*** 2222,2227 **** AddRelationNewConstraints(Relation rel,
--- 2222,2241 ----
  		if (cdef->contype != CONSTR_CHECK)
  			continue;
  
+ 		/* Don't allow NO INHERIT for foreign tables */
+ 		if (cdef->is_no_inherit &&
+ 			rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 					 errmsg("CHECK constraints on foreign tables cannot be marked NO INHERIT")));
+ 
+ 		/* Don't allow NOT VALID for foreign tables */
+ 		if (cdef->skip_validation &&
+ 			rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 					 errmsg("CHECK constraints on foreign tables cannot be marked NOT VALID")));
+ 
  		if (cdef->raw_expr != NULL)
  		{
  			Assert(cdef->cooked_expr == NULL);
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 477,486 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
  		ereport(ERROR,
  				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
  				 errmsg("ON COMMIT can only be used on temporary tables")));
- 	if (stmt->constraints != NIL && relkind == RELKIND_FOREIGN_TABLE)
- 		ereport(ERROR,
- 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- 				 errmsg("constraints are not supported on foreign tables")));
  
  	/*
  	 * Look up the namespace in which we are supposed to create the relation,
--- 477,482 ----
***************
*** 3142,3148 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
  			pass = AT_PASS_ADD_INDEX;
  			break;
  		case AT_AddConstraint:	/* ADD CONSTRAINT */
! 			ATSimplePermissions(rel, ATT_TABLE);
  			/* Recursion occurs during execution phase */
  			/* No command-specific prep needed except saving recurse flag */
  			if (recurse)
--- 3138,3144 ----
  			pass = AT_PASS_ADD_INDEX;
  			break;
  		case AT_AddConstraint:	/* ADD CONSTRAINT */
! 			ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  			/* Recursion occurs during execution phase */
  			/* No command-specific prep needed except saving recurse flag */
  			if (recurse)
***************
*** 3156,3162 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
  			pass = AT_PASS_ADD_CONSTR;
  			break;
  		case AT_DropConstraint:	/* DROP CONSTRAINT */
! 			ATSimplePermissions(rel, ATT_TABLE);
  			/* Recursion occurs during execution phase */
  			/* No command-specific prep needed except saving recurse flag */
  			if (recurse)
--- 3152,3158 ----
  			pass = AT_PASS_ADD_CONSTR;
  			break;
  		case AT_DropConstraint:	/* DROP CONSTRAINT */
! 			ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  			/* Recursion occurs during execution phase */
  			/* No command-specific prep needed except saving recurse flag */
  			if (recurse)
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 515,526 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
  				break;
  
  			case CONSTR_CHECK:
- 				if (cxt->isforeign)
- 					ereport(ERROR,
- 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- 					errmsg("constraints are not supported on foreign tables"),
- 							 parser_errposition(cxt->pstate,
- 												constraint->location)));
  				cxt->ckconstraints = lappend(cxt->ckconstraints, constraint);
  				break;
  
--- 515,520 ----
***************
*** 529,535 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
  				if (cxt->isforeign)
  					ereport(ERROR,
  							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					errmsg("constraints are not supported on foreign tables"),
  							 parser_errposition(cxt->pstate,
  												constraint->location)));
  				if (constraint->keys == NIL)
--- 523,529 ----
  				if (cxt->isforeign)
  					ereport(ERROR,
  							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					errmsg("unique or primary key constraints are not supported on foreign tables"),
  							 parser_errposition(cxt->pstate,
  												constraint->location)));
  				if (constraint->keys == NIL)
***************
*** 546,552 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
  				if (cxt->isforeign)
  					ereport(ERROR,
  							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					errmsg("constraints are not supported on foreign tables"),
  							 parser_errposition(cxt->pstate,
  												constraint->location)));
  
--- 540,546 ----
  				if (cxt->isforeign)
  					ereport(ERROR,
  							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					errmsg("foreign key constraints are not supported on foreign tables"),
  							 parser_errposition(cxt->pstate,
  												constraint->location)));
  
***************
*** 605,614 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
  static void
  transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
  {
! 	if (cxt->isforeign)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 				 errmsg("constraints are not supported on foreign tables"),
  				 parser_errposition(cxt->pstate,
  									constraint->location)));
  
--- 599,612 ----
  static void
  transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
  {
! 	if (cxt->isforeign &&
! 		(constraint->contype == CONSTR_PRIMARY ||
! 		 constraint->contype == CONSTR_UNIQUE ||
! 		 constraint->contype == CONSTR_EXCLUSION ||
! 		 constraint->contype == CONSTR_FOREIGN))
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 				 errmsg("unique, primary key, exclusion, or foreign key constraints are not supported on foreign tables"),
  				 parser_errposition(cxt->pstate,
  									constraint->location)));
  
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
***************
*** 670,676 **** LINE 1: CREATE FOREIGN TABLE ft1 () SERVER s0 WITH OIDS;
                                                ^
  CREATE FOREIGN TABLE ft1 (
  	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3'),
  	c3 date
  ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
  COMMENT ON FOREIGN TABLE ft1 IS 'ft1';
--- 670,682 ----
                                                ^
  CREATE FOREIGN TABLE ft1 (
  	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> '') NO INHERIT,
! 	c3 date
! ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value'); -- ERROR
! ERROR:  CHECK constraints on foreign tables cannot be marked NO INHERIT
! CREATE FOREIGN TABLE ft1 (
! 	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> ''),
  	c3 date
  ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
  COMMENT ON FOREIGN TABLE ft1 IS 'ft1';
***************
*** 682,687 **** COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
--- 688,695 ----
   c1     | integer | not null  | ("param 1" 'val1')             | plain    |              | ft1.c1
   c2     | text    |           | (param2 'val2', param3 'val3') | extended |              | 
   c3     | date    |           |                                | plain    |              | 
+ Check constraints:
+     "ft1_c2_check" CHECK (c2 <> ''::text)
  Server: s0
  FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
  
***************
*** 740,745 **** ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STATISTICS -1;
--- 748,755 ----
   c8     | text    |           | (p2 'V2')                      | extended |              | 
   c9     | integer |           |                                | plain    |              | 
   c10    | integer |           | (p1 'v1')                      | plain    |              | 
+ Check constraints:
+     "ft1_c2_check" CHECK (c2 <> ''::text)
  Server: s0
  FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
  
***************
*** 748,763 **** CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;	-- ERROR
  ERROR:  cannot alter foreign table "ft1" because column "use_ft1_column_type.x" uses its row type
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR
! ERROR:  constraints are not supported on foreign tables
! LINE 1: ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c...
!                                     ^
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! ERROR:  "ft1" is not a table
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check;
! ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
--- 758,775 ----
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;	-- ERROR
  ERROR:  cannot alter foreign table "ft1" because column "use_ft1_column_type.x" uses its row type
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0) NO INHERIT; -- ERROR
! ERROR:  CHECK constraints on foreign tables cannot be marked NO INHERIT
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0) NOT VALID;  -- ERROR
! ERROR:  CHECK constraints on foreign tables cannot be marked NOT VALID
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0);
! ALTER FOREIGN TABLE ft1 ALTER CONSTRAINT ft1_c9_check DEFERRABLE; -- ERROR
  ERROR:  "ft1" is not a table
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c9_check;
+ ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
+ ERROR:  constraint "no_const" of relation "ft1" does not exist
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! NOTICE:  constraint "no_const" of relation "ft1" does not exist, skipping
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
***************
*** 785,790 **** ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1;
--- 797,804 ----
   c7               | integer |           | (p1 'v1', p2 'v2')
   c8               | text    |           | (p2 'V2')
   c10              | integer |           | (p1 'v1')
+ Check constraints:
+     "ft1_c2_check" CHECK (c2 <> ''::text)
  Server: s0
  FDW Options: (quote '~', "be quoted" 'value', escape '@')
  
*** a/src/test/regress/sql/foreign_data.sql
--- b/src/test/regress/sql/foreign_data.sql
***************
*** 270,276 **** CREATE FOREIGN TABLE ft1 () SERVER no_server;                   -- ERROR
  CREATE FOREIGN TABLE ft1 () SERVER s0 WITH OIDS;                -- ERROR
  CREATE FOREIGN TABLE ft1 (
  	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3'),
  	c3 date
  ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
  COMMENT ON FOREIGN TABLE ft1 IS 'ft1';
--- 270,281 ----
  CREATE FOREIGN TABLE ft1 () SERVER s0 WITH OIDS;                -- ERROR
  CREATE FOREIGN TABLE ft1 (
  	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> '') NO INHERIT,
! 	c3 date
! ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value'); -- ERROR
! CREATE FOREIGN TABLE ft1 (
! 	c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
! 	c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> ''),
  	c3 date
  ) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
  COMMENT ON FOREIGN TABLE ft1 IS 'ft1';
***************
*** 314,323 **** ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STATISTICS -1;
  CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;	-- ERROR
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
- ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check;
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
  ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape '@');
--- 319,331 ----
  CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;	-- ERROR
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0) NO INHERIT; -- ERROR
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0) NOT VALID;  -- ERROR
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0);
! ALTER FOREIGN TABLE ft1 ALTER CONSTRAINT ft1_c9_check DEFERRABLE; -- ERROR
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c9_check;
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
  ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape '@');
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to