CREATE FOREIGN TABLE ( ... LIKE ... )
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.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
foreign_table_like_05.difftext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..4a8e265 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -20,6 +20,7 @@
<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>
@@ -114,6 +115,15 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
</varlistentry>
<varlistentry>
+ <term><literal>LIKE <replaceable>source_table</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>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 19d19e5f..152fa01 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,7 +649,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
/*
* transformTableLikeClause
*
- * Change the LIKE <srctable> portion of a CREATE TABLE statement into
+ * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
@@ -669,10 +669,12 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
table_like_clause->relation->location);
/* we could support LIKE in many cases, but worry about it another day */
+ /* Let's see whether just dropping this enables LIKE :)
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);
@@ -689,6 +691,25 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
cancel_parser_errposition_callback(&pcbstate);
/*
+ * For foreign tables, disallow some options.
+ */
+ if (cxt->isforeign)
+ {
+ if (table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING CONSTRAINTS")));
+ else if (table_like_clause->options & CREATE_TABLE_LIKE_INDEXES)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING INDEXES")));
+ else if (table_like_clause->options & CREATE_TABLE_LIKE_STORAGE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING STORAGE")));
+ }
+
+ /*
* Check for privileges
*/
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
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.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
foreign_table_like_06.difftext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..4a8e265 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -20,6 +20,7 @@
<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>
@@ -114,6 +115,15 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
</varlistentry>
<varlistentry>
+ <term><literal>LIKE <replaceable>source_table</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>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 19d19e5f..219c910 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,7 +649,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
/*
* transformTableLikeClause
*
- * Change the LIKE <srctable> portion of a CREATE TABLE statement into
+ * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
@@ -668,12 +668,6 @@ 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 &&
@@ -689,6 +683,25 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
cancel_parser_errposition_callback(&pcbstate);
/*
+ * For foreign tables, disallow some options.
+ */
+ if (cxt->isforeign)
+ {
+ if (table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING CONSTRAINTS")));
+ else if (table_like_clause->options & CREATE_TABLE_LIKE_INDEXES)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING INDEXES")));
+ else if (table_like_clause->options & CREATE_TABLE_LIKE_STORAGE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ERROR: foreign tables do not support LIKE INCLUDING STORAGE")));
+ }
+
+ /*
* Check for privileges
*/
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 60506e0..84f5265 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -699,6 +699,21 @@ SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
EXPLAIN SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING CONSTRAINTS
+) SERVER s0; -- ERROR
+ERROR: ERROR: foreign tables do not support LIKE INCLUDING CONSTRAINTS
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING INDEXES
+) SERVER s0; -- ERROR
+ERROR: ERROR: foreign tables do not support LIKE INCLUDING INDEXES
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING STORAGE
+) SERVER s0; -- ERROR
+ERROR: ERROR: foreign tables do not support LIKE INCLUDING STORAGE
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
COMMENT ON FOREIGN TABLE ft1 IS NULL;
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index f819eb1..f36a10c 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -280,6 +280,18 @@ COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
CREATE INDEX id_ft1_c2 ON ft1 (c2); -- ERROR
SELECT * FROM ft1; -- ERROR
EXPLAIN SELECT * FROM ft1; -- ERROR
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING CONSTRAINTS
+) SERVER s0; -- ERROR
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING INDEXES
+) SERVER s0; -- ERROR
+CREATE FOREIGN TABLE ft2 (
+ LIKE pg_catalog.pg_enum
+ INCLUDING STORAGE
+) SERVER s0; -- ERROR
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
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
Attachments:
foreign_table_like_20131124.patchtext/x-diff; name=foreign_table_like_20131124.patchDownload
*** 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
*/
On 11/24/2013 02:03 AM, Vik Fearing wrote:
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 hereI 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.
It looks like this patch got left behind in the previous commitfest.
What is the policy for moving it? Is it too late and will have to wait
until the next commitfest?
https://commitfest.postgresql.org/action/patch_view?id=1254
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jan 16, 2014 at 01:07:50AM +0100, Vik Fearing wrote:
On 11/24/2013 02:03 AM, Vik Fearing wrote:
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 hereI 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.
It looks like this patch got left behind in the previous commitfest.
What is the policy for moving it? Is it too late and will have to wait
until the next commitfest?
I think we should still be OK putting it in the current one.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Vik Fearing <vik.fearing@dalibo.com> writes:
I am marking this patch as 'returned with feedback' in the commitfest app.
It looks like this patch got left behind in the previous commitfest.
What is the policy for moving it? Is it too late and will have to wait
until the next commitfest?
I think you were in error to mark it "returned with feedback", as that
caused everyone to stop paying attention to it in that commitfest.
(And David dropped the ball too, as he should have done something to
bring it back from that state, if it was committable or nearly so.)
I see no reason why you shouldn't move it to the new fest; perhaps
mark it as waiting on author, since really it's his responsibility
to take the next step, ie comment on your version of the patch.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/16/2014 01:21 AM, Tom Lane wrote:
Vik Fearing <vik.fearing@dalibo.com> writes:
I am marking this patch as 'returned with feedback' in the commitfest app.
It looks like this patch got left behind in the previous commitfest.
What is the policy for moving it? Is it too late and will have to wait
until the next commitfest?
https://commitfest.postgresql.org/action/patch_view?id=1254I think you were in error to mark it "returned with feedback", as that
caused everyone to stop paying attention to it in that commitfest.
(And David dropped the ball too, as he should have done something to
bring it back from that state, if it was committable or nearly so.)
I see. Sorry about that.
I see no reason why you shouldn't move it to the new fest; perhaps
mark it as waiting on author, since really it's his responsibility
to take the next step, ie comment on your version of the patch.
Done.
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Nov 24, 2013 at 02:03:18AM +0100, Vik Fearing wrote:
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
Fixed.
I fixed that, and then noticed that like_option is not explained like it
is in CREATE TABLE.
Also fixed.
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.
Fixed.
Please find attached the next rev :)
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
foreign_table_like_07.difftext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..375bd1a 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -20,6 +20,7 @@
<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,6 +32,8 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
{ 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,6 +117,19 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
</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 like <literal>INCLUDING STORAGE</literal> are ignored.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index eb07ca3..82c77eb 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,7 +649,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
/*
* transformTableLikeClause
*
- * Change the LIKE <srctable> portion of a CREATE TABLE statement into
+ * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
@@ -668,12 +668,6 @@ 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 &&
@@ -689,6 +683,12 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
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
*/
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
On 01/25/2014 06:25 AM, David Fetter wrote:
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 hereFixed.
I fixed that, and then noticed that like_option is not explained like it
is in CREATE TABLE.Also fixed.
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.Fixed.
Please find attached the next rev :)
This version looks committable to me, so I am marking it as such.
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-01-31 18:16:18 +0100, Vik Fearing wrote:
On 01/25/2014 06:25 AM, David Fetter wrote:
Please find attached the next rev :)
This version looks committable to me, so I am marking it as such.
This doesn't contain a single regression test, I don't see how that's
ok. Marking as waiting on author.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 15, 2014 at 03:14:03PM +0100, Andres Freund wrote:
On 2014-01-31 18:16:18 +0100, Vik Fearing wrote:
On 01/25/2014 06:25 AM, David Fetter wrote:
Please find attached the next rev :)
This version looks committable to me, so I am marking it as such.
This doesn't contain a single regression test, I don't see how that's
ok. Marking as waiting on author.
It now contains regression tests. Re-marking.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
foreign_table_like_08.difftext/plain; charset=us-asciiDownload
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 38c6cf8..eab48fe 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -72,6 +72,20 @@ CREATE FOREIGN TABLE ft2 (
c8 user_enum
) SERVER loopback;
ALTER FOREIGN TABLE ft2 DROP COLUMN cx;
+CREATE FOREIGN TABLE ft_tables (
+ LIKE information_schema.tables
+) SERVER loopback
+OPTIONS (
+ schema_name 'information_schema',
+ table_name 'tables'
+);
+CREATE FOREIGN TABLE ft_columns (
+ LIKE information_schema.columns
+) SERVER loopback
+OPTIONS (
+ schema_name 'information_schema',
+ table_name 'columns'
+);
-- ===================================================================
-- tests for validator
-- ===================================================================
@@ -113,12 +127,14 @@ ALTER FOREIGN TABLE ft2 OPTIONS (schema_name 'S 1', table_name 'T 1');
ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
ALTER FOREIGN TABLE ft2 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
\det+
- List of foreign tables
- Schema | Table | Server | FDW Options | Description
---------+-------+----------+---------------------------------------+-------------
- public | ft1 | loopback | (schema_name 'S 1', table_name 'T 1') |
- public | ft2 | loopback | (schema_name 'S 1', table_name 'T 1') |
-(2 rows)
+ List of foreign tables
+ Schema | Table | Server | FDW Options | Description
+--------+------------+----------+----------------------------------------------------------+-------------
+ public | ft1 | loopback | (schema_name 'S 1', table_name 'T 1') |
+ public | ft2 | loopback | (schema_name 'S 1', table_name 'T 1') |
+ public | ft_columns | loopback | (schema_name 'information_schema', table_name 'columns') |
+ public | ft_tables | loopback | (schema_name 'information_schema', table_name 'tables') |
+(4 rows)
-- Now we should be able to run ANALYZE.
-- To exercise multiple code paths, we use local stats on ft1
@@ -231,13 +247,25 @@ SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
101 | 1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
--- aggregate
+-- aggregates
SELECT COUNT(*) FROM ft1 t1;
count
-------
1000
(1 row)
+SELECT t.table_name, COUNT(*)
+FROM ft_tables t JOIN ft_columns c USING (table_catalog, table_schema, table_name)
+WHERE t.table_type = 'FOREIGN TABLE'
+GROUP BY t.table_name;
+ table_name | count
+------------+-------
+ ft_columns | 44
+ ft2 | 8
+ ft_tables | 12
+ ft1 | 8
+(4 rows)
+
-- join two tables
SELECT t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
c1
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index ce8bb75..c5d2fde 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -81,6 +81,22 @@ CREATE FOREIGN TABLE ft2 (
) SERVER loopback;
ALTER FOREIGN TABLE ft2 DROP COLUMN cx;
+CREATE FOREIGN TABLE ft_tables (
+ LIKE information_schema.tables
+) SERVER loopback
+OPTIONS (
+ schema_name 'information_schema',
+ table_name 'tables'
+);
+
+CREATE FOREIGN TABLE ft_columns (
+ LIKE information_schema.columns
+) SERVER loopback
+OPTIONS (
+ schema_name 'information_schema',
+ table_name 'columns'
+);
+
-- ===================================================================
-- tests for validator
-- ===================================================================
@@ -145,8 +161,12 @@ SELECT * FROM ft1 WHERE false;
-- with WHERE clause
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
--- aggregate
+-- aggregates
SELECT COUNT(*) FROM ft1 t1;
+SELECT t.table_name, COUNT(*)
+FROM ft_tables t JOIN ft_columns c USING (table_catalog, table_schema, table_name)
+WHERE t.table_type = 'FOREIGN TABLE'
+GROUP BY t.table_name;
-- join two tables
SELECT t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
-- subquery
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..375bd1a 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -20,6 +20,7 @@
<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,6 +32,8 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
{ 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,6 +117,19 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
</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 like <literal>INCLUDING STORAGE</literal> are ignored.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index eb07ca3..82c77eb 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,7 +649,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
/*
* transformTableLikeClause
*
- * Change the LIKE <srctable> portion of a CREATE TABLE statement into
+ * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement into
* column definitions which recreate the user defined column portions of
* <srctable>.
*/
@@ -668,12 +668,6 @@ 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 &&
@@ -689,6 +683,12 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
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
*/
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 60506e0..2df1c60 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -699,6 +699,13 @@ SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
EXPLAIN SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
+CREATE FOREIGN TABLE foreign_schema.ft_columns (LIKE information_schema.columns)
+ SERVER s0 OPTIONS (delimiter ',', quote '"');
+CREATE FOREIGN TABLE doesnt_exist_ft1 (LIKE doesnt_exist_lt1) -- ERROR
+ SERVER s0 OPTIONS (delimiter ',', quote '"');
+ERROR: relation "doesnt_exist_lt1" does not exist
+LINE 1: CREATE FOREIGN TABLE doesnt_exist_ft1 (LIKE doesnt_exist_lt1...
+ ^
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
COMMENT ON FOREIGN TABLE ft1 IS NULL;
@@ -919,7 +926,8 @@ SELECT * FROM information_schema.foreign_tables ORDER BY 1, 2, 3;
foreign_table_catalog | foreign_table_schema | foreign_table_name | foreign_server_catalog | foreign_server_name
-----------------------+----------------------+--------------------+------------------------+---------------------
regression | foreign_schema | foreign_table_1 | regression | s0
-(1 row)
+ regression | foreign_schema | ft_columns | regression | s0
+(2 rows)
SELECT * FROM information_schema.foreign_table_options ORDER BY 1, 2, 3, 4;
foreign_table_catalog | foreign_table_schema | foreign_table_name | option_name | option_value
@@ -927,7 +935,9 @@ SELECT * FROM information_schema.foreign_table_options ORDER BY 1, 2, 3, 4;
regression | foreign_schema | foreign_table_1 | be quoted | value
regression | foreign_schema | foreign_table_1 | escape | @
regression | foreign_schema | foreign_table_1 | quote | ~
-(3 rows)
+ regression | foreign_schema | ft_columns | delimiter | ,
+ regression | foreign_schema | ft_columns | quote | "
+(5 rows)
SET ROLE regress_test_role;
SELECT * FROM information_schema.user_mapping_options ORDER BY 1, 2, 3, 4;
@@ -1166,6 +1176,7 @@ NOTICE: foreign table "no_table" does not exist, skipping
DROP FOREIGN TABLE foreign_schema.foreign_table_1;
-- Cleanup
DROP SCHEMA foreign_schema CASCADE;
+NOTICE: drop cascades to foreign table foreign_schema.ft_columns
DROP ROLE regress_test_role; -- ERROR
ERROR: role "regress_test_role" cannot be dropped because some objects depend on it
DETAIL: privileges for server s4
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index f819eb1..54475d8 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -280,6 +280,10 @@ COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
CREATE INDEX id_ft1_c2 ON ft1 (c2); -- ERROR
SELECT * FROM ft1; -- ERROR
EXPLAIN SELECT * FROM ft1; -- ERROR
+CREATE FOREIGN TABLE foreign_schema.ft_columns (LIKE information_schema.columns)
+ SERVER s0 OPTIONS (delimiter ',', quote '"');
+CREATE FOREIGN TABLE doesnt_exist_ft1 (LIKE doesnt_exist_lt1) -- ERROR
+ SERVER s0 OPTIONS (delimiter ',', quote '"');
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
On 2014-02-16 20:27:09 -0800, David Fetter wrote:
On Sat, Feb 15, 2014 at 03:14:03PM +0100, Andres Freund wrote:
On 2014-01-31 18:16:18 +0100, Vik Fearing wrote:
On 01/25/2014 06:25 AM, David Fetter wrote:
Please find attached the next rev :)
This version looks committable to me, so I am marking it as such.
This doesn't contain a single regression test, I don't see how that's
ok. Marking as waiting on author.It now contains regression tests. Re-marking.
I don't think this really has gone above Needs Review yet.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(
By the way, I spent some time looking at it and here are some comments:
- Regression tests added are too sensitive with the other tests. For
example by not dropping tables or creating new tables on another tests
run before foreign_data you would need to update the output of this
test as well, something rather unfriendly.
- Regression coverage is limited (there is nothing done for comments
and default expressions)
- regression tests are added in postgres_fdw. This should be perhaps
the target of another patch so I removed them for now as this is only
a core feature (if I am wrong here don't hesitate). Same remark about
information_schema though, those tests are too fragile as they are.
- Documentation had some issues IMO:
-- A bracket was missing before "<replaceable class="PARAMETER">column_name"...
-- like_option should be clear about what it supports or not, more
precisely that it supports only default expressions and comments
-- some typos and formatting inconsistencies found
- In the case of CREATE TABLE, like_option is bypassed based on the
nature of the object linked, and not based on the nature of the object
created, so for CREATE FOREIGN TABLE, using this argument, I do not
think that we should simply ignore the options not directly supported
but return an error or a warning at least to user (attached patch
returns an ERROR). Documentation needs to reflect that precisely to
let the user know what can be and cannot be done.
After testing the patch, well it does what it is aimed for and it
works. It is somewhat unfortunate that we cannot enforce the name of
columns hidden behind LIKE directly with CREATE, but this would result
in some kludging in the code. It can as well be done simply with ALTER
FOREIGN TABLE.
All those comments result in the patch attached, which I think is in a
state close to committable, so I am marking it as "ready for
committer" (feel free to scream at me if you do not think so). Note
that the patch attached is not using context diffs but git diffs
(really I tried!) because of filterdiff that skipped a block of code
in parse_utilcmd.c.
Regards,
--
Michael
Attachments:
20140217_foreign_like_v9.patchtext/x-patch; charset=US-ASCII; name=20140217_foreign_like_v9.patchDownload
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..ecef3c0 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -19,7 +19,8 @@
<refsynopsisdiv>
<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> [ ... ] ]
+ { <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,6 +32,10 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
{ NOT NULL |
NULL |
DEFAULT <replaceable>default_expr</replaceable> }
+
+<phrase>and <replaceable class="PARAMETER">like_option</replaceable> is:</phrase>
+
+{ INCLUDING | EXCLUDING } { DEFAULTS | COMMENTS | ALL }
</synopsis>
</refsynopsisdiv>
@@ -114,6 +119,29 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable class="PARAMETER">table_name
</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>
+ Default expressions for the copied column definitions will only be
+ copied if <literal>INCLUDING DEFAULTS</literal> is specified.
+ Defaults that call database-modification functions, like
+ <function>nextval</>, create a linkage between the original and
+ new tables. The default behavior is to exclude default expressions,
+ resulting in the copied columns in the new table having null defaults.
+ </para>
+ <para>
+ The <literal>LIKE</literal> clause can also be used to copy columns from
+ views, tables, or composite types.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>NOT NULL</></term>
<listitem>
<para>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index eb07ca3..aec39b8 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,8 +649,8 @@ 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
+ * Change the LIKE <srctable> portion of a CREATE [FOREIGN] TABLE statement
+ * into column definitions which recreate the user defined column portions of
* <srctable>.
*/
static void
@@ -668,12 +668,6 @@ 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 &&
@@ -689,6 +683,27 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
cancel_parser_errposition_callback(&pcbstate);
/*
+ * Special handling for foreign tables as only default expressions
+ * and comments are supported.
+ */
+ if (cxt->isforeign)
+ {
+ /* ALL assumes default expressions and comments */
+ if (table_like_clause->options == CREATE_TABLE_LIKE_ALL)
+ table_like_clause->options &= CREATE_TABLE_LIKE_DEFAULTS |
+ CREATE_TABLE_LIKE_COMMENTS;
+
+ /* Other options are not supported */
+ if ((table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS) ||
+ (table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) ||
+ (table_like_clause->options & CREATE_TABLE_LIKE_STORAGE))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Foreign tables only support default expressions and comments with LIKE clause",
+ RelationGetRelationName(relation))));
+ }
+
+ /*
* Check for privileges
*/
if (relation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 60506e0..fe8e3ae 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -699,6 +699,47 @@ SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
EXPLAIN SELECT * FROM ft1; -- ERROR
ERROR: foreign-data wrapper "dummy" has no handler
+-- LIKE CLAUSE
+CREATE TABLE ft2 (a int DEFAULT 0);
+COMMENT ON COLUMN ft2.a IS 'Column a of ft2';
+CREATE FOREIGN TABLE ft2_like_1 (LIKE ft2) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_2 (LIKE ft2 INCLUDING DEFAULTS) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_3 (LIKE ft2 INCLUDING COMMENTS) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_4 (
+ LIKE ft2 INCLUDING INDEXES) SERVER s0; -- ERROR
+ERROR: Foreign tables only support default expressions and comments with LIKE clause
+CREATE FOREIGN TABLE ft2_like_4 (LIKE ft2 INCLUDING ALL) SERVER s0;
+\d+ ft2_like_*
+ Foreign table "public.ft2_like_1"
+ Column | Type | Modifiers | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+-------------+---------+--------------+-------------
+ a | integer | | | plain | |
+Server: s0
+Has OIDs: no
+
+ Foreign table "public.ft2_like_2"
+ Column | Type | Modifiers | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+-------------+---------+--------------+-------------
+ a | integer | default 0 | | plain | |
+Server: s0
+Has OIDs: no
+
+ Foreign table "public.ft2_like_3"
+ Column | Type | Modifiers | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+-------------+---------+--------------+-----------------
+ a | integer | | | plain | | Column a of ft2
+Server: s0
+Has OIDs: no
+
+ Foreign table "public.ft2_like_4"
+ Column | Type | Modifiers | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+-------------+---------+--------------+-----------------
+ a | integer | default 0 | | plain | | Column a of ft2
+Server: s0
+Has OIDs: no
+
+DROP TABLE ft2;
+DROP FOREIGN TABLE ft2_like_1, ft2_like_2, ft2_like_3, ft2_like_4;
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
COMMENT ON FOREIGN TABLE ft1 IS NULL;
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index f819eb1..c4dc1f1 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -281,6 +281,19 @@ CREATE INDEX id_ft1_c2 ON ft1 (c2); -- ERROR
SELECT * FROM ft1; -- ERROR
EXPLAIN SELECT * FROM ft1; -- ERROR
+-- LIKE CLAUSE
+CREATE TABLE ft2 (a int DEFAULT 0);
+COMMENT ON COLUMN ft2.a IS 'Column a of ft2';
+CREATE FOREIGN TABLE ft2_like_1 (LIKE ft2) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_2 (LIKE ft2 INCLUDING DEFAULTS) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_3 (LIKE ft2 INCLUDING COMMENTS) SERVER s0;
+CREATE FOREIGN TABLE ft2_like_4 (
+ LIKE ft2 INCLUDING INDEXES) SERVER s0; -- ERROR
+CREATE FOREIGN TABLE ft2_like_4 (LIKE ft2 INCLUDING ALL) SERVER s0;
+\d+ ft2_like_*
+DROP TABLE ft2;
+DROP FOREIGN TABLE ft2_like_1, ft2_like_2, ft2_like_3, ft2_like_4;
+
-- ALTER FOREIGN TABLE
COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
COMMENT ON FOREIGN TABLE ft1 IS NULL;
On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(
Uh. What should I then say if a patch is marked as ready for committer
by the author, after it previously had been marked such when it clearly
wasn't? Your review just seems to confirm that it wasn't ready?
If the patch is isn't marked "needs review" in the CF it's less likely
to get timely review. And when a committer looks at the patch it'll just
be determined at not being ready again, making it less likely to get
committed.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(By the way, I spent some time looking at it and here are some
comments:
David just pinged me and tricked me into having a quick look :)
Unless I miss something this possibly allows column definition to slip
by that shouldn't because normally all fdw column definitions are passed
through transformColumnDefinition() which does some checks, but the
copied ones aren't.
I haven't looked long enough to see whether that's currently
problematic, but even if not, it's sure a trap waiting to spring.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Feb 18, 2014 at 7:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(By the way, I spent some time looking at it and here are some
comments:David just pinged me and tricked me into having a quick look :)
Unless I miss something this possibly allows column definition to slip
by that shouldn't because normally all fdw column definitions are passed
through transformColumnDefinition() which does some checks, but the
copied ones aren't.
I haven't looked long enough to see whether that's currently
problematic, but even if not, it's sure a trap waiting to spring.
transformColumnDefinition contains checks about serial and constraints
mainly. The only thing that could be problematic IMO is the process
done exclusively for foreign tables which is the creation of some
ALTER FOREIGN TABLE ALTER COLUMN commands when per-column options are
detected, something that is not passed to a like'd table with this
patch. This may meritate a comment in the code.
Actually after more thinking I think that it would make sense to have
another INCLUDING/EXCLUDING option for foreign tables: OPTIONS to pass
the column options when link is done from another foreign table. This
should be another patch though.
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-02-18 08:35:35 +0900, Michael Paquier wrote:
On Tue, Feb 18, 2014 at 7:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-02-17 23:07:45 +0900, Michael Paquier wrote:
On Mon, Feb 17, 2014 at 6:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
I don't think this really has gone above Needs Review yet.
I am not sure that this remark makes the review of this patch much
progressing :(By the way, I spent some time looking at it and here are some
comments:David just pinged me and tricked me into having a quick look :)
Unless I miss something this possibly allows column definition to slip
by that shouldn't because normally all fdw column definitions are passed
through transformColumnDefinition() which does some checks, but the
copied ones aren't.
I haven't looked long enough to see whether that's currently
problematic, but even if not, it's sure a trap waiting to spring.
transformColumnDefinition contains checks about serial and constraints
mainly. The only thing that could be problematic IMO is the process
done exclusively for foreign tables which is the creation of some
ALTER FOREIGN TABLE ALTER COLUMN commands when per-column options are
detected, something that is not passed to a like'd table with this
patch. This may meritate a comment in the code.
As I said, I am not all that concerned that it's a big problem today,
but imo it's an accident waiting to happen.
I rather wonder if the code shouln't just ensure it's running
transformTableLikeClause() before transformColumnDefinition() by doing
it in a separate loop.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier <michael.paquier@gmail.com> writes:
On Tue, Feb 18, 2014 at 7:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:
Unless I miss something this possibly allows column definition to slip
by that shouldn't because normally all fdw column definitions are passed
through transformColumnDefinition() which does some checks, but the
copied ones aren't.
I haven't looked long enough to see whether that's currently
problematic, but even if not, it's sure a trap waiting to spring.
transformColumnDefinition contains checks about serial and constraints
mainly. The only thing that could be problematic IMO is the process
done exclusively for foreign tables which is the creation of some
ALTER FOREIGN TABLE ALTER COLUMN commands when per-column options are
detected, something that is not passed to a like'd table with this
patch. This may meritate a comment in the code.
Actually after more thinking I think that it would make sense to have
another INCLUDING/EXCLUDING option for foreign tables: OPTIONS to pass
the column options when link is done from another foreign table. This
should be another patch though.
ISTM this is because the proposed feature is wrongheaded. The basic
concept of CREATE TABLE LIKE is that you're copying properties from
another object of the same type. You might or might not want every
property, but there's no question of whether you *could* copy every
property. In contrast, what this is proposing to do is copy properties
from (what might be) a plain table to a foreign table, and those things
aren't even remotely the same kind of object.
It would make sense to me to restrict LIKE to copy from another foreign
table, and then there would be a different set of INCLUDING/EXCLUDING
options that would be relevant (options yes, indexes no, for example).
In any case, I agree with Andres' concern: whether or not it's a bug
currently that this bypasses some of the normal processing, it's a hazard
that can be expected to bite us someday.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-04-05 11:46:16 -0400, Tom Lane wrote:
ISTM this is because the proposed feature is wrongheaded. The basic
concept of CREATE TABLE LIKE is that you're copying properties from
another object of the same type. You might or might not want every
property, but there's no question of whether you *could* copy every
property. In contrast, what this is proposing to do is copy properties
from (what might be) a plain table to a foreign table, and those things
aren't even remotely the same kind of object.It would make sense to me to restrict LIKE to copy from another foreign
table, and then there would be a different set of INCLUDING/EXCLUDING
options that would be relevant (options yes, indexes no, for example).
I actually think it's quite useful to create a foreign table that's the
same shape as a local table. And the patches approach of refusing to
copy thinks that aren't supported sounds sane to me.
Consider e.g. moving off older partitioned data off to an archiving
server. New local partitions are often created using CREATE TABLE LIKE,
but that's not possible for the foreign ones.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Apr 8, 2014 at 5:24 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-04-05 11:46:16 -0400, Tom Lane wrote:
ISTM this is because the proposed feature is wrongheaded. The basic
concept of CREATE TABLE LIKE is that you're copying properties from
another object of the same type. You might or might not want every
property, but there's no question of whether you *could* copy every
property. In contrast, what this is proposing to do is copy properties
from (what might be) a plain table to a foreign table, and those things
aren't even remotely the same kind of object.It would make sense to me to restrict LIKE to copy from another foreign
table, and then there would be a different set of INCLUDING/EXCLUDING
options that would be relevant (options yes, indexes no, for example).I actually think it's quite useful to create a foreign table that's the
same shape as a local table. And the patches approach of refusing to
copy thinks that aren't supported sounds sane to me.
This could be improved as well: it would be useful to be able to copy
the column options of another foreign table.
Consider e.g. moving off older partitioned data off to an archiving
server. New local partitions are often created using CREATE TABLE LIKE,
but that's not possible for the foreign ones.
Definitely a use case.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Apr 7, 2014 at 4:24 PM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-04-05 11:46:16 -0400, Tom Lane wrote:
ISTM this is because the proposed feature is wrongheaded. The basic
concept of CREATE TABLE LIKE is that you're copying properties from
another object of the same type. You might or might not want every
property, but there's no question of whether you *could* copy every
property. In contrast, what this is proposing to do is copy properties
from (what might be) a plain table to a foreign table, and those things
aren't even remotely the same kind of object.It would make sense to me to restrict LIKE to copy from another foreign
table, and then there would be a different set of INCLUDING/EXCLUDING
options that would be relevant (options yes, indexes no, for example).I actually think it's quite useful to create a foreign table that's the
same shape as a local table. And the patches approach of refusing to
copy thinks that aren't supported sounds sane to me.
Consider e.g. moving off older partitioned data off to an archiving
server. New local partitions are often created using CREATE TABLE LIKE,
but that's not possible for the foreign ones.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/04/08 9:26), Michael Paquier wrote:
On Tue, Apr 8, 2014 at 5:24 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-04-05 11:46:16 -0400, Tom Lane wrote:
ISTM this is because the proposed feature is wrongheaded. The basic
concept of CREATE TABLE LIKE is that you're copying properties from
another object of the same type. You might or might not want every
property, but there's no question of whether you *could* copy every
property. In contrast, what this is proposing to do is copy properties
from (what might be) a plain table to a foreign table, and those things
aren't even remotely the same kind of object.It would make sense to me to restrict LIKE to copy from another foreign
table, and then there would be a different set of INCLUDING/EXCLUDING
options that would be relevant (options yes, indexes no, for example).I actually think it's quite useful to create a foreign table that's the
same shape as a local table. And the patches approach of refusing to
copy thinks that aren't supported sounds sane to me.This could be improved as well: it would be useful to be able to copy
the column options of another foreign table.
Yes, I think so, too. But to think of validating generic column/table
options, I think we would probably need to restrict LIKE to copy from
another foreign table maybe using the same FDW. So, I'd like to vote
for Tom's idea.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers