Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag

On Tue, Oct 3, 2023 at 5:45 PM Drouvot, Bertrand
<bertranddrouvot(dot)pg(at)gmail(dot)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