Re: [HACKERS] Loading the PL/pgSQL debugger (and other plugins)

[korry]

Thanks for the quick feedback. 1) I think the most straightforward way to load an instrumentation plugin is to define a new custom GUC variable (using the custom_variable_classes mechanism). This seems a bit messy and special-purpose. Agreed, I'm not crazy about using a custom_variable_class variable either. I see no good reason to tie it to plpgsql; we'll just need another one for every other language. Hmmm... but the plugins themselves would be language-specific.  I can't imagine that a plugin (say a profiler) for PL/python would work for PL/pgSQL.  It seems to me that, even if we come up with a common mechanism, we'll still need a separate GUC variable *name* for each PL.  Or am I not understanding something?  Can you post an example of what you are thinking (what would such a GUC variable look like)? IMHO what we want is something with similar properties to preload_libraries, but processed on a per-backend basis instead of once at postmaster start. (You could almost just tell people to select the plugin they want by LOADing it, but that is hard to use if you're trying to debug a non-interactive application. A GUC variable can be set for an app without much cooperation from the app.) Agreed. When the plugin's shared library gets loaded, one way or the other, it should construct the function-pointer struct and then pass it to a function defined by plpgsql (this lets us hide/postpone the decision about whether there can be more than one active plugin). But there's a timing issue there.  If you ask the plugin to call a call-handler function, then you can't load the plugin at backend startup because the PL/pgSQL call-handler isn't loaded until it's required.  Since both the plugin and the call-handler are dynamically loaded, I think one of them has to load the other.  We already have a mechanism for loading call-handlers on demand - it seems kind of messy to introduce another mechanism for loading plugins (that in turn load the call-handlers). The PL/pgSQL call-handler has a convenient initialization function that could read the GUC variable and load the referenced plugin (that's what I'm doing right now). What I'm thinking is that the plpgsql_init() function would look something like this (my changes in red); PLpgSQL_plugin   pluginHooks; typedef void (*plugin_loader_func)(PLpgSQL_plugin *hooks); void plpgsql_init(void) {     static char * pluginName;     plugin_load_func   plugin_loader();     /* Do initialization only once */     if (!plpgsql_firstcall)         return;     plpgsql_HashTableInit();     RegisterXactCallback(plpgsql_xact_cb, NULL);     plpgsql_firstcall = false;     /* Load any instrumentation plugins */     DefineCustomStringVariable( "plpgsql.plugin",                                 "Name of instrumentation plugin to use when PL/pgSQL function is invoked",                                 NULL,                                 &pluginName,                                 PGC_USERSET,                                 NULL,                                 NULL );     EmitWarningsOnPlaceholders("plpgsql");     if (pluginName )     {         plugin_loader = (plugin_loader_func *)load_external_function(pluginName, "plugin_loader", false, NULL );         if (plugin_loader)             (*plugin_loader)(&pluginHooks);     } }    (Ignore the custom variable stuff for now) Each plugin would export a plugin_loader() function - that function, given a pointer to a PLpgSQL_plugin structure, would fill in that structure with the required function pointers. One issue that needs to be thought about with either this proposal or your original is what permissions are needed to set the GUC variable. I don't think we dare allow non-superusers to specify LOADing of arbitrary shared libraries, so there has to be some filter function. Perhaps a better way is that the GUC variable specifies a (list of) initialization functions to call at backend start, and then the superuserness is involved with installing the init functions into pg_proc, and the GUC variable itself needs no special permissions. Again, a plugin's init function would just register its function-pointer struct with plpgsql. You're right, privileges are an issue.  Is it safe enough if we force all plugins to reside in $libdir?  Each plugin could enforce additional security as needed that way, but you'd have to hold enough privileges to get your plugin into $libdir to begin with so you can't write your own nasty plugin to gain more privileges than you ought to have. We should also think about a deregistration function. This would allow you to turn debugging on and off within an interactive session. The GUC variable is really only for coercing non-interactive applications into being debuggable --- I don't see it as being important for interactive debugging, as compared to just "select plugin_init();" ... Ok. 3) Any comments on the PLpgSQL_plugin structure? Should it include (as it's first member) a structure version number so we can add to/change the structure as needed? Given our current plans for enforcing recompiles at major version changes (via magic-block checking), I'm not sure I see a need for this. That makes a lot more sense - I don't like the idea of each plugin managing its own version information.  We can always add more function pointers to the end of the plugin structure - if the pointers are non-NULL, you gain more functionality. 4) Do we need to support multiple active plugins? Probably, but let's fix the API to hide this, so we don't have to commit now. Cool.           -- Korry