On Fri, Aug 6, 2010 at 11:36 AM, Alvaro Herrera <alvhe...@commandprompt.com> wrote: > Excerpts from Robert Haas's message of vie ago 06 11:02:58 -0400 2010: > >> Any comments? (ha ha ha...) > > Interesting idea. The patch looks fine on a quick once-over.
Thanks for taking a look. > Two small > things: this comment > > + /* > + * Databases, tablespaces, and roles are cluster-wide objects, so any > + * comments on those objects are record in the shared pg_shdescription > + * catalog. Comments on all other objects are recorded in > pg_description. > + */ > > says "record" where it should say "recorded". Thanks, good eye. > Also, it strikes me that perhaps the ObjectAddress struct definition > should be moved to the new header file; seems a more natural place for > it (but then, it seems like a lot of files will need to include the new > header, so perhaps that should be left to a subsequent patch). I thought about that, but erred on the side of being conservative and didn't move it. I like the idea, though. > Thirdly, is it just me or just could replace a lot of code in DropFoo > functions with this new auxiliary code? Seems it's just missing > "missing_ok" support ... I am not sure how much code it would save you at the level of the individual DropFoo() functions; I'd have to look through them more carefully. But now that you mention it, what about getting rid of all of the individual parse nodes for drop statements? Right now we have: DropTableSpaceStmt DropFdwStmt DropForeignServerStmt DropUserMappingStmt DropPLangStmt DropRoleStmt DropStmt (table, sequence, view, index, domain, conversion, schema, text search {parser, dictionary, template, configuration} DropPropertyStmt (rules and triggers) DropdbStmt (capitalized differently, just for fun) DropCastStmt RemoveFuncStmt (so you can't find it by grepping for Drop!) RemoveOpClassStmt RemoveOpFamilyStmt It seems like we could probably unify all of these into a single DropStmt, following the same pattern as CommentStmt, although I think perhaps that should be a follow-on patch rather than doing it as part of this one. GRANT also has some code to translate object names to OIDs, which I thought might be able to use this machinery as well, though I haven't really checked whether it makes sense. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers