On 2021/04/06 16:05, Amit Langote wrote:
On Tue, Apr 6, 2021 at 8:34 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
For now I have no objection to this feature.

-IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch)
+IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch, t4_part)

Isn't it better to create also another partition like "t4_part2"?
If we do this, for example, the above test can confirm that both
partitions in EXCEPT and not in are excluded.

+    All tables or foreign tables which are partitions of some other table
+    are automatically excluded from <xref linkend="sql-importforeignschema"/>
+    unless they are explicitly included in the <literal>LIMIT TO</literal>

IMO it's better to document that partitions are imported when they are
included in LIMIT TO, instead. What about the following?

      Tables or foreign tables which are partitions of some other table are
      imported only when they are explicitly specified in
      <literal>LIMIT TO</literal> clause.  Otherwise they are automatically
      excluded from <xref linkend="sql-importforeignschema"/>.

+    clause.  Since all data can be accessed through the partitioned table
+    which is the root of the partitioning hierarchy, this approach should
+    allow access to all the data without creating extra objects.

Now "this approach" in the above is not clear? What about replacing it with
something like "importing only partitioned tables"?

+1, that wording is better.

Thanks! So I applied all the changes that I suggested upthread to the patch.
I also updated the comment as follows.

                 * Import table data for partitions only when they are 
explicitly
-                * specified in LIMIT TO clause. Otherwise ignore them and
-                * only include the definitions of the root partitioned tables 
to
-                * allow access to the complete remote data set locally in
-                * the schema imported.
+                * specified in LIMIT TO clause. Otherwise ignore them and only
+                * include the definitions of the root partitioned tables to 
allow
+                * access to the complete remote data set locally in the schema
+                * imported.

Attached is the updated version of the patch. Barring any objection,
I'm thinking to commit this.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out 
b/contrib/postgres_fdw/expected/postgres_fdw.out
index 59e4e27ffb..9d472d2d3d 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8228,6 +8228,8 @@ ALTER TABLE import_source."x 5" DROP COLUMN c1;
 CREATE TABLE import_source.t4 (c1 int) PARTITION BY RANGE (c1);
 CREATE TABLE import_source.t4_part PARTITION OF import_source.t4
   FOR VALUES FROM (1) TO (100);
+CREATE TABLE import_source.t4_part2 PARTITION OF import_source.t4
+  FOR VALUES FROM (100) TO (200);
 CREATE SCHEMA import_dest1;
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest1;
 \det+ import_dest1.*
@@ -8419,27 +8421,29 @@ FDW options: (schema_name 'import_source', table_name 
'x 5')
 
 -- Check LIMIT TO and EXCEPT
 CREATE SCHEMA import_dest4;
-IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch)
+IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch, t4_part)
   FROM SERVER loopback INTO import_dest4;
 \det+ import_dest4.*
-                                     List of foreign tables
-    Schema    | Table |  Server  |                  FDW options                
   | Description 
---------------+-------+----------+------------------------------------------------+-------------
- import_dest4 | t1    | loopback | (schema_name 'import_source', table_name 
't1') | 
-(1 row)
+                                        List of foreign tables
+    Schema    |  Table  |  Server  |                     FDW options           
          | Description 
+--------------+---------+----------+-----------------------------------------------------+-------------
+ import_dest4 | t1      | loopback | (schema_name 'import_source', table_name 
't1')      | 
+ import_dest4 | t4_part | loopback | (schema_name 'import_source', table_name 
't4_part') | 
+(2 rows)
 
-IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch)
+IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch, t4_part)
   FROM SERVER loopback INTO import_dest4;
 \det+ import_dest4.*
-                                     List of foreign tables
-    Schema    | Table |  Server  |                   FDW options               
    | Description 
