From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CREATE FOREGIN TABLE LACUNA |
Date: | 2012-06-23 22:08:31 |
Message-ID: | CAEZATCVdM8Lz0pctXu-fYpAnGLkqKs3UgL2yNBpjB9Jyf9cZGw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 23 March 2012 18:38, David Fetter <david(at)fetter(dot)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 <david(at)fetter(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2012-06-23 22:26:58 | new --maintenance-db options |
Previous Message | Peter Eisentraut | 2012-06-23 21:34:27 | Re: initdb and fsync |