psql document fix about showing FDW options

Lists: pgsql-hackers
From: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
To: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: psql document fix about showing FDW options
Date: 2011-08-08 05:51:52
Message-ID: 4E3F7978.8050901@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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 Content-Type Size
fix_psql_doc.patch text/plain 972 bytes

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Cc: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql document fix about showing FDW options
Date: 2011-08-08 16:20:14
Message-ID: CA+Tgmoan5Bgo_yNezrL_KZxJGCwjyVeieGWZNnxUUq4oCbgWMA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/8/8 Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)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


From: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql document fix about showing FDW options
Date: 2011-08-09 10:19:13
Message-ID: 4E4109A1.5030601@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

(2011/08/09 1:20), Robert Haas wrote:
> 2011/8/8 Shigeru Hanada<shigeru(dot)hanada(at)gmail(dot)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 Content-Type Size
show_per_table_options.patch text/plain 4.3 KB
rename_desc_headers.patch text/plain 36.8 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Cc: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql document fix about showing FDW options
Date: 2011-08-11 15:48:13
Message-ID: CA+TgmoaCQZJx87nh93ngYMXULfJcxt2b83Ed8=uTGMok83kjEA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/8/9 Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)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


From: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql document fix about showing FDW options
Date: 2011-08-12 02:28:33
Message-ID: 4E448FD1.4020409@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

(2011/08/12 0:48), Robert Haas wrote:
> 2011/8/9 Shigeru Hanada<shigeru(dot)hanada(at)gmail(dot)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 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.

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


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Cc: "pgsql-hackers >> PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql document fix about showing FDW options
Date: 2011-08-12 03:01:14
Message-ID: CA+TgmoaAyYO67xYKFy_c6ExP8_TqCC0tbPd06_rtUXAdYhKHGw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/8/11 Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)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