On Tue, Oct 3, 2023 at 5:45 PM Drouvot, Bertrand <bertranddrouvot...@gmail.com> wrote: > > > While I like the idea of the flag to skip login checks for bg workers, > > I don't quite like the APIs being changes InitializeSessionUserId and > > InitPostgres (adding a new input parameter), > > BackgroundWorkerInitializeConnection and > > BackgroundWorkerInitializeConnectionByOid (changing of input parameter > > type) given that all of these functions are available for external > > modules and will break things for sure. > > > > What if BGWORKER_BYPASS_ROLELOGINCHECK be part of bgw_flags? With > > this, none of the API needs to be changed, so no compatibility > > problems as such for external modules and the InitializeSessionUserId > > can just do something like [1]. We might be tempted to add > > BGWORKER_BYPASS_ALLOWCONN also to bgw_flags, but I'd prefer not to do > > it for the same compatibility reasons. > > > > Thoughts? > > > > Thanks for looking at it! > > I did some research and BGWORKER_BYPASS_ALLOWCONN has been added in > eed1ce72e1 and > at that time the bgw_flags did already exist. > > In this the related thread [1], Tom mentioned: > > " > We change exported APIs in new major versions all the time. As > long as it's just a question of an added parameter, people can deal > with it. > "
It doesn't have to be always/all the time. If the case here is okay to change the bgw and other core functions API, I honestly feel that we must move BGWORKER_BYPASS_ALLOWCONN to bgw_flags. > Now, I understand your point but it looks to me that bgw_flags is more > about the capabilities (Access to shared memory with BGWORKER_SHMEM_ACCESS > or ability to establish database connection with > BGWORKER_BACKEND_DATABASE_CONNECTION), > > While with BGWORKER_BYPASS_ROLELOGINCHECK (and BGWORKER_BYPASS_ALLOWCONN) > it's more related to > the BGW behavior once the capability is in place. I look at the new flag as a capability of the bgw to connect with a role without login access. IMV, all are the same. > So, I think I'm fine with the current proposal and don't see the need to move > BGWORKER_BYPASS_ROLELOGINCHECK in bgw_flags. > > [1]: https://www.postgresql.org/message-id/22769.1519323861%40sss.pgh.pa.us I prefer to have it as bgw_flag, however, let's hear from others. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com