[PATCH] Connection time for \conninfo

Started by Rodrigo Ramírez Norambuenaover 6 years ago18 messages
#1Rodrigo Ramírez Norambuena
decipher.hk@gmail.com
1 attachment(s)

I've work in the a little patch to add into the \conninfo of psql
command the connection time against a server backend

Maybe could add after, the precheck to if the connection is alive.
I've take first look to the pqPacketSend, my idea is send a ECHO
packet over to the database connection. If someone has a better idea
it would be great to know.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

Attachments:

0001-Add-to-the-conninfo-for-time-of-current-connection.patchtext/x-patch; charset=US-ASCII; name=0001-Add-to-the-conninfo-for-time-of-current-connection.patchDownload
From e43fa7055af61262a95edcb7f18df1de8e4cf593 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Ram=C3=ADrez=20Norambuena?= <a@rodrigoramirez.com>
Date: Tue, 3 Sep 2019 17:15:39 -0400
Subject: [PATCH] Add to  the \conninfo for time of current connection:

Add extra information for \conninfo psql command to show the current
time lapsed of the connection.

The time will be reestablished again if the connection is reset and
reconnected successfully with the backend.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c0a7a5566e..d1d249e5db 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -263,6 +263,32 @@ HandleSlashCmds(PsqlScanState scan_state,
 }
 
 
+static void
+printTimeConnected(void)
+{
+
+	int hours = 0 , minutes = 0 , seconds = 0;
+	int secs = time(NULL) - pset.connected;
+
+	#define SECOND (1)
+	#define MINUTE (SECOND * 60)
+	#define HOUR (MINUTE * 60)
+
+	if (secs > HOUR) {
+		hours = (secs / HOUR);
+		secs -= (hours * HOUR);
+	}
+	if (secs > MINUTE) {
+		minutes = (secs / MINUTE);
+		secs -= (minutes * MINUTE);
+	}
+	if (secs > 0)
+		seconds = secs;
+
+	printf(_("The time connection is %d hours, %d minutes and %d seconds.\n"),
+			hours, minutes, seconds);
+}
+
 /*
  * Subroutine to actually try to execute a backslash command.
  *
@@ -624,6 +650,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
 			}
 			printSSLInfo();
 			printGSSInfo();
+			printTimeConnected();
 		}
 	}
 
@@ -3318,6 +3345,7 @@ SyncVariables(void)
 	pset.encoding = PQclientEncoding(pset.db);
 	pset.popt.topt.encoding = pset.encoding;
 	pset.sversion = PQserverVersion(pset.db);
+	pset.connected =  time(NULL);
 
 	SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
 	SetVariable(pset.vars, "USER", PQuser(pset.db));
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 44a782478d..c9d4eaf0e7 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -408,7 +408,10 @@ CheckConnection(void)
 			UnsyncVariables();
 		}
 		else
+		{
+			pset.connected = time(NULL);
 			fprintf(stderr, _("Succeeded.\n"));
+		}
 	}
 
 	return OK;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..ed3ab368f5 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -141,6 +141,7 @@ typedef struct _psqlSettings
 	const char *prompt3;
 	PGVerbosity verbosity;		/* current error verbosity level */
 	PGContextVisibility show_context;	/* current context display level */
+	int connected; /* unixtime for connected init time */
 } PsqlSettings;
 
 extern PsqlSettings pset;
-- 
2.21.0

