(2011/12/09 21:16), Etsuro Fujita wrote:
> I updated the patch.  Please find attached a patch.

I've examined v5 patch, and got reasonable EXPLAIN results which reflect
collected statistics!  As increasing STATISTICS option, estimated rows
become better.  Please see attached stats_*.txt for what I
tested.

  stats_none.txt  : before ANALYZE
  stats_100.txt   : SET STATISTICS = 100 for all columns, and ANALYZE
  stats_10000.txt : SET STATISTICS = 10000 for all columns, and ANALYZE

I think that this patch become ready for committer after some
minor corrections:

Couldn't set n_distinct
=======================
I couldn't set n_distinct to columns of foreign tables.  With some
research, I noticed that ATSimplePermissions should accept
ATT_FOREIGN_TABLE too for that case.  In addition, regression tests for
ALTER FOREIGN TABLE should be added to detect this kind of problem.

Showing stats target
====================
We can see stats target of ordinary tables with \d+, but it is not
available for foreign tables.  I think "Stats target" column should be
added even though output of \d+ for foreign tables become wider.  One
possible idea is to remove useless "Storage" column instead, but views
have that column even though their source could come from foreign tables.

Please see attached patch for these two items.

Comments of FdwRoutine
======================
Mention about optional handler is obsolete.  We should clearly say
AnalyzeForeignTable is optional (can be set to NULL) and rest are
required.  IMO separating them with comment would help FDW authors to
understand requirements, e.g.:

typedef struct FdwRoutine
{
    NodeTag     type;

    /*
     * These Handlers are required to execute simple scan on a foreign
     * table.  If any of them was set to NULL, scan on a foreign table
     * managed by such FDW would fail.
     */
    PlanForeignScan_function PlanForeignScan;
    ExplainForeignScan_function ExplainForeignScan;
    BeginForeignScan_function BeginForeignScan;
    IterateForeignScan_function IterateForeignScan;
    ReScanForeignScan_function ReScanForeignScan;
    EndForeignScan_function EndForeignScan;

    /*
     * Handlers below are optional.  You can set any of them to
     * NULL to tell PostgreSQL that the FDW doesn't have the
     * capability.
     */
    AnalyzeForeignTable_function AnalyzeForeignTable;
} FdwRoutine;

Code formatting
===============
Some code lines go past 80 columns.

Message style
=============
The terms 'cannot support option "n_distinct"...' used in
ATPrepSetOptions seems little unusual in PostgreSQL.  Should we say
'cannot set "n_distinct_inherited" for foreign tables' for that case?

Typo
====
Typo "spcify" is in document of analyze.

Regards,
-- 
Shigeru Hanada

postgres=# explain select * from csv_accounts where aid = 1;
                               QUERY PLAN
-------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (aid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid < 1000;
                                QUERY PLAN
---------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=124904 width=100)
   Filter: (aid < 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid > 1000;
                                QUERY PLAN
---------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=124904 width=100)
   Filter: (aid > 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 1;
                               QUERY PLAN
-------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (bid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 2;
                               QUERY PLAN
-------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (bid = 2)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 4;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=102117 width=97)
   Filter: (bid = 4)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 6;
                             QUERY PLAN
---------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1 width=97)
   Filter: (bid = 6)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)
postgres=# explain select * from csv_accounts where aid = 1;
                             QUERY PLAN
---------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1 width=97)
   Filter: (aid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid < 1000;
                              QUERY PLAN
-----------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=943 width=97)
   Filter: (aid < 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid > 1000;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=499057 width=97)
   Filter: (aid > 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 1;
                               QUERY PLAN
-------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=98383 width=97)
   Filter: (bid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 2;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=101200 width=97)
   Filter: (bid = 2)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 4;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=102117 width=97)
   Filter: (bid = 4)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 6;
                             QUERY PLAN
---------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1 width=97)
   Filter: (bid = 6)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid = 1;
                             QUERY PLAN
---------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1 width=97)
   Filter: (aid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid < 1000;
                               QUERY PLAN
------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1000 width=97)
   Filter: (aid < 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid > 1000;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=499000 width=97)
   Filter: (aid > 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 1;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=100000 width=97)
   Filter: (bid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 2;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=100000 width=97)
   Filter: (bid = 2)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 4;
                                QUERY PLAN
--------------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=100000 width=97)
   Filter: (bid = 4)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 6;
                             QUERY PLAN
---------------------------------------------------------------------
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=1 width=97)
   Filter: (bid = 6)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

commit b056c0cc38a9460c083741bc021a9b5eddee22f1
Author: Shigeru Hanada <shigeru.han...@gmail.com>
Date:   Mon Dec 12 18:14:26 2011 +0900

    Fix psql to show stats target for foreign tables too.
    
    Regression tests are also added for this change, and one simple bug
    is detected and fixed.

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5db476b..6dc736d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2917,7 +2917,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        break;
                case AT_SetOptions:             /* ALTER COLUMN SET ( options ) 
*/
                case AT_ResetOptions:   /* ALTER COLUMN RESET ( options ) */
