On 2014-02-18 08:35:35 +0900, Michael Paquier wrote: > On Tue, Feb 18, 2014 at 7:22 AM, Andres Freund <and...@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 <and...@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
