Re: cleanup log_disconnections() - Mailing list pgsql-patches

On Mon, 3 Oct 2005, Neil Conway wrote:
> Why does the patch change behavior when
>
> - port->remote_port is the empty string

Ooops, this wording change is inconsistent with Log_connections, so I
reverted it. Updated patch is attached.

> - or, when end.tv_sec is negative
>

I can only imagine end.tv_sec becomes negative if the root changes system
time during the connection. We can't prevent this, and it is not our
problem to detect this (in fact, we can't), so just tell the truth
directly. If we detect this here, other places in the code should detect
this as well.

Thanks for pointing out, Qingqing

---

Index: postgres.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.463
diff -u -r1.463 postgres.c
--- postgres.c    26 Sep 2005 15:51:12 -0000    1.463
+++ postgres.c    3 Oct 2005 16:20:01 -0000
@@ -3526,29 +3526,13 @@
 static void
 log_disconnections(int code, Datum arg)
 {
-    Port       *port = MyProcPort;
-    struct timeval end;
-    int            hours,
-                minutes,
-                seconds;
+    Port    *port = MyProcPort;
+    struct    timeval end;
+    int        hours,
+            minutes,
+            seconds;

-    char        session_time[20];
-    char        uname[6 + NAMEDATALEN];
-    char        dbname[10 + NAMEDATALEN];
-    char        remote_host[7 + NI_MAXHOST];
-    char        remote_port[7 + NI_MAXSERV];
-
-    snprintf(uname, sizeof(uname), " user=%s", port->user_name);
-    snprintf(dbname, sizeof(dbname), " database=%s", port->database_name);
-    snprintf(remote_host, sizeof(remote_host), " host=%s",
-             port->remote_host);
-    if (port->remote_port[0])
-        snprintf(remote_port, sizeof(remote_port), " port=%s", port->remote_port);
-    else
-        remote_port[0] = '\0';
-
     gettimeofday(&end, NULL);
-
     if (end.tv_usec < port->session_start.tv_usec)
     {
         end.tv_sec--;
@@ -3557,28 +3541,16 @@
     end.tv_sec -= port->session_start.tv_sec;
     end.tv_usec -= port->session_start.tv_usec;

+    /* for stricter accuracy here we could round - this is close enough */
     hours = end.tv_sec / SECS_PER_HOUR;
     end.tv_sec %= SECS_PER_HOUR;
     minutes = end.tv_sec / SECS_PER_MINUTE;
     seconds = end.tv_sec % SECS_PER_MINUTE;

-    /* if time has gone backwards for some reason say so, or print time */
-
-    if (end.tv_sec < 0)
-        snprintf(session_time, sizeof(session_time), "negative!");
-    else
-
-        /*
-         * for stricter accuracy here we could round - this is close
-         * enough
-         */
-        snprintf(session_time, sizeof(session_time),
-                 "%d:%02d:%02d.%02d",
-                 hours, minutes, seconds, (int) (end.tv_usec / 10000));
-
-    ereport(
-            LOG,
-            (errmsg("disconnection: session time: %s%s%s%s%s",
-                session_time, uname, dbname, remote_host, remote_port)));
-
+    ereport(LOG,
+            (errmsg("disconnection: session time=%d:%02d:%02d.%02d,"
+                    " user=%s, database=%s, host=%s, %s%s",
+                    hours, minutes, seconds, (int) (end.tv_usec / 10000),
+                    port->user_name, port->database_name, port->remote_host,
+                    port->remote_port[0] ? "port=" : "", port->remote_port)));
 }