---------------+-------+----------+-------------------------------------------------+-------------
- import_dest4 | t1    | loopback | (schema_name 'import_source', table_name 
't1')  | 
- import_dest4 | t2    | loopback | (schema_name 'import_source', table_name 
't2')  | 
- import_dest4 | t3    | loopback | (schema_name 'import_source', table_name 
't3')  | 
- import_dest4 | t4    | loopback | (schema_name 'import_source', table_name 
't4')  | 
- import_dest4 | x 5   | loopback | (schema_name 'import_source', table_name 'x 
5') | 
-(5 rows)
+                                        List of foreign tables
+    Schema    |  Table  |  Server  |                     FDW options           
          | Description 
+--------------+---------+----------+-----------------------------------------------------+-------------
+ import_dest4 | t1      | loopback | (schema_name 'import_source', table_name 
't1')      | 
+ import_dest4 | t2      | loopback | (schema_name 'import_source', table_name 
't2')      | 
+ import_dest4 | t3      | loopback | (schema_name 'import_source', table_name 
't3')      | 
+ import_dest4 | t4      | loopback | (schema_name 'import_source', table_name 
't4')      | 
+ import_dest4 | t4_part | loopback | (schema_name 'import_source', table_name 
't4_part') | 
+ import_dest4 | x 5     | loopback | (schema_name 'import_source', table_name 
'x 5')     | 
+(6 rows)
 
 -- Assorted error cases
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest4;
diff --git a/contrib/postgres_fdw/postgres_fdw.c 
b/contrib/postgres_fdw/postgres_fdw.c
index 16c2979f2d..b6442070a3 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -5095,9 +5095,11 @@ postgresImportForeignSchema(ImportForeignSchemaStmt 
*stmt, Oid serverOid)
                 * should save a few cycles to not process excluded tables in 
the
                 * first place.)
                 *
-                * Ignore table data for partitions and only include the 
definitions
-                * of the root partitioned tables to allow access to the 
complete
-                * remote data set locally in the schema imported.
+                * Import table data for partitions only when they are 
explicitly
+                * specified in LIMIT TO clause. Otherwise ignore them and only
+                * include the definitions of the root partitioned tables to 
allow
+                * access to the complete remote data set locally in the schema
+                * imported.
                 *
                 * Note: because we run the connection with search_path 
restricted to
                 * pg_catalog, the format_type() and pg_get_expr() outputs will 
always
@@ -5153,7 +5155,8 @@ postgresImportForeignSchema(ImportForeignSchemaStmt 
*stmt, Oid serverOid)
                deparseStringLiteral(&buf, stmt->remote_schema);
 
                /* Partitions are supported since Postgres 10 */
-               if (PQserverVersion(conn) >= 100000)
+               if (PQserverVersion(conn) >= 100000 &&
+                       stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO)
                        appendStringInfoString(&buf, " AND NOT c.relispartition 
");
 
                /* Apply restrictions for LIMIT TO and EXCEPT */
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql 
b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 107d1c0e03..3b4f90a99c 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2366,6 +2366,8 @@ ALTER TABLE import_source."x 5" DROP COLUMN c1;
 CREATE TABLE import_source.t4 (c1 int) PARTITION BY RANGE (c1);
 CREATE TABLE import_source.t4_part PARTITION OF import_source.t4
   FOR VALUES FROM (1) TO (100);
+CREATE TABLE import_source.t4_part2 PARTITION OF import_source.t4
+  FOR VALUES FROM (100) TO (200);
 
 CREATE SCHEMA import_dest1;
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest1;
@@ -2386,10 +2388,10 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER 
loopback INTO import_dest3
 
 -- Check LIMIT TO and EXCEPT
 CREATE SCHEMA import_dest4;
-IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch)
+IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch, t4_part)
   FROM SERVER loopback INTO import_dest4;
 \det+ import_dest4.*
-IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch)
+IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch, t4_part)
   FROM SERVER loopback INTO import_dest4;
 \det+ import_dest4.*
 
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index a7c695b000..b0792a13b1 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -510,10 +510,12 @@ OPTIONS (ADD password_required 'false');
 
    <para>
     Tables or foreign tables which are partitions of some other table are
-    automatically excluded.  Partitioned tables are imported, unless they
-    are a partition of some other table.  Since all data can be accessed
-    through the partitioned table which is the root of the partitioning
-    hierarchy, this approach should allow access to all the data without
+    imported only when they are explicitly specified in
+    <literal>LIMIT TO</literal> clause.  Otherwise they are automatically
+    excluded from <xref linkend="sql-importforeignschema"/>.
+    Since all data can be accessed through the partitioned table
+    which is the root of the partitioning hierarchy, importing only
+    partitioned tables should allow access to all the data without
     creating extra objects.
    </para>
 

Reply via email to