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
