On Thu, Oct 9, 2008 at 03:06, ITAGAKI Takahiro <[EMAIL PROTECTED]> wrote: > Thanks for your reviewing, Alex. > I applied your comments to my patch.
Sorry for the late reply! Somehow I missed this, saw it on the commit fest wiki :) >> *custom_guc_flags-0828.patch >> My only other concern is the changes to DefineCustom*() to tag the new >> flags param. Now I think anyone who uses Custom gucs will want/should >> be able to set that. I did not see any people in contrib using it but >> did not look on PGfoundry. Do we need to document the change >> somewhere for people who might be using it??? > > Now it is done with DefineCustomVariable(type, variable) and keep > existing functions as-is for backward compatibility. Ok that seems better... > Some people will be happy if the functions are documented, > but we need to define 'stable-internal-functions' between > SPI (stable expoted functions) and unstable internal functions. Right, thats why I was asking :) >> *auto_explalin.c: >> init_instrument() >> The only "cleaner" way I can >> see is to add a hook for CreateQueryDesc so we can overload >> doInstrument and ExecInitNode will InstrAlloc them all for us. > > I wanted to avoid modifying core codes as far as possible, > but I see it was ugly. Now I added 'force_instrument' global > variable as a hook for CreateQueryDesc. Yeah, well if we are not to worried about it getting out of sync when people add new node/scan types what you had before was probably ok. I was just trying to stimulate my own and maybe others brains who are on the list that might have better ideas. But at least now the commiter has 2 options here :) >> the only other comment I have is suset_assign() do we really need to >> be a superuser if its all going to LOG ? There was some concern about >> explaining security definer functions right? but surely a regular >> explain on those shows the same thing as this explain? Or what am I >> missing? > > Almost logging options in postgres are only for superusers. So I think > auto_explain options should not be modified by non-superusers, too. Ok thanks that makes sense. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
