On 23 March 2012 18:38, David Fetter <da...@fetter.org> 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 <da...@fetter.org> 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
