Re: Improve OAuth discovery logging

From: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: Improve OAuth discovery logging
Date: 2026-03-17 05:29:04
Message-ID: CAN4CZFOwmgyv1002=EZTSM__97gX-1fG0Q3Q4Zy=XviVtZPRxg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> As is_log_level_output() returns false against FATAL_CLIENT_ONLY, so that FATAL_CLIENT_ONLY should not reach send_message_to_server_log(). Should we assert edata->elevel != FATAL_CLIENT_ONLY?

Andrey asked the same question upthread, this mirrors how
WARNING_CLIENT_ONLY is implemented.

> As far as I understand, for a programmer error, Assert should be used. Why do we use elog(ERROR) here?

+1, I would also say that for CheckSASLAuth, specifying abandoned is
always required, since the caller can't know when it will result in an
error. So the assert/if should be at the beginning of the function,
not in the error path.

Or instead:

+ /*
+ * "Abandoned" is a SASL-specific state similar to STATUS_EOF, in that we
+ * don't want to generate any server logs. But it's caused by an in-band
+ * client action that requires a server response, not an out-of-band
+ * connection closure, so we can't just proc_exit() like we do with
+ * STATUS_EOF.
+ */
+ bool abandoned = false;
+

Have you considered adding the error level here instead, the same way
as in auth_failed, explicitly defaulted to normal fatal by the caller,
so existing code don't have to change it? That wouldn't need an
SASL-specific explanation or flag in the generic code.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-03-17 05:36:48 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Previous Message Ashutosh Bapat 2026-03-17 05:25:51 Re: Report bytes and transactions actually sent downtream