Hi,
On 10/3/23 11:21 AM, Bharath Rupireddy wrote:
On Mon, Oct 2, 2023 at 4:58 PM Drouvot, Bertrand
<bertranddrouvot...@gmail.com> wrote:
On 9/29/23 8:19 AM, Michael Paquier wrote:
On Thu, Sep 28, 2023 at 02:37:02PM +0200, Drouvot, Bertrand wrote:
This patch allows the role provided in BackgroundWorkerInitializeConnection()
and BackgroundWorkerInitializeConnectionByOid() to lack login authorization.
Interesting. Yes, there would be use cases for that, I suppose.
Correct. It allows the roles that don't have LOGIN capabilities to
start and use bg workers.
This may be more adapted with a bits32 for the flags.
Done that way in v2 attached.
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.
"
And I agree with that.
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.
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
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
