On 10/15/2013 07:50 AM, David Fetter wrote:
> On Mon, Oct 07, 2013 at 11:16:56PM -0700, David Fetter wrote:
>> Folks,
>>
>> Please find attached a patch implementing and documenting, to some
>> extent, $subject. I did this in aid of being able to import SQL
>> standard catalogs and other entities where a known example could
>> provide a template for a foreign table.
>>
>> Should there be errhint()s, too? Should we pile up all such errors
>> and mention them at the end rather than simply bailing on the first
>> one?
>>
>> TBD: regression tests.
> Now included: regression tests for disallowed LIKE options.
I like this patch, but I don't like its implementation at all.
First of all, the documentation doesn't compile:
openjade:ref/create_foreign_table.sgml:124:17:E: end tag for "LISTITEM"
omitted, but OMITTAG NO was specified
openjade:ref/create_foreign_table.sgml:119:4: start tag was here
I fixed that, and then noticed that like_option is not explained like it
is in CREATE TABLE.
Then I got down to the description of the LIKE clause in both pages, and
I noticed the last line of CREATE TABLE, which is "Inapplicable options
(e.g., INCLUDING INDEXES from a view) are ignored.". This is
inconsistent with the behavior of this patch to throw errors for
inapplicable options.
Attached is a patch which corrects and completes the documentation
issues noted above, and also silently ignores inapplicable options. In
addition to reducing patch size, this also allows the use of INCLUDING
ALL. Because these options no longer produce errors, and that's all the
regression tests were looking for, I have removed those tests
(unfortunately leaving none).
Aside from this difference in behavior, I see no fault in this patch.
I am marking this patch as 'returned with feedback' in the commitfest app.
--
Vik
*** a/doc/src/sgml/ref/create_foreign_table.sgml
--- b/doc/src/sgml/ref/create_foreign_table.sgml
***************
*** 20,25 ****
--- 20,26 ----
<synopsis>
CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name</replaceable> ( [
<replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ OPTIONS ( <replaceable class="PARAMETER">option</replaceable> '<replaceable class="PARAMETER">value</replaceable>' [, ... ] ) ] [ COLLATE <replaceable>collation</replaceable> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
+ | LIKE <replaceable>source_table</replaceable> [ <replaceable>like_option</replaceable> ... ] }
[, ... ]
] )
SERVER <replaceable class="parameter">server_name</replaceable>
***************
*** 31,36 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
--- 32,39 ----
{ NOT NULL |
NULL |
DEFAULT <replaceable>default_expr</replaceable> }
+
+ <phrase>and <replaceable class="PARAMETER">like_option</replaceable> is the same as for <xref linkend="SQL-CREATETABLE">.</phrase>
</synopsis>
</refsynopsisdiv>
***************
*** 114,119 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
--- 117,135 ----
</varlistentry>
<varlistentry>
+ <term><literal>LIKE <replaceable>source_table</replaceable> [ <replaceable>like_option</replaceable> ... ]</literal></term>
+ <listitem>
+ <para>
+ The <literal>LIKE</literal> clause specifies a table from which
+ the new foreign table automatically copies all column names and their data types.
+ </para>
+ <para>
+ Inapplicable options (e.g., <literal>INCLUDING STORAGE</literal>) are ignored.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 649,655 **** transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
/*
* transformTableLikeClause
*
! * Change the LIKE <srctable> portion of a CREATE TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
--- 649,655 ----
/*
* transformTableLikeClause
*
! * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
***************
*** 668,679 **** transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
setup_parser_errposition_callback(&pcbstate, cxt->pstate,
table_like_clause->relation->location);
- /* we could support LIKE in many cases, but worry about it another day */
- if (cxt->isforeign)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("LIKE is not supported for creating foreign tables")));
-
relation = relation_openrv(table_like_clause->relation, AccessShareLock);
if (relation->rd_rel->relkind != RELKIND_RELATION &&
--- 668,673 ----
***************
*** 688,693 **** transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
--- 682,691 ----
cancel_parser_errposition_callback(&pcbstate);
+ /* For foreign tables, ignore all but applicable options. */
+ if (cxt->isforeign)
+ table_like_clause->options &= CREATE_TABLE_LIKE_DEFAULTS | CREATE_TABLE_LIKE_COMMENTS;
+
/*
* Check for privileges
*/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
