Thread: psql document fix about showing FDW options

psql document fix about showing FDW options

From
Shigeru Hanada
Date:
I noticed that psql document wrongly says that \d+ command shows
per-table FDW options of a foreign table, but in fact, per-table FDW
options are shown only in the result of \det+ command.  Attached patch
removes this wrong description.

This fix should be applied to 9.1 too.

Regards,
--
Shigeru Hanada

Attachment

Re: psql document fix about showing FDW options

From
Robert Haas
Date:
2011/8/8 Shigeru Hanada <shigeru.hanada@gmail.com>:
> I noticed that psql document wrongly says that \d+ command shows
> per-table FDW options of a foreign table, but in fact, per-table FDW
> options are shown only in the result of \det+ command.  Attached patch
> removes this wrong description.
>
> This fix should be applied to 9.1 too.

I think we ought to fix the behavior, rather than the documentation.
It's just weird to have something show up in the list view that
doesn't appear in the detail view.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: psql document fix about showing FDW options

From
Shigeru Hanada
Date:
(2011/08/09 1:20), Robert Haas wrote:
> 2011/8/8 Shigeru Hanada<shigeru.hanada@gmail.com>:
>> I noticed that psql document wrongly says that \d+ command shows
>> per-table FDW options of a foreign table, but in fact, per-table FDW
>> options are shown only in the result of \det+ command.  Attached patch
>> removes this wrong description.
>>
>> This fix should be applied to 9.1 too.
>
> I think we ought to fix the behavior, rather than the documentation.
> It's just weird to have something show up in the list view that
> doesn't appear in the detail view.

Agreed, but you might have misunderstand behavior of \d* commands,
because currently per-table options aren't shown in detail view of \d
command.  Please let me clarify the specs about FDW options in backslash
commands.

Current head shows FDW options in the result of:

      command    | shown FDW options
   --------------+-------------------
    \d (list)    | none
    \d+ (list)   | none
    \d (detail)  | per-column
    \d+ (detail) | per-column
    \det         | none
    \det+        | per-table

Note that \det doesn't have detail view.

I think showing per-table FDW options in footer of \d (not only \d+) is
reasonable because it shows per-column FDW options too.  Please see a
sample below.

postgres=# \d pgbench_accounts
       Foreign table "public.pgbench_accounts"
  Column  |     Type      | Modifiers |    Options
----------+---------------+-----------+---------------
 aid      | integer       | not null  | {colname=aid}
 bid      | integer       |           |
 abalance | integer       |           |
 filler   | character(84) |           |
Server: pgbench
Options: {nspname=public,"relname=foo bar"}

It seems fine ... but here I recall that the header "Options" is also
used for reloptions.  So it might be worth changing those headers to
"FDW Options" to avoid future conflicts.  If we do so, we would also
have to change existing headers for FDW/server/user mapping as well for
consistency, but it breaks backward compatibility.  Thoughts?

I attached WIP patch for these fixes.
Please apply show_per_table_options.patch first against head, and then
rename_desc_headers.patch can be applied.

Regards,
--
Shigeru Hanada

Attachment

Re: psql document fix about showing FDW options

From
Robert Haas
Date:
2011/8/9 Shigeru Hanada <shigeru.hanada@gmail.com>:
> postgres=# \d pgbench_accounts
>       Foreign table "public.pgbench_accounts"
>  Column  |     Type      | Modifiers |    Options
> ----------+---------------+-----------+---------------
>  aid      | integer       | not null  | {colname=aid}
>  bid      | integer       |           |
>  abalance | integer       |           |
>  filler   | character(84) |           |
> Server: pgbench
> Options: {nspname=public,"relname=foo bar"}

Looks like you've got postgresql_fdw working there... do you plan to
submit that for 9.2 soon?  Any plans for qual pushdown?

> I attached WIP patch for these fixes.
> Please apply show_per_table_options.patch first against head, and then
> rename_desc_headers.patch can be applied.

Looks good to me.  Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: psql document fix about showing FDW options

From
Shigeru Hanada
Date:
(2011/08/12 0:48), Robert Haas wrote:
> 2011/8/9 Shigeru Hanada<shigeru.hanada@gmail.com>:
>> postgres=# \d pgbench_accounts
>>        Foreign table "public.pgbench_accounts"
>>   Column  |     Type      | Modifiers |    Options
>> ----------+---------------+-----------+---------------
>>   aid      | integer       | not null  | {colname=aid}
>>   bid      | integer       |           |
>>   abalance | integer       |           |
>>   filler   | character(84) |           |
>> Server: pgbench
>> Options: {nspname=public,"relname=foo bar"}
> 
> Looks like you've got postgresql_fdw working there... do you plan to
> submit that for 9.2 soon?  Any plans for qual pushdown?

Yeah, I have (hopefully) working FDW for PostgreSQL which is based on
the one which has been proposed for 9.1, and updates done by Heikki.
I've implemented:
 * SELECT clause omitting * WHERE clause pushdown (assuming remote has same functions/oprators) * private connection
caching,and VIEW which shows active connections * switch simple SELECT and CURSOR based on estimated # of rows,   and
FDWoptions to control this behavior
 

I'd like to submit this fdw in CF 2011-09 (ASAP) for discussion of
itself and FDW API enhancement.  Please see the wiki page below for details.

http://wiki.postgresql.org/wiki/SQL/MED#PostgreSQL

>> I attached WIP patch for these fixes.
>> Please apply show_per_table_options.patch first against head, and then
>> rename_desc_headers.patch can be applied.
> 
> Looks good to me.  Committed.

Thanks!

Regards,
-- 
Shigeru Hanada


Re: psql document fix about showing FDW options

From
Robert Haas
Date:
2011/8/11 Shigeru Hanada <shigeru.hanada@gmail.com>:
> Yeah, I have (hopefully) working FDW for PostgreSQL which is based on
> the one which has been proposed for 9.1, and updates done by Heikki.
> I've implemented:
>
>  * SELECT clause omitting
>  * WHERE clause pushdown (assuming remote has same functions/oprators)
>  * private connection caching, and VIEW which shows active connections
>  * switch simple SELECT and CURSOR based on estimated # of rows,
>    and FDW options to control this behavior
>
> I'd like to submit this fdw in CF 2011-09 (ASAP) for discussion of
> itself and FDW API enhancement.  Please see the wiki page below for details.

Great stuff.  Look forward to seeing the patch(es).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company