On Thu, Jul 21, 2022 at 05:39:35PM -0700, Gurjeet Singh wrote: > One notable side effect of this change is that > process_session_preload_libraries() is now called _before_ we > SetProcessingMode(NormalProcessing). Which means any database access > performed by _PG_init() of an extension will be doing it in > InitProcessing mode. I'm not sure if that's problematic.
I cannot see a reason why on top of my mind. The restrictions of InitProcessing apply to two code paths of bgworkers connecting to a database, and normal processing is used as a barrier to prevent the creation of some objects. > The patch also adds an assertion in pg_parameter_aclcheck() to ensure > that there's a transaction is in progress before it's called. + /* It's pointless to call this function, unless we're in a transaction. */ + Assert(IsTransactionState()); This can involve extension code, I think that this should be at least an elog(ERROR) so as we have higher chances of knowing if something still goes wrong in the wild. > The patch now lets the user connect, throws a warning, and does not crash. > > $ PGOPTIONS="-c plperl.on_plperl_init=" psql -U test > WARNING: permission denied to set parameter "plperl.on_plperl_init" > Expanded display is used automatically. > psql (15beta1) > Type "help" for help. I am wondering whether we'd better have a test on this one with a non-superuser. Except for a few tests in the unsafe section, session_preload_libraries has a limited amount of coverage. + /* + * process any libraries that should be preloaded at backend start (this + * can't be done until GUC settings are complete). Note that these libraries + * can declare new GUC variables. + */ + process_session_preload_libraries(); There is no point in doing that during bootstrap anyway, no? -- Michael
signature.asc
Description: PGP signature
