Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On lör, 2012-06-23 at 23:08 +0100, Dean Rasheed wrote: > I spotted a couple of other issues during testing: > > * You're still allowing INCLUDING DEFAULTS and INCLUDING STORAGE, even > though these options are not supported on foreign tables. > > * If I do INCLUDING ALL, I get an error because of the unsupported > options. I think that "ALL" in this context needs to be made to mean > all the options that foreign tables support (just COMMENTS at the > moment). Note that when I added CREATE TABLE LIKE to support composite types, it was decided to ignore non-applicable options (like copying indexes from types or views etc.). The same should be done here, unless we have reasons to revise the earlier decision. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On 24 June 2012 04:01, Alvaro Herrera wrote: > > Excerpts from Dean Rasheed's message of sáb jun 23 18:08:31 -0400 2012: > >> I spotted a couple of other issues during testing: > > David, when you generate a new version of the patch, please also make > sure to use RELKIND_RELATION and RELKIND_FOREIGN instead of 'r' and 'f'. > >> * You're still allowing INCLUDING DEFAULTS and INCLUDING STORAGE, even >> though these options are not supported on foreign tables. > > Maybe the code should list options allowed instead of the ones > disallowed. > >> * If I do INCLUDING ALL, I get an error because of the unsupported >> options. I think that "ALL" in this context needs to be made to mean >> all the options that foreign tables support (just COMMENTS at the >> moment). > > I agree. > David, do you have an updated version of this patch? Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
Excerpts from Dean Rasheed's message of sáb jun 23 18:08:31 -0400 2012: > I spotted a couple of other issues during testing: David, when you generate a new version of the patch, please also make sure to use RELKIND_RELATION and RELKIND_FOREIGN instead of 'r' and 'f'. > * You're still allowing INCLUDING DEFAULTS and INCLUDING STORAGE, even > though these options are not supported on foreign tables. Maybe the code should list options allowed instead of the ones disallowed. > * If I do INCLUDING ALL, I get an error because of the unsupported > options. I think that "ALL" in this context needs to be made to mean > all the options that foreign tables support (just COMMENTS at the > moment). I agree. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On 23 March 2012 18:38, David Fetter wrote: > On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote: >> Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: >> > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: >> > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: >> > > >> I think that instead of inventing new grammar productions and a new >> > > >> node type for this, you should just reuse the existing productions for >> > > >> LIKE clauses and then reject invalid options during parse analysis. >> > > > >> > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and >> > > > submit that as a separate patch? >> > > >> > > I don't see any reason to do that. I merely meant that you could >> > > reuse TableLikeClause or maybe even TableElement in the grammer for >> > > CreateForeignTableStmt. >> > >> > Next WIP patch attached implementing this via reusing TableLikeClause >> > and refactoring transformTableLikeClause(). >> > >> > What say? >> >> Looks much better to me, but the use of strcmp() doesn't look good. >> ISTM that stmtType is mostly used for error messages. I think you >> should add some kind of identifier (such as the original parser Node) >> into the CreateStmtContext so that you can do a IsA() test instead -- a >> bit more invasive as a patch, but much cleaner. >> >> Also the error messages need more work. > > How about this one? > I spotted a couple of other issues during testing: * You're still allowing INCLUDING DEFAULTS and INCLUDING STORAGE, even though these options are not supported on foreign tables. * If I do INCLUDING ALL, I get an error because of the unsupported options. I think that "ALL" in this context needs to be made to mean all the options that foreign tables support (just COMMENTS at the moment). Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
The patch uses literals such as 'r' to identify the relkind values. This should be using RELKIND_RELATION et al instead -- see src/include/catalog/pg_class.h. Other than that, it seems simple enough ... -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On 23 March 2012 19:07, David Fetter wrote: > On Fri, Mar 23, 2012 at 11:38:56AM -0700, David Fetter wrote: >> How about this one? > > Oops, forgot to put the latest docs in. I think the docs need some additional supporting content. The LIKE clause and its source_table parameter isn't explained on the CREATE FOREIGN TABLE page. There's no mention of the like_option parameter too which should be valid since you can specify whether it includes comments (among less relevant options). Also you appear to have modified the documented command definition so that OPTIONS can't be applied per-column anymore. It's now placed "[, ... ]" prior to the column's OPTIONS clause. The patch works for me though, and allows tables, foreign tables, views and composite types to be used in the LIKE clause. Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Fri, Mar 23, 2012 at 11:38:56AM -0700, David Fetter wrote:
> On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote:
> > Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012:
> > > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote:
> > > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote:
> > > > >> I think that instead of inventing new grammar productions and a new
> > > > >> node type for this, you should just reuse the existing productions
> > > > >> for
> > > > >> LIKE clauses and then reject invalid options during parse analysis.
> > > > >
> > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and
> > > > > submit that as a separate patch?
> > > >
> > > > I don't see any reason to do that. I merely meant that you could
> > > > reuse TableLikeClause or maybe even TableElement in the grammer for
> > > > CreateForeignTableStmt.
> > >
> > > Next WIP patch attached implementing this via reusing TableLikeClause
> > > and refactoring transformTableLikeClause().
> > >
> > > What say?
> >
> > Looks much better to me, but the use of strcmp() doesn't look good.
> > ISTM that stmtType is mostly used for error messages. I think you
> > should add some kind of identifier (such as the original parser Node)
> > into the CreateStmtContext so that you can do a IsA() test instead -- a
> > bit more invasive as a patch, but much cleaner.
> >
> > Also the error messages need more work.
>
> How about this one?
Oops, forgot to put the latest docs in.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@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
*** a/doc/src/sgml/ref/create_foreign_table.sgml
--- b/doc/src/sgml/ref/create_foreign_table.sgml
***
*** 19,26
CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name ( [
! { column_name data_type [ OPTIONS ( option 'value' [, ... ] ) ] [ NULL | NOT NULL ] }
! [, ... ]
] )
SERVER server_name
[ OPTIONS ( option 'value' [, ... ] ) ]
--- 19,26
CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name ( [
! { { column_name data_type [ NULL | NOT NULL ] | LIKE
source_table } [, ... ]
! [ OPTIONS ( option
'value' [, ... ] ) ] }
] )
SERVER server_name
[ OPTIONS ( option 'value' [, ... ] ) ]
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 3945,3950 ForeignTableElementList:
--- 3945,3951
ForeignTableElement:
columnDef { $$ =
$1; }
+ | TableLikeClause { $$ = $1; }
;
/*
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***
*** 66,71 typedef struct
--- 66,72
{
ParseState *pstate; /* overall parser state */
const char *stmtType; /* "CREATE [FOREIGN] TABLE" or "ALTER
TABLE" */
+ charrelkind;/* r = ordinary table, f =
foreign table, cf. pg_catalog.pg_class */
RangeVar *relation; /* relation to create */
Relationrel;/* opened/locked rel, if ALTER
*/
List *inhRelations; /* relations to inherit from */
***
*** 194,202 transformCreateStmt(CreateStmt *stmt, const char *queryString)
--- 195,209
cxt.pstate = pstate;
if (IsA(stmt, CreateForeignTableStmt))
+ {
cxt.stmtType = "CREATE FOREIGN TABLE";
+ cxt.relkind = 'f';
+ }
else
+ {
cxt.stmtType = "CREATE TABLE";
+ cxt.relkind = 'r';
+ }
cxt.relation = stmt->relation;
cxt.rel = NULL;
cxt.inhRelations = stmt->inhRelations;
***
*** 623,629 transformTableConstraint(CreateStmtContext *cxt, Constraint
*constraint)
/*
* transformTableLikeClause
*
! * Change the LIKE portion of a CREATE TABLE statement into
* column definitions which recreate the user defined column portions of
* .
*/
--- 630,636
/*
* transformTableLikeClause
*
! * Change the LIKE portion of a CREATE [FOREIGN] TABLE statement
into
* column definitions which recreate the user defined column portions of
* .
*/
***
*** 652,657 transformTableLikeClause(CreateStmtContext *cxt,
TableLikeClause *table_like_cla
--- 659,683
table_like_clause->relation->relname)));
cancel_parser_errposition_callback(&pcbstate);
+
+ /*
+* For foreign tables, disallow some options.
+*/
+ if (cxt->relkind ==
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote:
> Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012:
> > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote:
> > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote:
> > > >> I think that instead of inventing new grammar productions and a new
> > > >> node type for this, you should just reuse the existing productions for
> > > >> LIKE clauses and then reject invalid options during parse analysis.
> > > >
> > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and
> > > > submit that as a separate patch?
> > >
> > > I don't see any reason to do that. I merely meant that you could
> > > reuse TableLikeClause or maybe even TableElement in the grammer for
> > > CreateForeignTableStmt.
> >
> > Next WIP patch attached implementing this via reusing TableLikeClause
> > and refactoring transformTableLikeClause().
> >
> > What say?
>
> Looks much better to me, but the use of strcmp() doesn't look good.
> ISTM that stmtType is mostly used for error messages. I think you
> should add some kind of identifier (such as the original parser Node)
> into the CreateStmtContext so that you can do a IsA() test instead -- a
> bit more invasive as a patch, but much cleaner.
>
> Also the error messages need more work.
How about this one?
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@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
*** a/doc/src/sgml/ref/create_foreign_table.sgml
--- b/doc/src/sgml/ref/create_foreign_table.sgml
***
*** 19,26
CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name ( [
! { column_name data_type [ OPTIONS ( option 'value' [, ... ] ) ] [ NULL | NOT NULL ] }
! [, ... ]
] )
SERVER server_name
[ OPTIONS ( option 'value' [, ... ] ) ]
--- 19,26
CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name ( [
! { { column_name data_type [ NULL | NOT NULL ] | LIKE
source_table } [, ... ]
! [ OPTIONS ( option
'value' [, ... ] ) ] }
] )
SERVER server_name
[ OPTIONS ( option 'value' [, ... ] ) ]
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 3945,3950 ForeignTableElementList:
--- 3945,3951
ForeignTableElement:
columnDef { $$ =
$1; }
+ | TableLikeClause { $$ = $1; }
;
/*
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***
*** 66,71 typedef struct
--- 66,72
{
ParseState *pstate; /* overall parser state */
const char *stmtType; /* "CREATE [FOREIGN] TABLE" or "ALTER
TABLE" */
+ charrelkind;/* r = ordinary table, f =
foreign table, cf. pg_catalog.pg_class */
RangeVar *relation; /* relation to create */
Relationrel;/* opened/locked rel, if ALTER
*/
List *inhRelations; /* relations to inherit from */
***
*** 194,202 transformCreateStmt(CreateStmt *stmt, const char *queryString)
--- 195,209
cxt.pstate = pstate;
if (IsA(stmt, CreateForeignTableStmt))
+ {
cxt.stmtType = "CREATE FOREIGN TABLE";
+ cxt.relkind = 'f';
+ }
else
+ {
cxt.stmtType = "CREATE TABLE";
+ cxt.relkind = 'r';
+ }
cxt.relation = stmt->relation;
cxt.rel = NULL;
cxt.inhRelations = stmt->inhRelations;
***
*** 623,629 transformTableConstraint(CreateStmtContext *cxt, Constraint
*constraint)
/*
* transformTableLikeClause
*
! * Change the LIKE portion of a CREATE TABLE statement into
* column definitions which recreate the user defined column portions of
* .
*/
--- 630,636
/*
* transformTableLikeClause
*
! * Change the LIKE portion of a CREATE [FOREIGN] TABLE statement
into
* column definitions which recreate the user defined column portions of
* .
*/
***
*** 652,657 transformTableLikeClause(CreateStmtContext *cxt,
TableLikeClause *table_like_cla
--- 659,683
table_like_clause->relation->relname)));
cancel_parser_errposition_callback(&pcbstate);
+
+ /*
+* For foreign tables, disallow some options.
+*/
+ if (cxt->relkind == 'f')
+ {
+ if (table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS)
+ {
+ ereport(ERROR,
+
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote: > Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > > >> I think that instead of inventing new grammar productions and a new > > > >> node type for this, you should just reuse the existing productions for > > > >> LIKE clauses and then reject invalid options during parse analysis. > > > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and > > > > submit that as a separate patch? > > > > > > I don't see any reason to do that. I merely meant that you could > > > reuse TableLikeClause or maybe even TableElement in the grammer for > > > CreateForeignTableStmt. > > > > Next WIP patch attached implementing this via reusing TableLikeClause > > and refactoring transformTableLikeClause(). > > > > What say? > > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. Is it used for anything at all? > I think you should add some kind of identifier (such as the original > parser Node) into the CreateStmtContext so that you can do a IsA() > test instead -- a bit more invasive as a patch, but much cleaner. OK Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On ons, 2012-03-14 at 17:38 -0400, Tom Lane wrote: > Peter Eisentraut writes: > > On ons, 2012-03-14 at 17:16 -0400, Robert Haas wrote: > >> If a constraint is NOT ENFORCED, then the query planner presumably > >> won't rely on it for planning purposes > > > Why do you presume that? > > What does SQL:2011 say exactly about the semantics of NOT ENFORCED? > Is an implementation allowed to fail in undefined ways if a constraint > is marked NOT ENFORCED and is not actually true? It doesn't say anything about that. I might have to dig deeper into the change proposals to see if any rationale came with this change. But in any case, even if we spell it differently, I think there are use cases for a constraint mode that says, assume it's true for optimization purposes, but don't spend any cycles on verifying it. And that constraint mode could apply to regular tables and foreign tables alike. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 10:23 AM, Alvaro Herrera wrote: > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. I think you > should add some kind of identifier (such as the original parser Node) > into the CreateStmtContext so that you can do a IsA() test instead -- a > bit more invasive as a patch, but much cleaner. +1. Or maybe add a relkind to CreateStmt, if it isn't there already, and test that. > Also the error messages need more work. +1. I suggest something like "ERROR: foreign tables do not support LIKE INCLUDING INDEXES". -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote: > Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > > >> I think that instead of inventing new grammar productions and > > > >> a new node type for this, you should just reuse the existing > > > >> productions for LIKE clauses and then reject invalid options > > > >> during parse analysis. > > > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE > > > > TABLE and submit that as a separate patch? > > > > > > I don't see any reason to do that. I merely meant that you > > > could reuse TableLikeClause or maybe even TableElement in the > > > grammer for CreateForeignTableStmt. > > > > Next WIP patch attached implementing this via reusing > > TableLikeClause and refactoring transformTableLikeClause(). > > > > What say? > > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. I think you > should add some kind of identifier (such as the original parser > Node) into the CreateStmtContext so that you can do a IsA() test > instead -- a bit more invasive as a patch, but much cleaner. OK > Also the error messages need more work. What sort? The more I look at this, the more I think that CREATE TABLE and CREATE FOREIGN TABLE should be merged, but that's the subject of a later patch. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > >> I think that instead of inventing new grammar productions and a new > > >> node type for this, you should just reuse the existing productions for > > >> LIKE clauses and then reject invalid options during parse analysis. > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and > > > submit that as a separate patch? > > > > I don't see any reason to do that. I merely meant that you could > > reuse TableLikeClause or maybe even TableElement in the grammer for > > CreateForeignTableStmt. > > Next WIP patch attached implementing this via reusing TableLikeClause > and refactoring transformTableLikeClause(). > > What say? Looks much better to me, but the use of strcmp() doesn't look good. ISTM that stmtType is mostly used for error messages. I think you should add some kind of identifier (such as the original parser Node) into the CreateStmtContext so that you can do a IsA() test instead -- a bit more invasive as a patch, but much cleaner. Also the error messages need more work. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
(2012/03/15 0:29), Tom Lane wrote: > The posted patch for file_fdw takes the > approach of silently filtering out rows for which they're not true, > which is not obviously the right thing either --- quite aside from > whether that's a sane semantics, it's not going to scale to foreign key > constraints, and even for simple NOT NULL and CHECK constraints it > results in a runtime penalty on selects, which is not what people would > expect from a constraint. I investigated DB2 a little bit. In DB2, the user can specify the VALIDATE_DATA_FILE option as a generic option for an external table attached to a data file, which specifies if the wrapper verifies that the data file is sorted. How about introducing this kind of option to file_fdw? It might be better that the default value for the option is 'false', and if the value is set to 'true', then file_fdw verifies NOT NULL, CHECK, and foreign key constraints. 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote:
> On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote:
> >> I think that instead of inventing new grammar productions and a new
> >> node type for this, you should just reuse the existing productions for
> >> LIKE clauses and then reject invalid options during parse analysis.
> >
> > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and
> > submit that as a separate patch?
>
> I don't see any reason to do that. I merely meant that you could
> reuse TableLikeClause or maybe even TableElement in the grammer for
> CreateForeignTableStmt.
Next WIP patch attached implementing this via reusing TableLikeClause
and refactoring transformTableLikeClause().
What say?
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@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
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 3950,3955 ForeignTableElementList:
--- 3950,3956
ForeignTableElement:
columnDef { $$ =
$1; }
+ | TableLikeClause { $$ = $1; }
;
/*
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***
*** 652,657 transformTableLikeClause(CreateStmtContext *cxt,
TableLikeClause *table_like_cla
--- 652,678
table_like_clause->relation->relname)));
cancel_parser_errposition_callback(&pcbstate);
+
+ /*
+* For foreign tables, disallow some options.
+*/
+ if (strcmp(cxt->stmtType, "CREATE FOREIGN TABLE")==0)
+ {
+ if (table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg("\"%s\" is a foreign table.
Only local tables can take LIKE CONSTRAINTS",
+
table_like_clause->relation->relname)));
+ }
+ else if (table_like_clause->options & CREATE_TABLE_LIKE_INDEXES)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg("\"%s\" is a foreign table.
Only local tables can take LIKE INDEXES",
+
table_like_clause->relation->relname)));
+ }
+ }
/*
* 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 5:21 PM, Peter Eisentraut wrote: > On ons, 2012-03-14 at 17:16 -0400, Robert Haas wrote: >> If a constraint is NOT ENFORCED, then the query planner presumably >> won't rely on it for planning purposes > > Why do you presume that? Well, as Tom alludes to, I'm guessing that NOT ENFORCED is not a license to deliver wrong answers. But also as Tom says, what does the spec say? -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
Peter Eisentraut writes: > On ons, 2012-03-14 at 17:16 -0400, Robert Haas wrote: >> If a constraint is NOT ENFORCED, then the query planner presumably >> won't rely on it for planning purposes > Why do you presume that? What does SQL:2011 say exactly about the semantics of NOT ENFORCED? Is an implementation allowed to fail in undefined ways if a constraint is marked NOT ENFORCED and is not actually true? 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On ons, 2012-03-14 at 17:16 -0400, Robert Haas wrote: > If a constraint is NOT ENFORCED, then the query planner presumably > won't rely on it for planning purposes Why do you presume that? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 5:14 PM, Peter Eisentraut wrote: > On ons, 2012-03-14 at 16:44 -0400, Tom Lane wrote: >> On reflection I don't see anything much wrong with the "if you lied >> about the constraint it's your fault that things broke" position. >> It seems quite comparable to the fact that we take the user's >> assertions on faith as to the number and data types of the columns in >> a foreign table. > > We do enforce the data types of a foreign table. We can't ensure that > the data that a foreign table "contains" is valid at any moment, but > when we read the data and interact with it in SQL, we reject it if it's > not valid. Similarly, one could conceivably apply not-null and check > constraints as the data is read, which is exactly what the other patch > you referred to proposes. And I think we must do it that way, otherwise > check constraints on domains and check constraints on tables would > behave quite differently. > > So if we want to have fake constraints on foreign tables, I think we > should require the NOT ENFORCED decoration or something similar, unless > the FDW signals that it can enforce the constraint. I think that would be missing the point. If a constraint is NOT ENFORCED, then the query planner presumably won't rely on it for planning purposes, but the whole point of having constraints on foreign tables is that we want the query planner to do just that. -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On ons, 2012-03-14 at 16:44 -0400, Tom Lane wrote: > On reflection I don't see anything much wrong with the "if you lied > about the constraint it's your fault that things broke" position. > It seems quite comparable to the fact that we take the user's > assertions on faith as to the number and data types of the columns in > a foreign table. We do enforce the data types of a foreign table. We can't ensure that the data that a foreign table "contains" is valid at any moment, but when we read the data and interact with it in SQL, we reject it if it's not valid. Similarly, one could conceivably apply not-null and check constraints as the data is read, which is exactly what the other patch you referred to proposes. And I think we must do it that way, otherwise check constraints on domains and check constraints on tables would behave quite differently. So if we want to have fake constraints on foreign tables, I think we should require the NOT ENFORCED decoration or something similar, unless the FDW signals that it can enforce the constraint. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On 03/14/2012 04:44 PM, Tom Lane wrote: Peter Eisentraut writes: On ons, 2012-03-14 at 10:27 -0400, Tom Lane wrote: That opinion seems to me to connect to the recently-posted patch to make contrib/file_fdw enforce NOT NULL constraints. Should we instead have the position that constraints declared for foreign tables are statements that we can take on faith, and it's the user's fault if they are wrong? We should look into the NOT ENFORCED stuff for constraints in SQL:2011. Then we can have both, and both for regular and foreign tables. Have both what? The key point here is that we *can't* enforce constraints on foreign tables, at least not with anything like the semantics SQL constraints normally have. Ignoring that point leads to nonsensical conclusions. Declaring a foreign constraint as NOT ENFORCED might be a reasonable thing to do, but it doesn't help us decide what to do when that clause isn't attached. On reflection I don't see anything much wrong with the "if you lied about the constraint it's your fault that things broke" position. It seems quite comparable to the fact that we take the user's assertions on faith as to the number and data types of the columns in a foreign table. Maybe we should say that for foreign tables NOT ENFORCED is implied. That seems to amount to much the same thing. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
Peter Eisentraut writes: > On ons, 2012-03-14 at 10:27 -0400, Tom Lane wrote: >> That opinion seems to me to connect to the recently-posted patch to >> make contrib/file_fdw enforce NOT NULL constraints. Should we instead >> have the position that constraints declared for foreign tables are >> statements that we can take on faith, and it's the user's fault if >> they are wrong? > We should look into the NOT ENFORCED stuff for constraints in SQL:2011. > Then we can have both, and both for regular and foreign tables. Have both what? The key point here is that we *can't* enforce constraints on foreign tables, at least not with anything like the semantics SQL constraints normally have. Ignoring that point leads to nonsensical conclusions. Declaring a foreign constraint as NOT ENFORCED might be a reasonable thing to do, but it doesn't help us decide what to do when that clause isn't attached. On reflection I don't see anything much wrong with the "if you lied about the constraint it's your fault that things broke" position. It seems quite comparable to the fact that we take the user's assertions on faith as to the number and data types of the columns in a foreign table. 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On ons, 2012-03-14 at 10:27 -0400, Tom Lane wrote: > That opinion seems to me to connect to the recently-posted patch to > make contrib/file_fdw enforce NOT NULL constraints. Should we instead > have the position that constraints declared for foreign tables are > statements that we can take on faith, and it's the user's fault if > they are wrong? We should look into the NOT ENFORCED stuff for constraints in SQL:2011. Then we can have both, and both for regular and foreign tables. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 12:00 PM, Tom Lane wrote: > David Fetter writes: >> On Wed, Mar 14, 2012 at 11:29:14AM -0400, Tom Lane wrote: >>> The posted patch for file_fdw takes the approach of silently >>> filtering out rows for which they're not true, which is not >>> obviously the right thing either --- quite aside from whether that's >>> a sane semantics, > >> It clearly is for the author's use case. Other use cases will differ. > > You're assuming facts not in evidence. Fujita-san posted that patch not > because he had any use case one way or the other, but because he read > something in fdwhandler.sgml that made him think it was required to > avoid planner malfunctions. (Actually it is not, at the moment, since > we don't do any optimizations based on NOT NULL properties; but we might > in future.) The question on the table is precisely whether believing a > contrary-to-fact NOT NULL assertion would constitute planner malfunction > or user error. +1 for user error. I think at some point I had taken the view that perhaps the FDW should check the data it's emitting against the NOT NULL constraints, but that would imply that we ought to cross-check CHECK constraints as well, once we have those, which sounds unreasonably expensive. So defining the constraint as a promise by the user seems best. -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On 14/03/2012 16:47, David Fetter wrote: > On Wed, Mar 14, 2012 at 11:29:14AM -0400, Tom Lane wrote: >> David Fetter writes: >>> On Wed, Mar 14, 2012 at 10:27:28AM -0400, Tom Lane wrote: Hm. That opinion seems to me to connect to the recently-posted patch to make contrib/file_fdw enforce NOT NULL constraints. Should we instead have the position that constraints declared for foreign tables are statements that we can take on faith, and it's the user's fault if they are wrong? >> >>> I think that's something FDWs need to be able to communicate to >>> PostgreSQL. For example, something talking to another PostgreSQL >>> would (potentially, anyhow) have access to deep knowledge of the >>> remote side, but file_fdw would only have best efforts even for >>> clever things like statistics. >> >> I think we're talking at cross-purposes. What you're saying seems >> to assume that it's the system's responsibility to do something >> about a constraint declared on a foreign table. What I'm suggesting >> is that maybe it isn't. > > Actually, I'm suggesting that this behavior needs to be controlled, > not system-wide, but per FDW, and eventually per server, table and > column. >> A constraint, ordinarily, would be enforced against table *updates*, >> and then just assumed valid during reads. In the case of a foreign >> table, we can't enforce constraints during updates because we don't >> have control of all updates. > > I think that the situation will become a bit more nuanced than that. > A FDW could delegate constraints to the remote side, and in principle, > the remote side could inform PostgreSQL of all types of changes (DML, > DCL, DDL). > >> Should we ignore declared constraints because they're not >> necessarily true? Should we assume on faith that they're true? > > Neither. We should instead have ways for FDWs to say which > constraints are local-only, and which presumed correct on the remote > side. If they lie when asserting the latter, that's pilot error. > I don't understand what value would that bring. Do you propose that, if a FDW declares a constraint as local-only, the planner should ignore them but when declared as remote, it could use this information ? Let me describe a simple use case we have in one of our web applications, which would benefit from foreign keys on foreign tables. The application has users, stored in a users table, which can upload files. The files are stored on the server's filesystem, using one folder per user, named after the user_id. Ex: / 1/ myfile.png 2/ myotherfile.png This filesystem is accessed using the StructuredFS FDW, which maps a file system tree to a set of columns corresponding to parts of the file path: every file which path matches the pattern results in a row. Using the aforementioned structure, the foreign table would have an user_id column, and a filename column. Now, the FDW itself cannot know that the foreign key will be enforced, but as the application developer, I know that every directory will be named after an user_id. Allowing foreign keys on such a foreign table would allow us to describe the model more precisely in PostgreSQL, and external tools could use this knowledge too, even if PostgreSQL completely ignore them. Especially ORMs relying on foreign keys to determine join conditions between tables. On the other hand, should foreign keys referencing a foreign table be allowed too ? From a foreign table to another, from a local table to a foreign table ? Regards, -- Ronan Dunklau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: >> I think that instead of inventing new grammar productions and a new >> node type for this, you should just reuse the existing productions for >> LIKE clauses and then reject invalid options during parse analysis. > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and > submit that as a separate patch? I don't see any reason to do that. I merely meant that you could reuse TableLikeClause or maybe even TableElement in the grammer for CreateForeignTableStmt. >> INCLUDING COMMENTS would be OK, but the the rest are no good. > > At least for now. I can see FDWs in the future that would delegate > the decision to the remote side, and if the remote side happens to be > PostgreSQL, a lot of those delegations could be in force. Of course, > this would either create a dependency that would need to be tracked in > the other node or not be able to guarantee the durability of DDL, the > latter being the current situation. I suspect there would be use > cases for each. What's relevant for LIKE is whether we want to create constraints, defaults, comments, etc. on the *local* side, not the remote side - and that has nothing do with with the particular choice of FDW in use. I don't think we should conflate the local and remote sides. Even if a foreign table refers to a remote table that has comments on its columns, there's no rule that the comments on the foreign side must match up with the comments on the local side, and in fact I think that in general we want to keep those concepts clearly distinct. There's no guarantee that the two systems are controlled by the same DBA, and they might each have their own choice words about those columns. IOW, even if we had the ability to keep those things synchronized, we shouldn't do it, or at least not by default. >> Obviously, we can't enforce constraints on remote data, but the point >> would be allow the system administrator to supply the query planner >> with enough knowledge to make constraint exclusion work. The fact >> that you can't make that work today is a major gap, IMV. > > I didn't do INHERITS because most FDWs won't ever have that concept, > i.e. aren't PostgreSQL. Are you thinking about this as a general way > to handle remote partitioned tables? The original foreign table patch included constraints and the ability to inherit back and forth between regular tables and foreign tables. I ripped all that out before committing because it wasn't sufficiently well thought-out, but I'm not convinced it's something we never want to do. Either way, constraint exclusion can also be used in other scenarios, like a UNION ALL view over several foreign tables. And yes, I am thinking about remote partitioned tables. :-) -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
David Fetter writes: > On Wed, Mar 14, 2012 at 11:29:14AM -0400, Tom Lane wrote: >> The posted patch for file_fdw takes the approach of silently >> filtering out rows for which they're not true, which is not >> obviously the right thing either --- quite aside from whether that's >> a sane semantics, > It clearly is for the author's use case. Other use cases will differ. You're assuming facts not in evidence. Fujita-san posted that patch not because he had any use case one way or the other, but because he read something in fdwhandler.sgml that made him think it was required to avoid planner malfunctions. (Actually it is not, at the moment, since we don't do any optimizations based on NOT NULL properties; but we might in future.) The question on the table is precisely whether believing a contrary-to-fact NOT NULL assertion would constitute planner malfunction or user error. In general, the approach you're sketching towards foreign constraints seems to me to be 100% overdesign with no basis in known user requirements. We have a list longer than my arm of things that are more pressing than doing anything like that. 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 11:29:14AM -0400, Tom Lane wrote: > David Fetter writes: > > On Wed, Mar 14, 2012 at 10:27:28AM -0400, Tom Lane wrote: > >> Hm. That opinion seems to me to connect to the recently-posted > >> patch to make contrib/file_fdw enforce NOT NULL constraints. > >> Should we instead have the position that constraints declared for > >> foreign tables are statements that we can take on faith, and it's > >> the user's fault if they are wrong? > > > I think that's something FDWs need to be able to communicate to > > PostgreSQL. For example, something talking to another PostgreSQL > > would (potentially, anyhow) have access to deep knowledge of the > > remote side, but file_fdw would only have best efforts even for > > clever things like statistics. > > I think we're talking at cross-purposes. What you're saying seems > to assume that it's the system's responsibility to do something > about a constraint declared on a foreign table. What I'm suggesting > is that maybe it isn't. Actually, I'm suggesting that this behavior needs to be controlled, not system-wide, but per FDW, and eventually per server, table and column. > A constraint, ordinarily, would be enforced against table *updates*, > and then just assumed valid during reads. In the case of a foreign > table, we can't enforce constraints during updates because we don't > have control of all updates. I think that the situation will become a bit more nuanced than that. A FDW could delegate constraints to the remote side, and in principle, the remote side could inform PostgreSQL of all types of changes (DML, DCL, DDL). > Should we ignore declared constraints because they're not > necessarily true? Should we assume on faith that they're true? Neither. We should instead have ways for FDWs to say which constraints are local-only, and which presumed correct on the remote side. If they lie when asserting the latter, that's pilot error. > The posted patch for file_fdw takes the approach of silently > filtering out rows for which they're not true, which is not > obviously the right thing either --- quite aside from whether that's > a sane semantics, It clearly is for the author's use case. Other use cases will differ. > it's not going to scale to foreign key constraints, and even for > simple NOT NULL and CHECK constraints it results in a runtime > penalty on selects, which is not what people would expect from a > constraint. If people expect FK constraints on, say, a Twitter feed, they're riding for a very hard fall. If they expect them on a system with several PostgreSQL nodes in it, that could very well be reasonable. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
David Fetter writes: > On Wed, Mar 14, 2012 at 10:27:28AM -0400, Tom Lane wrote: >> Hm. That opinion seems to me to connect to the recently-posted >> patch to make contrib/file_fdw enforce NOT NULL constraints. Should >> we instead have the position that constraints declared for foreign >> tables are statements that we can take on faith, and it's the user's >> fault if they are wrong? > I think that's something FDWs need to be able to communicate to > PostgreSQL. For example, something talking to another PostgreSQL > would (potentially, anyhow) have access to deep knowledge of the > remote side, but file_fdw would only have best efforts even for clever > things like statistics. I think we're talking at cross-purposes. What you're saying seems to assume that it's the system's responsibility to do something about a constraint declared on a foreign table. What I'm suggesting is that maybe it isn't. A constraint, ordinarily, would be enforced against table *updates*, and then just assumed valid during reads. In the case of a foreign table, we can't enforce constraints during updates because we don't have control of all updates. Should we ignore declared constraints because they're not necessarily true? Should we assume on faith that they're true? The posted patch for file_fdw takes the approach of silently filtering out rows for which they're not true, which is not obviously the right thing either --- quite aside from whether that's a sane semantics, it's not going to scale to foreign key constraints, and even for simple NOT NULL and CHECK constraints it results in a runtime penalty on selects, which is not what people would expect from a constraint. 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
David Fetter writes: > I didn't do INHERITS because most FDWs won't ever have that concept, > i.e. aren't PostgreSQL. What's that have to do with it? Inheritance would be a local association of tables, having nothing to do with what the remote end is. IOW, if c inherits from p, that means to scan c as well in "SELECT FROM p". We can do this regardless of whether c or p or both are foreign tables. 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 10:27:28AM -0400, Tom Lane wrote: > Robert Haas writes: > > On Wed, Mar 14, 2012 at 8:28 AM, David Fetter wrote: > >> Here's a WIP patch (lots of cut/paste, no docs, no tests), but it does > >> work. �Still to do in addition: decide whether ALTER FOREIGN TABLE > >> should also handle LIKE. > > > I think that instead of inventing new grammar productions and a new > > node type for this, you should just reuse the existing productions for > > LIKE clauses and then reject invalid options during parse analysis. > > +1; in this approach, adding more features will make it worse not better. OK :) > > I'd actually like to see us allow foreign tables to have constraints. > > Obviously, we can't enforce constraints on remote data, but the point > > would be allow the system administrator to supply the query planner > > with enough knowledge to make constraint exclusion work. The fact > > that you can't make that work today is a major gap, IMV. > > Hm. That opinion seems to me to connect to the recently-posted > patch to make contrib/file_fdw enforce NOT NULL constraints. Should > we instead have the position that constraints declared for foreign > tables are statements that we can take on faith, and it's the user's > fault if they are wrong? I think that's something FDWs need to be able to communicate to PostgreSQL. For example, something talking to another PostgreSQL would (potentially, anyhow) have access to deep knowledge of the remote side, but file_fdw would only have best efforts even for clever things like statistics. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
Robert Haas writes: > On Wed, Mar 14, 2012 at 8:28 AM, David Fetter wrote: >> Here's a WIP patch (lots of cut/paste, no docs, no tests), but it does >> work. Still to do in addition: decide whether ALTER FOREIGN TABLE >> should also handle LIKE. > I think that instead of inventing new grammar productions and a new > node type for this, you should just reuse the existing productions for > LIKE clauses and then reject invalid options during parse analysis. +1; in this approach, adding more features will make it worse not better. > I'd actually like to see us allow foreign tables to have constraints. > Obviously, we can't enforce constraints on remote data, but the point > would be allow the system administrator to supply the query planner > with enough knowledge to make constraint exclusion work. The fact > that you can't make that work today is a major gap, IMV. Hm. That opinion seems to me to connect to the recently-posted patch to make contrib/file_fdw enforce NOT NULL constraints. Should we instead have the position that constraints declared for foreign tables are statements that we can take on faith, and it's the user's fault if they are wrong? 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 08:53:17AM -0400, Robert Haas wrote: > On Wed, Mar 14, 2012 at 8:28 AM, David Fetter wrote: > > On Tue, Mar 13, 2012 at 08:24:47AM -0700, David Fetter wrote: > >> Folks, > >> > >> This is for 9.3, of course. > >> > >> I noticed that CREATE FOREIGN TABLE (LIKE some_table) doesn't work. I > >> believe it should, as it would: > >> > >> - Remove a POLA violation > >> - Make data loading into an extant table even easier, especially if > >> there need to be filtering or other cleanup steps > >> > >> Come to think of it, which CREATE TABLE options are inappropriate to > >> CREATE FOREIGN TABLE? > > > > Here's a WIP patch (lots of cut/paste, no docs, no tests), but it does > > work. Still to do in addition: decide whether ALTER FOREIGN TABLE > > should also handle LIKE. > > I think that instead of inventing new grammar productions and a new > node type for this, you should just reuse the existing productions for > LIKE clauses and then reject invalid options during parse analysis. OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and submit that as a separate patch? > INCLUDING COMMENTS would be OK, but the the rest are no good. At least for now. I can see FDWs in the future that would delegate the decision to the remote side, and if the remote side happens to be PostgreSQL, a lot of those delegations could be in force. Of course, this would either create a dependency that would need to be tracked in the other node or not be able to guarantee the durability of DDL, the latter being the current situation. I suspect there would be use cases for each. > I'd actually like to see us allow foreign tables to have constraints. So would I :) > Obviously, we can't enforce constraints on remote data, but the point > would be allow the system administrator to supply the query planner > with enough knowledge to make constraint exclusion work. The fact > that you can't make that work today is a major gap, IMV. I didn't do INHERITS because most FDWs won't ever have that concept, i.e. aren't PostgreSQL. Are you thinking about this as a general way to handle remote partitioned tables? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Wed, Mar 14, 2012 at 8:28 AM, David Fetter wrote: > On Tue, Mar 13, 2012 at 08:24:47AM -0700, David Fetter wrote: >> Folks, >> >> This is for 9.3, of course. >> >> I noticed that CREATE FOREIGN TABLE (LIKE some_table) doesn't work. I >> believe it should, as it would: >> >> - Remove a POLA violation >> - Make data loading into an extant table even easier, especially if >> there need to be filtering or other cleanup steps >> >> Come to think of it, which CREATE TABLE options are inappropriate to >> CREATE FOREIGN TABLE? > > Here's a WIP patch (lots of cut/paste, no docs, no tests), but it does > work. Still to do in addition: decide whether ALTER FOREIGN TABLE > should also handle LIKE. I think that instead of inventing new grammar productions and a new node type for this, you should just reuse the existing productions for LIKE clauses and then reject invalid options during parse analysis. INCLUDING COMMENTS would be OK, but the the rest are no good. I'd actually like to see us allow foreign tables to have constraints. Obviously, we can't enforce constraints on remote data, but the point would be allow the system administrator to supply the query planner with enough knowledge to make constraint exclusion work. The fact that you can't make that work today is a major gap, IMV. -- 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
Re: [HACKERS] CREATE FOREGIN TABLE LACUNA
On Tue, Mar 13, 2012 at 08:24:47AM -0700, David Fetter wrote:
> Folks,
>
> This is for 9.3, of course.
>
> I noticed that CREATE FOREIGN TABLE (LIKE some_table) doesn't work. I
> believe it should, as it would:
>
> - Remove a POLA violation
> - Make data loading into an extant table even easier, especially if
> there need to be filtering or other cleanup steps
>
> Come to think of it, which CREATE TABLE options are inappropriate to
> CREATE FOREIGN TABLE?
>
> Cheers,
> David.
Here's a WIP patch (lots of cut/paste, no docs, no tests), but it does
work. Still to do in addition: decide whether ALTER FOREIGN TABLE
should also handle LIKE.
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@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
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 5cde225..c634e19 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2727,6 +2727,16 @@ _copyTableLikeClause(const TableLikeClause *from)
return newnode;
}
+static ForeignTableLikeClause *
+_copyForeignTableLikeClause(const ForeignTableLikeClause *from)
+{
+ ForeignTableLikeClause *newnode = makeNode(ForeignTableLikeClause);
+
+ COPY_NODE_FIELD(relation);
+
+ return newnode;
+}
+
static DefineStmt *
_copyDefineStmt(const DefineStmt *from)
{
@@ -4126,6 +4136,9 @@ copyObject(const void *from)
case T_TableLikeClause:
retval = _copyTableLikeClause(from);
break;
+ case T_ForeignTableLikeClause:
+ retval = _copyForeignTableLikeClause(from);
+ break;
case T_DefineStmt:
retval = _copyDefineStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index d2a79eb..55cc2db 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1170,6 +1170,14 @@ _equalTableLikeClause(const TableLikeClause *a, const
TableLikeClause *b)
}
static bool
+_equalForeignTableLikeClause(const ForeignTableLikeClause *a, const
ForeignTableLikeClause *b)
+{
+ COMPARE_NODE_FIELD(relation);
+
+ return true;
+}
+
+static bool
_equalDefineStmt(const DefineStmt *a, const DefineStmt *b)
{
COMPARE_SCALAR_FIELD(kind);
@@ -2685,6 +2693,9 @@ equal(const void *a, const void *b)
case T_TableLikeClause:
retval = _equalTableLikeClause(a, b);
break;
+ case T_ForeignTableLikeClause:
+ retval = _equalForeignTableLikeClause(a, b);
+ break;
case T_DefineStmt:
retval = _equalDefineStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 51181a9..88599ba 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2057,6 +2057,14 @@ _outTableLikeClause(StringInfo str, const
TableLikeClause *node)
}
static void
+_outForeignTableLikeClause(StringInfo str, const ForeignTableLikeClause *node)
+{
+ WRITE_NODE_TYPE("FOREIGNTABLELIKECLAUSE");
+
+ WRITE_NODE_FIELD(relation);
+}
+
+static void
_outLockingClause(StringInfo str, const LockingClause *node)
{
WRITE_NODE_TYPE("LOCKINGCLAUSE");
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index feb28a4..34e5bfc 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -373,7 +373,7 @@ static void processCASbits(int cas_bits, int location,
const char *constrType,
%type set_rest set_rest_more SetResetClause FunctionSetResetClause
%typeTableElement TypedTableElement ConstraintElem TableFuncElement
- ForeignTableElement
+ ForeignTableElement ForeignTableLikeClause
%typecolumnDef columnOptions
%type def_elem reloption_elem old_aggr_elem
%typedef_arg columnElem where_clause where_or_current_clause
@@ -3950,6 +3950,16 @@ ForeignTableElementList:
ForeignTableElement:
columnDef { $$ =
$1; }
+| ForeignTableLikeClause { $$ = $1; }
+ ;
+
+ForeignTableLikeClause:
+ LIKE qualified_name
+ {
+ ForeignTableLikeClause *n =
makeNode(ForeignTableLikeClause);
+ n->relation = $2;
+ $$ = (Node *)n;
+ }
;
/*
diff --git a
[HACKERS] CREATE FOREGIN TABLE LACUNA
Folks, This is for 9.3, of course. I noticed that CREATE FOREIGN TABLE (LIKE some_table) doesn't work. I believe it should, as it would: - Remove a POLA violation - Make data loading into an extant table even easier, especially if there need to be filtering or other cleanup steps Come to think of it, which CREATE TABLE options are inappropriate to CREATE FOREIGN TABLE? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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
