> On Feb 13, 2026, at 21:13, Zsolt Parragi <[email protected]> wrote:
> 
> These all are good suggestions, attached updated patch.
> 
>> Maybe something like PG_SASL_EXCHANGE_ABANDONED?
> 
> This is the only one I wasn't sure of, I used RESTART because I was
> focusing more on the intention of the server ("please restart
> authentication with this additional information"), and a bit also on
> the idea that later restart could stay even within the same
> connection, both in this case and if we add support for
> reauthentication on token expiration.
> 
> On the other hand I'm not 100% sure how the other two would work, and
> ABANDONED is a better description for the current situation, so I
> adjusted the patch to use that.
> <v3-0001-Improve-OAuth-discovery-logging.patch>

Hi Zsolt,

Thanks for the patch. A few small comments:

1 - commit message
```
SASL/Oauth code, by introducing a new SASL authentication status,
PG_SASL_EXCHANGE_RESTART. The expectation is that authentication
```

Looks like you forgot to update the commit message to change 
PG_SASL_EXCHANGE_RESTART to PG_SASL_EXCHANGE_ABANDONED.

2 - auth-oauth.c
```
                        /* The (failed) handshake is now complete. */
+                       if (ctx->state == OAUTH_STATE_ERROR_DISCOVERY)
+                       {
+                               ctx->state = OAUTH_STATE_FINISHED;
+                               ereport(DEBUG1,
+                                               errmsg("OAuth issuer discovery 
requested"));
+                               return PG_SASL_EXCHANGE_ABANDONED;
+                       }
+
                        ctx->state = OAUTH_STATE_FINISHED;
                        return PG_SASL_EXCHANGE_FAILURE;
```

"ctx->state = OAUTH_STATE_FINISHED;" is duplicated in the “if” and after the 
“if”, so it can be pull up to before the “if”.


Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to