Re: per-column generic option

(2011/06/14 21:20), Robert Haas wrote:
> I haven't looked at the patch yet, but here are a few comments on the
> design, which overall looks good.

Thanks for the review. Please find attached a revised patch.

In addition to responding to your comments, I also added pg_dump
support. Now pg_dump dumps per-column generic options with ALTER
FOREIGN TABLE ALTER COLUMN statement just after CREATE FOREIGN TABLE.
Once I've though to dump them in each column definition of a CREATE
FOREIGN TABLE statement, but that seems to makes the statement too complex.

> 2011/6/14 Shigeru Hanada<shigeru(dot)hanada(at)gmail(dot)com>:
>> 1) psql should support describing per-column generic options, so \dec
>> command was added. If the form \dec+ is used, generic options are also
>> displayed. Output sample is:
>
> I would not add a new backslash command for this - it's unlikely to be
> useful to see this information across all tables. It would be more
> helpful to somehow (not sure of the details) incorporate this into the
> output of running \d on a foreign table.

Hm, belatedly I noticed that relation-kind-specific column are added
preceding to verbose-only columns such as expression for indexes and
column values for sequences. It seems suitable place to show per-column
generic options. Please see attached "desc_results.txt" as sample.

I also noticed that relation-kind-specific information are not mentioned
in any document (at least in the section of psql[1]), even about
existing ones such as sequence values and index definition. I also
added short brief of them to psql document.

BTW, while working around \d command, I noticed that we can avoid
variable width (# of columns) query result, which is used to fetch
column information, with using NULL as placeholder (and it has already
been used partially). I think that it would enhance maintainability
little, so I've separated this fix to another patch
avoid_variable_width_result.patch. The main patch
per_column_option_v2.patch assumes that this fix has been applied.

[1] http://developer.postgresql.org/pgdocs/postgres/app-psql.html

>> Here I found an inconsistency about privilege to see generic options
>> (not only column but also FDW and server et al). The
>> information_schema.*_options only shows options which are associated to
>> objects that current user can access, but \de*+ doesn't have such
>> restriction. \de* commands should be fixed to hide forbidden objects?
>
> It's less important whether \de* is consistent with information_schema
> in this regard than it is whether it is consistent with other psql
> backslash commands, e.g. \dv or \db or \dC. AFAIK those commands do
> not filter by privilege.

Agreed, I'll leave \de* to show results unconditionally.

>> 1) Is "generic options" proper term to mean FDW-specific option
>> associated to a FDW object? It's used in the SQL/MED standard, but
>> seems not popular... "FDW option" would be better than "generic option"?
>
> I think FDW option is much clearer.

So do I, but I didn't touch them because "generic option" appears in
many documents, source files including comments and psql's \d* output.
Most of them have been there since 8.4. Is it acceptable to change them
to "FDW option", at least for only documents?

OTOH, psql's \d* commands use "Options" as column header of FDW options
and reloptions. I also left them because I thought that this would not
cause misunderstanding.

Regards,
--
Shigeru Hanada