-                       ATSimplePermissions(rel, ATT_TABLE | ATT_INDEX);
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_INDEX | 
ATT_FOREIGN_TABLE);
                        ATPrepSetOptions(rel, cmd->name, cmd->def, lockmode);
                        /* This command never recurses */
                        pass = AT_PASS_MISC;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index dcafdd2..802abf2 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1099,7 +1099,7 @@ describeOneTableDetails(const char *schemaname,
        bool            printTableInitialized = false;
        int                     i;
        char       *view_def = NULL;
-       char       *headers[6];
+       char       *headers[7];
        char      **seq_values = NULL;
        char      **modifiers = NULL;
        char      **ptr;
@@ -1390,7 +1390,7 @@ describeOneTableDetails(const char *schemaname,
        if (verbose)
        {
                headers[cols++] = gettext_noop("Storage");
-               if (tableinfo.relkind == 'r')
+               if (tableinfo.relkind == 'r' || tableinfo.relkind == 'f')
                        headers[cols++] = gettext_noop("Stats target");
                /* Column comments, if the relkind supports this feature. */
                if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
@@ -1493,7 +1493,7 @@ describeOneTableDetails(const char *schemaname,
                                                          false, false);
 
                        /* Statistics target, if the relkind supports this 
feature */
-                       if (tableinfo.relkind == 'r')
+                       if (tableinfo.relkind == 'r' || tableinfo.relkind == 
'f')
                        {
                                printTableAddCell(&cont, PQgetvalue(res, i, 
firstvcol + 1),
                                                                  false, false);
diff --git a/src/test/regress/expected/foreign_data.out 
b/src/test/regress/expected/foreign_data.out
index 122e285..4a16238 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -678,12 +678,12 @@ CREATE FOREIGN TABLE ft1 (
 COMMENT ON FOREIGN TABLE ft1 IS 'ft1';
 COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
 \d+ ft1
-                               Foreign table "public.ft1"
- Column |  Type   | Modifiers |          FDW Options           | Storage  | 
Description 
---------+---------+-----------+--------------------------------+----------+-------------
- c1     | integer | not null  | ("param 1" 'val1')             | plain    | 
ft1.c1
- c2     | text    |           | (param2 'val2', param3 'val3') | extended | 
- c3     | date    |           |                                | plain    | 
+                                      Foreign table "public.ft1"
+ Column |  Type   | Modifiers |          FDW Options           | Storage  | 
Stats target | Description 
+--------+---------+-----------+--------------------------------+----------+--------------+-------------
+ c1     | integer | not null  | ("param 1" 'val1')             | plain    |    
          | ft1.c1
+ c2     | text    |           | (param2 'val2', param3 'val3') | extended |    
          | 
+ c3     | date    |           |                                | plain    |    
          | 
 Server: s0
 FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
 Has OIDs: no
@@ -729,19 +729,24 @@ ERROR:  cannot alter system column "xmin"
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 OPTIONS (ADD p1 'v1', ADD p2 'v2'),
                         ALTER COLUMN c8 OPTIONS (ADD p1 'v1', ADD p2 'v2');
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 OPTIONS (SET p2 'V2', DROP p1);
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET STATISTICS 10000;
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET (n_distinct = 100);
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET (n_distinct_inherited = 100); -- 
ERROR
+ERROR:  cannot set "n_distinct_inherited" on foreign tables
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STATISTICS -1;
 \d+ ft1
-                               Foreign table "public.ft1"
- Column |  Type   | Modifiers |          FDW Options           | Storage  | 
Description 
---------+---------+-----------+--------------------------------+----------+-------------
- c1     | integer | not null  | ("param 1" 'val1')             | plain    | 
- c2     | text    |           | (param2 'val2', param3 'val3') | extended | 
- c3     | date    |           |                                | plain    | 
- c4     | integer |           |                                | plain    | 
- c6     | integer | not null  |                                | plain    | 
- c7     | integer |           | (p1 'v1', p2 'v2')             | plain    | 
- c8     | text    |           | (p2 'V2')                      | extended | 
- c9     | integer |           |                                | plain    | 
- c10    | integer |           | (p1 'v1')                      | plain    | 
+                                      Foreign table "public.ft1"
+ Column |  Type   | Modifiers |          FDW Options           | Storage  | 
Stats target | Description 
+--------+---------+-----------+--------------------------------+----------+--------------+-------------
+ c1     | integer | not null  | ("param 1" 'val1')             | plain    | 
10000        | 
+ c2     | text    |           | (param2 'val2', param3 'val3') | extended |    
          | 
+ c3     | date    |           |                                | plain    |    
          | 
+ c4     | integer |           |                                | plain    |    
          | 
+ c6     | integer | not null  |                                | plain    |    
          | 
+ c7     | integer |           | (p1 'v1', p2 'v2')             | plain    |    
          | 
+ c8     | text    |           | (p2 'V2')                      | extended |    
          | 
+ c9     | integer |           |                                | plain    |    
          | 
+ c10    | integer |           | (p1 'v1')                      | plain    |    
          | 
 Server: s0
 FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
 Has OIDs: no
diff --git a/src/test/regress/sql/foreign_data.sql 
b/src/test/regress/sql/foreign_data.sql
index e99e707..5908ff3 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -306,6 +306,10 @@ ALTER FOREIGN TABLE ft1 ALTER COLUMN xmin OPTIONS (ADD p1 
'v1'); -- ERROR
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 OPTIONS (ADD p1 'v1', ADD p2 'v2'),
                         ALTER COLUMN c8 OPTIONS (ADD p1 'v1', ADD p2 'v2');
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 OPTIONS (SET p2 'V2', DROP p1);
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET STATISTICS 10000;
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET (n_distinct = 100);
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 SET (n_distinct_inherited = 100); -- 
ERROR
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STATISTICS -1;
 \d+ ft1
 -- can't change the column type if it's used elsewhere
 CREATE TABLE use_ft1_column_type (x ft1);
-- 
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