On Wed, Apr 20, 2011 at 7:13 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > "Marko Tiikkaja" <marko.tiikk...@2ndquadrant.com> writes: >> CREATE TABLE IF NOT EXISTS duplicates some constraints if the new table >> isn't created: > > What this means is that the feature was implemented in the wrong place, > ie the short-circuit is after rather than before parse_utilcmds.c splits > up the CREATE TABLE command into multiple primitive actions. > > I have stated previously my opinion that this is a misconceived feature, > and it's now apparent that the implementation is as poorly thought > through as the definition. My recommendation is to revert that patch > altogether. > > (BTW, before anyone suggests that s/continue/break/ would fix it, > I suggest experimenting with a table containing a serial column.)
IIRC, quite a few people voiced support for this feature, so I think that ripping it out because you don't personally like it is not a good solution. That is exactly the sort of thing that gives us a reputation for parochialism. In fact, I believe that the person in response to whose complaint I wrote this patch had exactly that complaint. Having said that, it's not altogether obvious to me what a good fix for this problem would look like. The reason why the check isn't done before transformCreateStmt() is because we don't decide on the creation namespace until we get down to DefineRelation(). We could duplicate that logic prior to calling transformCreateStmt(). That would rely on getting the same answer both times, but apparently we're relying on that anyway in the case of a table with a serial column; else we'd be at risk of putting the sequence and table in different schemas. To avoid information leakage, we'd also need to duplicate the permissions-checking logic that immediately follows, which is a little ugly... but maybe not too ugly, if we encapsulate the whole thing in a function that gets called from multiple places, rather than duplicating the code. In fact, it looks like we already have a bit of information leakage here: rhaas=> create table secret.foo (a serial); NOTICE: CREATE TABLE will create implicit sequence "foo_a_seq2" for serial column "foo.a" ERROR: permission denied for schema secret We can infer the existence of foo_a_seq and foo_a_seq1. It seems like a better solution here would be to derive the creation namespace before we break up the statement into subcommands, and pass the OID around after that. Then we wouldn't be doing it multiple times and hoping to get the same answer, and the permissions checking would be happening at the beginning of the process instead of at some point in the middle, which is really the root cause of the above information leak. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