#2Michael Paquier
michael@paquier.xyz
In reply to: Rodrigo Ramírez Norambuena (#1)
Re: [PATCH] Connection time for \conninfo

On Tue, Sep 03, 2019 at 10:13:57PM -0400, Rodrigo Ramírez Norambuena wrote:

I've work in the a little patch to add into the \conninfo of psql
command the connection time against a server backend

Maybe could add after, the precheck to if the connection is alive.
I've take first look to the pqPacketSend, my idea is send a ECHO
packet over to the database connection. If someone has a better idea
it would be great to know.

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp. Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.
--
Michael

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#2)
Re: [PATCH] Connection time for \conninfo

On 2019-Sep-04, Michael Paquier wrote:

On Tue, Sep 03, 2019 at 10:13:57PM -0400, Rodrigo Ram�rez Norambuena wrote:

I've work in the a little patch to add into the \conninfo of psql
command the connection time against a server backend

Maybe could add after, the precheck to if the connection is alive.
I've take first look to the pqPacketSend, my idea is send a ECHO
packet over to the database connection. If someone has a better idea
it would be great to know.

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp. Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.

Hmm, couldn't you say the same about the other details that \conninfo
gives? You can get them from pg_stat_activity or pg_stat_ssl, yet
they're still shown \conninfo for convenience. Surely we don't want to
remove everything from \conninfo and tell users to query the stat views
instead.

The only thing that seems wrong about this proposal is that the time
format is a bit too verbose. I would have it do "N days 12:23:34".

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Rodrigo Ramírez Norambuena
decipher.hk@gmail.com
In reply to: Michael Paquier (#2)
Re: [PATCH] Connection time for \conninfo

On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp. Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.

Hi Michael,

Thanks you for paying attention, I appreciate this.

What are you say seams little trick to what I propose in this patch
because you'll need filter what is your connection, and if the
connection is through the connection pooler could be more.

The main goal this is only getting information from the client side
(psql) as frontend.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

#5Rodrigo Ramírez Norambuena
decipher.hk@gmail.com
In reply to: Alvaro Herrera (#3)
1 attachment(s)
Re: [PATCH] Connection time for \conninfo

On Wed, Sep 4, 2019 at 11:04 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

The only thing that seems wrong about this proposal is that the time
format is a bit too verbose. I would have it do "N days 12:23:34".

Hi Alvaro!,

I attach a second version with this change.

--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

Attachments:

0001-Add-to-the-conninfo-for-time-of-current-connection-v2.patchtext/x-patch; charset=US-ASCII; name=0001-Add-to-the-conninfo-for-time-of-current-connection-v2.patchDownload
From 3526b58e836f92da6c5da6b105c6748948b99365 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Ram=C3=ADrez=20Norambuena?= <a@rodrigoramirez.com>
Date: Tue, 3 Sep 2019 17:15:39 -0400
Subject: [PATCH] Add to  the \conninfo for time of current connection:

Add extra information for \conninfo psql command to show the current
time lapsed of the connection.

The time will be reestablished again if the connection is reset and
reconnected successfully with the backend.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c0a7a5566e..8b0e87211d 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -263,6 +263,42 @@ HandleSlashCmds(PsqlScanState scan_state,
 }
 
 
+static void
+printTimeConnected(void)
+{
+
+	int days = 0, hours = 0 , minutes = 0 , seconds = 0;
+	int secs = time(NULL) - pset.connected;
+
+	#define SECOND (1)
+	#define MINUTE (SECOND * 60)
+	#define HOUR (MINUTE * 60)
+	#define DAY (HOUR * 24)
+
+	if (secs > DAY) {
+		days = (secs / DAY);
+		secs -= (days * DAY);
+	}
+	if (secs > HOUR) {
+		hours = (secs / HOUR);
+		secs -= (hours * HOUR);
+	}
+	if (secs > MINUTE) {
+		minutes = (secs / MINUTE);
+		secs -= (minutes * MINUTE);
+	}
+	if (secs > 0)
+		seconds = secs;
+
+	if (days > 0)
+		printf(ngettext("The time connection is %d day %02d:%02d:%02d\n",
+				"The time connection is %d days %02d:%02d:%02d\n", days),
+				days, hours, minutes, seconds);
+	else
+		printf(_("The time connection is %02d:%02d:%02d\n"),
+			hours, minutes, seconds);
+}
+
 /*
  * Subroutine to actually try to execute a backslash command.
  *
@@ -624,6 +660,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
 			}
 			printSSLInfo();
 			printGSSInfo();
+			printTimeConnected();
 		}
 	}
 
@@ -3318,6 +3355,7 @@ SyncVariables(void)
 	pset.encoding = PQclientEncoding(pset.db);
 	pset.popt.topt.encoding = pset.encoding;
 	pset.sversion = PQserverVersion(pset.db);
+	pset.connected =  time(NULL);
 
 	SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
 	SetVariable(pset.vars, "USER", PQuser(pset.db));
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 44a782478d..c9d4eaf0e7 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -408,7 +408,10 @@ CheckConnection(void)
 			UnsyncVariables();
 		}
 		else
+		{
+			pset.connected = time(NULL);
 			fprintf(stderr, _("Succeeded.\n"));
+		}
 	}
 
 	return OK;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..ed3ab368f5 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -141,6 +141,7 @@ typedef struct _psqlSettings
 	const char *prompt3;
 	PGVerbosity verbosity;		/* current error verbosity level */
 	PGContextVisibility show_context;	/* current context display level */
+	int connected; /* unixtime for connected init time */
 } PsqlSettings;
 
 extern PsqlSettings pset;
-- 
2.21.0

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Rodrigo Ramírez Norambuena (#4)
Re: [PATCH] Connection time for \conninfo

=?UTF-8?Q?Rodrigo_Ram=C3=ADrez_Norambuena?= <decipher.hk@gmail.com> writes:

On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp. Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.

What are you say seams little trick to what I propose in this patch
because you'll need filter what is your connection, and if the
connection is through the connection pooler could be more.
The main goal this is only getting information from the client side
(psql) as frontend.

A couple of thoughts on this ---

* Michael's right that it's *possible* to get the session start time in
SQL, but having to find one's session's entry in pg_stat_activity is
pretty awkward. I wonder if we should invent pg_backend_start_time()
analogous to pg_backend_pid(). Most of the other columns in
pg_stat_activity either already have similar functions (e.g. CURRENT_USER)
or don't seem especially useful to be able to retrieve for one's own
session, but this one maybe is. That's somewhat orthogonal to the
proposed patch but we should probably be considering it in the same
discussion.

* It's not quite clear what the use-case is for displaying this in
\conninfo. That's easy for humans to look at, but I don't really
understand why a human would care, and it's just about useless for
any programmatic application.

* I wonder why the proposal is to display duration of connection rather
than start time. I don't especially like the idea that \conninfo
would change every time you look at it, so I'd tend to lean to the
latter, but maybe there are other arguments for one or the other.

* The patch appears to be tracking time measured at the client side,
which is going to be just enough different from the server's idea of
the session start time to be confusing/annoying. Do we really want
to do it that way?

* There are other definitional questions like what happens when you
reconnect (either due to intentional \connect or momentary connection
loss), or what should happen when you have no connection at all thanks
to a non-recoverable connection loss. I suppose the patch's code
provides some answers but I'm not sure that it's consistent or well
thought out. (Commit aef362385 provides some precedent you should look
at in this regard, plus I think this patch has to be rebased over it
anyway.)

BTW, the result type of time(2) is not "int", so this is just wrong:

+ int connected; /* unixtime for connected init time */

This field name is pretty poorly chosen as well; it sounds like
it's a boolean "am I connected" state, and there's certainly no
hint that it's a time value.

regards, tom lane

#7Rodrigo Ramírez Norambuena
decipher.hk@gmail.com
In reply to: Tom Lane (#6)
1 attachment(s)
Re: [PATCH] Connection time for \conninfo

On Thu, Sep 5, 2019 at 2:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

=?UTF-8?Q?Rodrigo_Ram=C3=ADrez_Norambuena?= <decipher.hk@gmail.com> writes:

On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp. Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.

What are you say seams little trick to what I propose in this patch
because you'll need filter what is your connection, and if the
connection is through the connection pooler could be more.
The main goal this is only getting information from the client side
(psql) as frontend.

A couple of thoughts on this ---
[...]

Hi Tom, about your concerns and feedback I send a new proposal of
patch related with the original idea.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

Attachments:

0001-Add-to-the-conninfo-start-time-current-connection.patchtext/x-patch; charset=US-ASCII; name=0001-Add-to-the-conninfo-start-time-current-connection.patchDownload
From c71a88d67bfa7f3c7fe2d4a684057f364027e1ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Ram=C3=ADrez=20Norambuena?= <a@rodrigoramirez.com>
Date: Tue, 3 Sep 2019 17:15:39 -0400
Subject: [PATCH] Add to  the \conninfo start time current connection:

Add extra information for \conninfo psql command to show the start time
of the connection.

The time will be reestablished again if the connection is reset and
reconnected successfully with the backend.

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 93953fc8e7..1758e20ad7 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -263,6 +263,19 @@ HandleSlashCmds(PsqlScanState scan_state,
 }
 
 
+static void
+printTimeConnected(void)
+{
+
+	const char *strftime_fmt;
+	char timebuf[128];
+
+	strftime_fmt = "%c";
+	strftime(timebuf, sizeof(timebuf), strftime_fmt, localtime(&pset.connection_start));
+
+	printf(_("The connection started at %s\n"), timebuf);
+}
+
 /*
  * Subroutine to actually try to execute a backslash command.
  *
@@ -624,6 +637,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
 			}
 			printSSLInfo();
 			printGSSInfo();
+			printTimeConnected();
 		}
 	}
 
@@ -3326,6 +3340,7 @@ SyncVariables(void)
 	pset.encoding = PQclientEncoding(pset.db);
 	pset.popt.topt.encoding = pset.encoding;
 	pset.sversion = PQserverVersion(pset.db);
+	pset.connection_start = time(NULL);
 
 	SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
 	SetVariable(pset.vars, "USER", PQuser(pset.db));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..955e696d8e 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -141,6 +141,7 @@ typedef struct _psqlSettings
 	const char *prompt3;
 	PGVerbosity verbosity;		/* current error verbosity level */
 	PGContextVisibility show_context;	/* current context display level */
+	time_t connection_start;	/* unixtime for start time connection */
 } PsqlSettings;
 
 extern PsqlSettings pset;
-- 
2.21.0

#8Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Rodrigo Ramírez Norambuena (#7)
Re: [PATCH] Connection time for \conninfo

My opinion is that this is not particularly useful and not appropriate
to piggy-back onto \conninfo. Connection information including host,
port, database, user name is a well-established concept in PostgreSQL
programs and tools and it contains a delimited set of information.
Knowing what server and what database you are connected to also seems
kind of important. Moreover, this is information that is under control
of the client, so it must be tracked on the client side.

Knowing how long you've been connected on the other hand seems kind of
fundamentally unimportant. If we add that, what's to stop us from
adding other statistics of minor interest such as how many commands
you've run, how many errors there were, etc. The connection time is
already available, and perhaps we should indeed make it a bit easier to
get, but it doesn't need to be a psql command.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#9David Steele
david@pgmasters.net
In reply to: Peter Eisentraut (#8)
Re: [PATCH] Connection time for \conninfo

Hi Rodrigo,

On 2/27/20 4:21 AM, Peter Eisentraut wrote:

My opinion is that this is not particularly useful and not appropriate
to piggy-back onto \conninfo.  Connection information including host,
port, database, user name is a well-established concept in PostgreSQL
programs and tools and it contains a delimited set of information.
Knowing what server and what database you are connected to also seems
kind of important.  Moreover, this is information that is under control
of the client, so it must be tracked on the client side.

Knowing how long you've been connected on the other hand seems kind of
fundamentally unimportant.  If we add that, what's to stop us from
adding other statistics of minor interest such as how many commands
you've run, how many errors there were, etc.  The connection time is
already available, and perhaps we should indeed make it a bit easier to
get, but it doesn't need to be a psql command.

The consensus from the committers (with the exception of Álvaro) seems
to be that this is not a feature we want. FWIW, I agree with the majority.

I'll mark this as rejected on MAR-16 unless Álvaro makes an argument for
committing it.

Regards,
--
-David
david@pgmasters.net

#10Stephen Frost
sfrost@snowman.net
In reply to: David Steele (#9)
Re: [PATCH] Connection time for \conninfo

Greetings,

* David Steele (david@pgmasters.net) wrote:

On 2/27/20 4:21 AM, Peter Eisentraut wrote:

My opinion is that this is not particularly useful and not appropriate to
piggy-back onto \conninfo.  Connection information including host, port,
database, user name is a well-established concept in PostgreSQL programs
and tools and it contains a delimited set of information. Knowing what
server and what database you are connected to also seems kind of
important.  Moreover, this is information that is under control of the
client, so it must be tracked on the client side.

I have to say that I disagree. Wishing to know when you connected to a
server is entirely reasonable and it's also rather clearly under control
of the client (though I don't entirely understand that argument in the
first place).

Knowing how long you've been connected on the other hand seems kind of
fundamentally unimportant.  If we add that, what's to stop us from adding
other statistics of minor interest such as how many commands you've run,
how many errors there were, etc.  The connection time is already
available, and perhaps we should indeed make it a bit easier to get, but
it doesn't need to be a psql command.

Of 'minor' interest, or not, if it's something you'd like to know then
it's pretty handy if there's a way to get that info. This also isn't
inventing a new psql command but rather adding on to one that's clearly
already a 'creature comfort' command as everything included could be
gotten at in other ways.

As for the comparison to other things of minor interest, we do give
users a way to get things like line number (%l in PROMPT), so including
something akin to that certainly doesn't seem out of the question.
Having a '%T' or something in the prompt for connected-at-time seems
like it could certainly be useful too.

Going a bit farther, I wonder if conninfo could be configurable as a
Prompt-like string, that seems like it'd be pretty neat.

Anyway, I don't anticipate having time to do anything with this patch
but I disagree that this is a "we don't want it" kind of thing, rather
we maybe want it, since someone cared enough to write a patch, but the
patch needs work and maybe we want it to look a bit different and be
better defined.

Thanks,

Stephen

#11Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Stephen Frost (#10)
Re: [PATCH] Connection time for \conninfo

On 2020-03-10 18:38, Stephen Frost wrote:

On 2/27/20 4:21 AM, Peter Eisentraut wrote:

My opinion is that this is not particularly useful and not appropriate to
piggy-back onto \conninfo.� Connection information including host, port,
database, user name is a well-established concept in PostgreSQL programs
and tools and it contains a delimited set of information. Knowing what
server and what database you are connected to also seems kind of
important.� Moreover, this is information that is under control of the
client, so it must be tracked on the client side.

I have to say that I disagree. Wishing to know when you connected to a
server is entirely reasonable and it's also rather clearly under control
of the client (though I don't entirely understand that argument in the
first place).

The argument is that the server already knows when the client connected
and that information is already available. So there is no reason for
the client to also track and display that information, other than
perhaps convenience. But the server does not, in general, know what
host and port the client connected to (because of proxying and other
network stuff), so this is something that the client must record itself.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#10)
Re: [PATCH] Connection time for \conninfo

Stephen Frost <sfrost@snowman.net> writes:

Anyway, I don't anticipate having time to do anything with this patch
but I disagree that this is a "we don't want it" kind of thing, rather
we maybe want it, since someone cared enough to write a patch, but the
patch needs work and maybe we want it to look a bit different and be
better defined.

I think Peter's primary argument was that this doesn't belong in
\conninfo, which is about reporting the parameters required to
establish the connection. We have kind of broken that already by
cramming SSL and GSS encryption info into the results, but that
doesn't mean it should become a kitchen-sink listing of anything
anybody says they'd like to know.

Anyway, I think your point is that maybe this should be RWF
not Rejected, and I agree with that.

(I had not looked at the last version of the patch, but now that
I have, I still don't like the fact that it has the client tracking
session start time separately from what the server does. The small
discrepancy that introduces is going to confuse somebody. I see
that there's no documentation update either.)

regards, tom lane

#13Stephen Frost
sfrost@snowman.net
In reply to: Peter Eisentraut (#11)
Re: [PATCH] Connection time for \conninfo

Greetings,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 2020-03-10 18:38, Stephen Frost wrote:

On 2/27/20 4:21 AM, Peter Eisentraut wrote:

My opinion is that this is not particularly useful and not appropriate to
piggy-back onto \conninfo.  Connection information including host, port,
database, user name is a well-established concept in PostgreSQL programs
and tools and it contains a delimited set of information. Knowing what
server and what database you are connected to also seems kind of
important.  Moreover, this is information that is under control of the
client, so it must be tracked on the client side.

I have to say that I disagree. Wishing to know when you connected to a
server is entirely reasonable and it's also rather clearly under control
of the client (though I don't entirely understand that argument in the
first place).

The argument is that the server already knows when the client connected and
that information is already available. So there is no reason for the client
to also track and display that information, other than perhaps convenience.

Yes, it'd be for convenience.

But the server does not, in general, know what host and port the client
connected to (because of proxying and other network stuff), so this is
something that the client must record itself.

I agree that there are some things the client has to track and not ask
the server for, because the server's answers could be different, but I
don't agree that conninfo should only ever include that info (and I
seriously doubt that was the original idea, considering 'database' is
included at least as far back as 1999..).

As a side-note, we should probably update our documentation for psql,
since the description of things under Prompting tends to presume that
there isn't anything between the client and the server (for example,
%> says "The port number at which the database server is listening.",
but it's really "The port number used to connect to the database
server." or something along those lines).

Thanks,

Stephen

#14Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#12)
Re: [PATCH] Connection time for \conninfo

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Anyway, I don't anticipate having time to do anything with this patch
but I disagree that this is a "we don't want it" kind of thing, rather
we maybe want it, since someone cared enough to write a patch, but the
patch needs work and maybe we want it to look a bit different and be
better defined.

I think Peter's primary argument was that this doesn't belong in
\conninfo, which is about reporting the parameters required to
establish the connection. We have kind of broken that already by
cramming SSL and GSS encryption info into the results, but that
doesn't mean it should become a kitchen-sink listing of anything
anybody says they'd like to know.

I could certainly agree with wanting to have a psql command that's "give
me what I need to connect", but that idea and what conninfo actually
returns are pretty distant from each other. For one thing, if I wanted
that from psql, I'd sure hope to get back something that I could
directly use when starting up a new psql session.

Anyway, I think your point is that maybe this should be RWF
not Rejected, and I agree with that.

Ok.

(I had not looked at the last version of the patch, but now that
I have, I still don't like the fact that it has the client tracking
session start time separately from what the server does. The small
discrepancy that introduces is going to confuse somebody. I see
that there's no documentation update either.)

This worries me about as much as I worry that someone's going to be
confused by explain-analyze output vs. \timing. Yes, it happens, and
actually pretty often, but I wouldn't change how it works because
they're two different things, not to mention that if I'm going to be
impacted by the time being off on one of the systems, I'd at least like
to know when my client thought it connected relative to the clock on my
client.

THanks,

Stephen

#15Rodrigo Ramírez Norambuena
decipher.hk@gmail.com
In reply to: Stephen Frost (#14)
Re: [PATCH] Connection time for \conninfo

Hi There!

I forgot about that ... It passed a little time from my new pushed
changes. So go ahead :)

On Tue, Mar 10, 2020 at 3:15 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Anyway, I don't anticipate having time to do anything with this patch
but I disagree that this is a "we don't want it" kind of thing, rather
we maybe want it, since someone cared enough to write a patch, but the
patch needs work and maybe we want it to look a bit different and be
better defined.

I think Peter's primary argument was that this doesn't belong in
\conninfo, which is about reporting the parameters required to
establish the connection. We have kind of broken that already by
cramming SSL and GSS encryption info into the results, but that
doesn't mean it should become a kitchen-sink listing of anything
anybody says they'd like to know.

I could certainly agree with wanting to have a psql command that's "give
me what I need to connect", but that idea and what conninfo actually
returns are pretty distant from each other. For one thing, if I wanted
that from psql, I'd sure hope to get back something that I could
directly use when starting up a new psql session.

Anyway, I think your point is that maybe this should be RWF
not Rejected, and I agree with that.

Ok.

(I had not looked at the last version of the patch, but now that
I have, I still don't like the fact that it has the client tracking
session start time separately from what the server does. The small
discrepancy that introduces is going to confuse somebody. I see
that there's no documentation update either.)

This worries me about as much as I worry that someone's going to be
confused by explain-analyze output vs. \timing. Yes, it happens, and
actually pretty often, but I wouldn't change how it works because
they're two different things, not to mention that if I'm going to be
impacted by the time being off on one of the systems, I'd at least like
to know when my client thought it connected relative to the clock on my
client.

So if the path it set as RWF could push a extra work in there but in
main point what be the address about that.

Thanks to all for you feedback.
Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

#16David Steele
david@pgmasters.net
In reply to: Rodrigo Ramírez Norambuena (#15)
Re: [PATCH] Connection time for \conninfo

On 3/14/20 3:16 PM, Rodrigo Ramírez Norambuena wrote:

I forgot about that ... It passed a little time from my new pushed
changes. So go ahead :)

This patch has been returned with feedback. Please feel free to resubmit
in a future CF when you believe the feedback has been addressed.

Regards,
--
-David
david@pgmasters.net

#17Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: David Steele (#16)
Re: [PATCH] Connection time for \conninfo

On Mon, Mar 16, 2020 at 1:24 PM David Steele <david@pgmasters.net> wrote:

On 3/14/20 3:16 PM, Rodrigo Ramírez Norambuena wrote:

I forgot about that ... It passed a little time from my new pushed
changes. So go ahead :)

This patch has been returned with feedback. Please feel free to resubmit
in a future CF when you believe the feedback has been addressed.

There is a duplicate entry as:

https://commitfest.postgresql.org/27/2423/

Regards,

Juan José Santamaría Flecha

#18David Steele
david@pgmasters.net
In reply to: Juan José Santamaría Flecha (#17)
Re: [PATCH] Connection time for \conninfo

On 3/16/20 10:59 AM, Juan José Santamaría Flecha wrote:

On Mon, Mar 16, 2020 at 1:24 PM David Steele <david@pgmasters.net
<mailto:david@pgmasters.net>> wrote:

On 3/14/20 3:16 PM, Rodrigo Ramírez Norambuena wrote:

I forgot about that ... It passed a little time from my new pushed
changes. So go ahead :)

This patch has been returned with feedback. Please feel free to
resubmit
in a future CF when you believe the feedback has been addressed.

There is a duplicate entry as:

Thanks. I've marked in as withdrawn but in the future you can feel free
to do so yourself.

Regards,
--
-David
david@pgmasters.net