Re: Setting log_connection in connection string doesn't work - Mailing list pgsql-hackers

This patch is from October of 2021.  I don't see any commitfest entry
for it.  Should it be applied?

---------------------------------------------------------------------------

On Wed, Oct 27, 2021 at 11:53:09AM +0900, Kyotaro Horiguchi wrote:
> At Wed, 27 Oct 2021 10:55:31 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> > On Wed, Oct 27, 2021 at 10:24:05AM +0900, Kyotaro Horiguchi wrote:
> > > I don't know. The fact is that it's a superuser-backend variable that
> > > is silently ignored (but acutally seems to be set in the session).
> > > Setting log_disconnection the same way works (of course the impliction
> > > of this is far less significant that the log_connection case).
> > 
> > fe550b2 is the commit that has changed both those parameters to be
> > PGC_SU_BACKEND, with the commit log mentioning the case you are
> > describing.  That would be the area of this thread:
> > https://www.postgresql.org/message-id/20408.1404329822@sss.pgh.pa.us
> 
> Thanks for the pointer.  (I didn't remember of that thread..)
> 
> > As Tom and this thread are saying, there may be a use-case for
> > making log_connections more effective at startup so as superusers
> > could hide their logs at will.  However, honestly, I am not sure that
> > this is worth spending time improving this as the use-case looks
> > rather thin to me.  Perhaps you are right and we could just mark both
> 
> I tend to agree.
> 
> > of those GUCs as PGC_SIGHUP, making the whole easier to understand and
> > more consistent, though.  If we do that, the patch is wrong, as the
> > docs would also need a refresh.
> 
> Yeah, this is the full version of the patch.
> 
> regards.
> 
> -- 
> Kyotaro Horiguchi
> NTT Open Source Software Center

> >From 11a9612c2590f57f431c3918d5b62c08a5b29efb Mon Sep 17 00:00:00 2001
> From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
> Date: Wed, 27 Oct 2021 11:39:02 +0900
> Subject: [PATCH] Change log_(dis)connections to PGC_SIGHUP
> 
> log_connections is not effective when it is given in connection
> options. Since no complaint has been heard for this behavior the
> use-case looks rather thin.  Thus we change it to PGC_SIGHUP, rahther
> than putting efforts to make it effective for the
> use-case. log_disconnections is working with the usage but be
> consistent by treating it the same way with log_connection.
> ---
>  doc/src/sgml/config.sgml     | 8 ++++----
>  src/backend/utils/misc/guc.c | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
> index de77f14573..64b04a47d2 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -6800,8 +6800,8 @@ local0.*    /var/log/postgresql
>          Causes each attempted connection to the server to be logged,
>          as well as successful completion of both client authentication (if
>          necessary) and authorization.
> -        Only superusers can change this parameter at session start,
> -        and it cannot be changed at all within a session.
> +        This parameter can only be set in the <filename>postgresql.conf</filename>
> +        file or on the server command line.
>          The default is <literal>off</literal>.
>         </para>
>  
> @@ -6827,8 +6827,8 @@ local0.*    /var/log/postgresql
>          Causes session terminations to be logged.  The log output
>          provides information similar to <varname>log_connections</varname>,
>          plus the duration of the session.
> -        Only superusers can change this parameter at session start,
> -        and it cannot be changed at all within a session.
> +        This parameter can only be set in the <filename>postgresql.conf</filename>
> +        file or on the server command line.
>          The default is <literal>off</literal>.
>         </para>
>        </listitem>
> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index e91d5a3cfd..57d810c80d 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -1353,7 +1353,7 @@ static struct config_bool ConfigureNamesBool[] =
>          NULL, NULL, NULL
>      },
>      {
> -        {"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
> +        {"log_connections", PGC_SIGHUP, LOGGING_WHAT,
>              gettext_noop("Logs each successful connection."),
>              NULL
>          },
> @@ -1362,7 +1362,7 @@ static struct config_bool ConfigureNamesBool[] =
>          NULL, NULL, NULL
>      },
>      {
> -        {"log_disconnections", PGC_SU_BACKEND, LOGGING_WHAT,
> +        {"log_disconnections", PGC_SIGHUP, LOGGING_WHAT,
>              gettext_noop("Logs end of a session, including duration."),
>              NULL
>          },
> -- 
> 2.27.0
> 


-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson