MIME-Version: 1.0
References: 
 <CAN4CZFMmDZMH56O9vb_g7vHqAk8ryWFxBMV19C39PFghENg8kA@mail.gmail.com>
 <CAOYmi+k_et3yXpJ8op71-95j7OYg-kX5bWLgW9YTV_5G7f+O1A@mail.gmail.com>
 <CAOYmi+kivcSnazEJA=KWknd3azGYnU3mMq9SUvht5Zq74qNcYQ@mail.gmail.com>
 <CAN4CZFMKCB2OXPGW0R_hCSu4Gg==B7dBSrv6Mf-YuFcrUncADg@mail.gmail.com>
 <CAOYmi+kCYZ3YiOu+oSv1gVW6LXQaNg4BcEpskYizWgfV1z12kA@mail.gmail.com>
In-Reply-To: 
 <CAOYmi+kCYZ3YiOu+oSv1gVW6LXQaNg4BcEpskYizWgfV1z12kA@mail.gmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Wed, 1 Apr 2026 10:35:18 +0100
Message-ID: 
 <CAN4CZFPNL4xuNAwmD1-TUR-z+C84axb+cdhip26k3YBFYo9r1g@mail.gmail.com>
Subject: Re: [oauth] Split and extend PGOAUTHDEBUG
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFPNL4xuNAwmD1-TUR-z%2BC84axb%2Bcdhip26k3YBFYo9r1g%40mail.gmail.com>
Precedence: bulk

> To keep the brainstorming going, I chose the following for v3, but
> they're by no means settled: ...

The names look good. I'm still not 100% sure about plugin-errors, but
since we already have debug in the variable name, it might be okay.

+/* all safe and unsafe flags, for the legacy UNSAFE behavior */
+#define OAUTHDEBUG_UNSAFE_ALL ((uint32) ~0)

The name of this variable is a bit confusing, it's not only about
unsafe settings. I understand why you added the unsafe to it, but
before checking the value/comment I thought this will be the bitmask
of all unsafe options. On the other hand I don't have a better idea
other than simply using ALL.

+oauth_get_debug_flags(void)
+{
+ uint32 flags = 0;
+ const char *env = getenv("PGOAUTHDEBUG");
...

One of the reasons why I implemented this in a C file is because I
wanted to avoid reparsing and warning spam/duplication. Reparsing
shouldn't be a major issue since this is a debug feature, but this
approach causes a warning to print twice in a few corner cases.

That's probably acceptable, the issue is limited in the current code,
but I wanted to mention it as a limitation, as I don't think there's a
good way to fix this without a .c file.