Request - repeat value of \pset title during \watch interations

Started by David G. Johnstonabout 10 years ago42 messages
#1David G. Johnston
david.g.johnston@gmail.com

When executing a query using \watch in psql the first execution of the
query includes "Title is [...]" when \pset title is in use. Subsequent
executions do not. Once that first display goes off-screen the information
in the title is no longer readily accessible. If using \watch for a
long-running monitoring query it can be helpful to incorporate some context
information into the title.

Does anyone capable agree enough to implement this?

Any suggestions for a better way to accomplish the goal?

I have a watch query whose title incorporates the name of the libpq service
name (via bash variables on Linux) so that I can verify which database it
is monitoring without having to kill the watched command and inspect the
prompt, command line, or process.

Working with 9.3 presently.

Thanks!

David J.

#2Michael Paquier
michael.paquier@gmail.com
In reply to: David G. Johnston (#1)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Jan 11, 2016 at 3:36 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

When executing a query using \watch in psql the first execution of the query
includes "Title is [...]" when \pset title is in use. Subsequent executions
do not. Once that first display goes off-screen the information in the
title is no longer readily accessible. If using \watch for a long-running
monitoring query it can be helpful to incorporate some context information
into the title.

Yeah, this sounds like a good idea to show it at each iteration if the
title is set. I am not sure we would want to treat that as a bug fix
as nothing is broken, it looks more like a new feature.

Any suggestions for a better way to accomplish the goal?

What I have been doing in such cases until now is updating the name of
the terminal tab to identify what was going on.
--
Michael

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Request - repeat value of \pset title during \watch interations

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, Jan 11, 2016 at 3:36 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

When executing a query using \watch in psql the first execution of the query
includes "Title is [...]" when \pset title is in use. Subsequent executions
do not. Once that first display goes off-screen the information in the
title is no longer readily accessible. If using \watch for a long-running
monitoring query it can be helpful to incorporate some context information
into the title.

Yeah, this sounds like a good idea to show it at each iteration if the
title is set.

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

regards, tom lane

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#3)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Jan 11, 2016 at 8:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, Jan 11, 2016 at 3:36 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

When executing a query using \watch in psql the first execution of the

query

includes "Title is [...]" when \pset title is in use. Subsequent

executions

do not. Once that first display goes off-screen the information in the
title is no longer readily accessible. If using \watch for a

long-running

monitoring query it can be helpful to incorporate some context

information

into the title.

Yeah, this sounds like a good idea to show it at each iteration if the
title is set.

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

​The extra line doesn't both me and given the length of the timestamp I
suspect many titles would cause the combined line length to exceed terminal
width and cause wrapping anyway. In my specific case it would though I am
using an abnormally narrow width.

David J.

#5David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#2)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Jan 11, 2016 at 4:14 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Mon, Jan 11, 2016 at 3:36 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

When executing a query using \watch in psql the first execution of the

query

includes "Title is [...]" when \pset title is in use. Subsequent

executions

do not. Once that first display goes off-screen the information in the
title is no longer readily accessible. If using \watch for a

long-running

monitoring query it can be helpful to incorporate some context

information

into the title.

Yeah, this sounds like a good idea to show it at each iteration if the
title is set. I am not sure we would want to treat that as a bug fix
as nothing is broken, it looks more like a new feature.

​I would agree...but wouldn't personally argue against the bug-fix
interpretation.​ Given the nature of watch, that it is used for human
interaction, the odds of it being used in an automation environment - where
a change in layout could have an impact - it highly unlikely.

Any suggestions for a better way to accomplish the goal?

What I have been doing in such cases until now is updating the name of
the terminal tab to identify what was going on.

​Except I run my two monitor queries inside a tmux pane and so cannot
directly give them names. I get the point and probably a window name would
end up being sufficient.

David J.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#4)
Re: Request - repeat value of \pset title during \watch interations

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Jan 11, 2016 at 8:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

​The extra line doesn't both me and given the length of the timestamp I
suspect many titles would cause the combined line length to exceed terminal
width and cause wrapping anyway. In my specific case it would though I am
using an abnormally narrow width.

You speak as though the title will be chosen without any regard for the
context it's used in, which I rather doubt. Wouldn't people pick the
title for a \watch query so that it fits? (In any case they could
force the issue by including a \n in their title...)

regards, tom lane

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#7David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#6)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Jan 11, 2016 at 9:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Jan 11, 2016 at 8:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

​The extra line doesn't both me and given the length of the timestamp I
suspect many titles would cause the combined line length to exceed

terminal

width and cause wrapping anyway. In my specific case it would though I

am

using an abnormally narrow width.

You speak as though the title will be chosen without any regard for the
context it's used in, which I rather doubt. Wouldn't people pick the
title for a \watch query so that it fits? (In any case they could
force the issue by including a \n in their title...)

​True that.

​I don't have a strong opinion either way. Having a single, condensed,
title line would be nice though using two in order to not be cryptic has
its own appeal.

David J.

#8Michael Paquier
michael.paquier@gmail.com
In reply to: David G. Johnston (#7)
1 attachment(s)
Re: Request - repeat value of \pset title during \watch interations

On Tue, Jan 12, 2016 at 1:15 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Mon, Jan 11, 2016 at 9:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Jan 11, 2016 at 8:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

The extra line doesn't both me and given the length of the timestamp I
suspect many titles would cause the combined line length to exceed
terminal
width and cause wrapping anyway. In my specific case it would though I
am
using an abnormally narrow width.

You speak as though the title will be chosen without any regard for the
context it's used in, which I rather doubt. Wouldn't people pick the
title for a \watch query so that it fits? (In any case they could
force the issue by including a \n in their title...)

True that.

I don't have a strong opinion either way. Having a single, condensed, title
line would be nice though using two in order to not be cryptic has its own
appeal.

Just looking at that I just hacked the attached that enforces \n at
the end of the user-defined title (that's easily changeable):
=# \pset title 'foo bar'
Title is "foo bar".
=# select 1;
foo bar
?column?
----------
1
(1 row)
=# \watch 1
foo bar
Watch every 1s Tue Jan 12 13:54:04 2016

?column?
----------
1
(1 row)
--
Michael

Attachments:

psql_watch_title.patchapplication/x-patch; name=psql_watch_title.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..9612c8a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,8 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	bool		 *head_title = NULL;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,6 +3035,18 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title = pg_malloc(strlen(myopt.title) + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+		title = pg_malloc(50);
+
 	for (;;)
 	{
 		int			res;
@@ -3045,8 +3058,13 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
-				 sleep, asctime(localtime(&timer)));
+		if (head_title)
+			snprintf(title, strlen(myopt.title) + 50,
+					 _("%s\nWatch every %lds\t%s"),
+					 head_title, sleep, asctime(localtime(&timer)));
+		else
+			snprintf(title, 50, _("Watch every %lds\t%s"),
+					 sleep, asctime(localtime(&timer)));
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3059,7 +3077,11 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		if (res == 0)
 			break;
 		if (res == -1)
+		{
+			pg_free(title);
+			pg_free(head_title);
 			return false;
+		}
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,6 +3106,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
+	pg_free(title);
+	pg_free(head_title);
 	return true;
 }
 
#9David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#8)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Jan 11, 2016 at 9:55 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Tue, Jan 12, 2016 at 1:15 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Mon, Jan 11, 2016 at 9:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Jan 11, 2016 at 8:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should replace the "Watch every Ns" text by the user-given
title if a title has been set? That would conserve screen space.

The extra line doesn't both me and given the length of the timestamp I
suspect many titles would cause the combined line length to exceed
terminal
width and cause wrapping anyway. In my specific case it would though

I

am
using an abnormally narrow width.

You speak as though the title will be chosen without any regard for the
context it's used in, which I rather doubt. Wouldn't people pick the
title for a \watch query so that it fits? (In any case they could
force the issue by including a \n in their title...)

True that.

I don't have a strong opinion either way. Having a single, condensed,

title

line would be nice though using two in order to not be cryptic has its

own

appeal.

Just looking at that I just hacked the attached that enforces \n at
the end of the user-defined title (that's easily changeable):
=# \pset title 'foo bar'
Title is "foo bar".
=# select 1;
foo bar
?column?
----------
1
(1 row)
=# \watch 1
foo bar
Watch every 1s Tue Jan 12 13:54:04 2016

?column?
----------
1
(1 row)
--
Michael

​On the code side calling it "caption" would make for less confusing
reading since "title" is already defined and "head_title" just doesn't
distinguish enough.

There doesn't seem to be any way to let the user decide - by adding a
newline to the end them-self - so a decision will have to be reached.

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

David J

#10Michael Paquier
michael.paquier@gmail.com
In reply to: David G. Johnston (#9)
Re: Request - repeat value of \pset title during \watch interations

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title
--
Michael

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#11David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#10)
Re: Request - repeat value of \pset title during \watch interations

On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title

​That works. I like having the title immediately above the table.

The other option that came to mind would be to place the time information
after the table display while leaving the title before it. On an output
that requires more vertical space than is available in the terminal one
would no longer have to scroll up to confirm last execution time. If doing
this I'd probably get rid of any logic that attempts to center the time
information on the table and simply leave it left-aligned.

David J.​

#12Michael Paquier
michael.paquier@gmail.com
In reply to: David G. Johnston (#11)
1 attachment(s)
Re: Request - repeat value of \pset title during \watch interations

On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title

That works. I like having the title immediately above the table.

The other option that came to mind would be to place the time information
after the table display while leaving the title before it. On an output
that requires more vertical space than is available in the terminal one
would no longer have to scroll up to confirm last execution time. If doing
this I'd probably get rid of any logic that attempts to center the time
information on the table and simply leave it left-aligned.

OK, attached is an updated patch. How does that look?
--
Michael

Attachments:

psql_watch_title-v2.patchtext/x-diff; charset=US-ASCII; name=psql_watch_title-v2.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..3241d27 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,8 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	bool		 *head_title = NULL;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,6 +3035,18 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title = pg_malloc(strlen(myopt.title) + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+		title = pg_malloc(50);
+
 	for (;;)
 	{
 		int			res;
@@ -3045,8 +3058,13 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
-				 sleep, asctime(localtime(&timer)));
+		if (head_title)
+			snprintf(title, strlen(myopt.title) + 50,
+					 _("Watch every %lds\t%s\n%s"),
+					 sleep, asctime(localtime(&timer)), head_title);
+		else
+			snprintf(title, 50, _("Watch every %lds\t%s"),
+					 sleep, asctime(localtime(&timer)));
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3059,7 +3077,11 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		if (res == 0)
 			break;
 		if (res == -1)
+		{
+			pg_free(title);
+			pg_free(head_title);
 			return false;
+		}
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,6 +3106,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
+	pg_free(title);
+	pg_free(head_title);
 	return true;
 }
 
#13David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#12)
Re: Request - repeat value of \pset title during \watch interations

On Thu, Jan 28, 2016 at 6:01 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <

michael.paquier@gmail.com>

wrote:

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title

That works. I like having the title immediately above the table.

The other option that came to mind would be to place the time information
after the table display while leaving the title before it. On an output
that requires more vertical space than is available in the terminal one
would no longer have to scroll up to confirm last execution time. If

doing

this I'd probably get rid of any logic that attempts to center the time
information on the table and simply leave it left-aligned.

OK, attached is an updated patch. How does that look?

Could you run the following and post the result, please? I have yet to
setup a compiled-from-source installation...

\pset title 'This is a medium length title'
SELECT repeat('a', 100) \watch 5

It seems fine though I'm curious how it plays with various combinations of
narrow and wide outputs on narrow and wide terminals - specifically the
centering​

​aspect that occurs when the table is wider than the Watch/Time content.
The terminal width should just cause wrapping relative to a wide-enough
terminal so really it just the centering aspect when the table is wider
than the header.

​Thanks!

David J.​

#14Michael Paquier
michael.paquier@gmail.com
In reply to: David G. Johnston (#13)
Re: Request - repeat value of \pset title during \watch interations

On Fri, Jan 29, 2016 at 1:55 AM, David G. Johnston wrote:

Could you run the following and post the result, please? I have yet to
setup a compiled-from-source installation...

\pset title 'This is a medium length title'
SELECT repeat('a', 100) \watch 5

It seems fine though I'm curious how it plays with various combinations of
narrow and wide outputs on narrow and wide terminals - specifically the
centering

aspect that occurs when the table is wider than the Watch/Time content.

The

terminal width should just cause wrapping relative to a wide-enough

terminal

so really it just the centering aspect when the table is wider than the
header.

No problem. Here you go:

Watch every 5s Fri Jan 29 13:06:31 2016

This is a medium length title
repeat

------------------------------------------------------------------------------------------------------
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
(1 row)
--
Michael

#15David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#14)
Re: Request - repeat value of \pset title during \watch interations

On Thu, Jan 28, 2016 at 9:07 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Fri, Jan 29, 2016 at 1:55 AM, David G. Johnston wrote:

Could you run the following and post the result, please? I have yet to
setup a compiled-from-source installation...

\pset title 'This is a medium length title'
SELECT repeat('a', 100) \watch 5

It seems fine though I'm curious how it plays with various combinations

of

narrow and wide outputs on narrow and wide terminals - specifically the
centering

aspect that occurs when the table is wider than the Watch/Time content.

The

terminal width should just cause wrapping relative to a wide-enough

terminal

so really it just the centering aspect when the table is wider than the
header.

No problem. Here you go:

Watch every 5s Fri Jan 29 13:06:31 2016

This is a medium length title
repeat

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

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
(1 row)
--
Michael

​Looks good to me.

Thank You!

David J.

#16David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#12)
1 attachment(s)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

Adding -hackers for consideration in the Commitfest.

Thanks!

David J.

Original request by me

/messages/by-id/CAKFQuwZqjz-je3Z=8JDOdYm3JM-N2UL4cuqy5VH8N75E5v1VVQ@mail.gmail.com

When executing a query using \watch in psql the first execution of the
query includes "Title is [...]" when \pset title is in use. Subsequent
executions do not. Once that first display goes off-screen the information
in the title is no longer readily accessible. If using \watch for a
long-running monitoring query it can be helpful to incorporate some context
information into the title.

---------- Forwarded message ----------
From: Michael Paquier <michael.paquier@gmail.com>
Date: Thu, Jan 28, 2016 at 6:01 AM
Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
interations
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, "pgsql-general@postgresql.org" <
pgsql-general@postgresql.org>

On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <

michael.paquier@gmail.com>

wrote:

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title

That works. I like having the title immediately above the table.

The other option that came to mind would be to place the time information
after the table display while leaving the title before it. On an output
that requires more vertical space than is available in the terminal one
would no longer have to scroll up to confirm last execution time. If

doing

this I'd probably get rid of any logic that attempts to center the time
information on the table and simply leave it left-aligned.

​And the example:

OK, attached is an updated patch. How does that look?

Watch every 5s Fri Jan 29 13:06:31 2016

This is a medium length title
repeat

------------------------------------------------------------
------------------------------------------
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
(1 row)

Attachments:

psql_watch_title-v2.patchtext/x-diff; charset=US-ASCII; name=psql_watch_title-v2.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..3241d27 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,8 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	bool		 *head_title = NULL;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,6 +3035,18 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title = pg_malloc(strlen(myopt.title) + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+		title = pg_malloc(50);
+
 	for (;;)
 	{
 		int			res;
@@ -3045,8 +3058,13 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
-				 sleep, asctime(localtime(&timer)));
+		if (head_title)
+			snprintf(title, strlen(myopt.title) + 50,
+					 _("Watch every %lds\t%s\n%s"),
+					 sleep, asctime(localtime(&timer)), head_title);
+		else
+			snprintf(title, 50, _("Watch every %lds\t%s"),
+					 sleep, asctime(localtime(&timer)));
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3059,7 +3077,11 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		if (res == 0)
 			break;
 		if (res == -1)
+		{
+			pg_free(title);
+			pg_free(head_title);
 			return false;
+		}
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,6 +3106,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
+	pg_free(title);
+	pg_free(head_title);
 	return true;
 }
 
#17David G. Johnston
david.g.johnston@gmail.com
In reply to: David G. Johnston (#16)
Re: Request - repeat value of \pset title during \watch interations

Figured out it had to be added to 2016-09...done

On Wed, Mar 9, 2016 at 11:40 PM, David G. Johnston <
david.g.johnston@gmail.com> wrote:

Show quoted text

Adding -hackers for consideration in the Commitfest.

Thanks!

David J.

Original request by me

/messages/by-id/CAKFQuwZqjz-je3Z=8JDOdYm3JM-N2UL4cuqy5VH8N75E5v1VVQ@mail.gmail.com

When executing a query using \watch in psql the first execution of the
query includes "Title is [...]" when \pset title is in use. Subsequent
executions do not. Once that first display goes off-screen the information
in the title is no longer readily accessible. If using \watch for a
long-running monitoring query it can be helpful to incorporate some context
information into the title.

---------- Forwarded message ----------
From: Michael Paquier <michael.paquier@gmail.com>
Date: Thu, Jan 28, 2016 at 6:01 AM
Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
interations
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, "pgsql-general@postgresql.org" <
pgsql-general@postgresql.org>

On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <

michael.paquier@gmail.com>

wrote:

On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

So how about:

+ snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\t%s"),
+  sleep, head_title, asctime(localtime(&timer)));

I would just keep the timestamp and the title separated so what do you
think about that instead?
Watch every Xs $timestamp
$head_title

That works. I like having the title immediately above the table.

The other option that came to mind would be to place the time information
after the table display while leaving the title before it. On an output
that requires more vertical space than is available in the terminal one
would no longer have to scroll up to confirm last execution time. If

doing

this I'd probably get rid of any logic that attempts to center the time
information on the table and simply leave it left-aligned.

​And the example:

OK, attached is an updated patch. How does that look?

Watch every 5s Fri Jan 29 13:06:31 2016

This is a medium length title
repeat

------------------------------------------------------------
------------------------------------------
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
(1 row)

#18David Steele
david@pgmasters.net
In reply to: David G. Johnston (#17)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

On 3/17/16 5:07 PM, David G. Johnston wrote:

Figured out it had to be added to 2016-09...done

Hmm ... this patch is currently marked "needs review" in CF 2016-03. Am
I missing something, should this have been closed?

--
-David
david@pgmasters.net

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Steele (#18)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

David Steele <david@pgmasters.net> writes:

On 3/17/16 5:07 PM, David G. Johnston wrote:

Figured out it had to be added to 2016-09...done

Hmm ... this patch is currently marked "needs review" in CF 2016-03. Am
I missing something, should this have been closed?

The message I saw was post-1-March. If it was in fact submitted in
time for 2016-03, then we owe it a review.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20David Steele
david@pgmasters.net
In reply to: Tom Lane (#19)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

On 3/17/16 7:00 PM, Tom Lane wrote:

David Steele <david@pgmasters.net> writes:

On 3/17/16 5:07 PM, David G. Johnston wrote:

Figured out it had to be added to 2016-09...done

Hmm ... this patch is currently marked "needs review" in CF 2016-03. Am
I missing something, should this have been closed?

The message I saw was post-1-March. If it was in fact submitted in
time for 2016-03, then we owe it a review.

I meant to add the CF record and forgot:

https://commitfest.postgresql.org/9/480

It was added 2016-01-13 by Michael Paquier.

--
-David
david@pgmasters.net

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Steele (#20)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

David Steele <david@pgmasters.net> writes:

On 3/17/16 7:00 PM, Tom Lane wrote:

The message I saw was post-1-March. If it was in fact submitted in
time for 2016-03, then we owe it a review.

I meant to add the CF record and forgot:
https://commitfest.postgresql.org/9/480
It was added 2016-01-13 by Michael Paquier.

OK, so it looks like David's 10-Mar patch was actually just a repost of
Michael's 28-Jan patch, which was already in the queue to be reviewed in
2016-03 (and hasn't yet been). So the addition to 2016-09 was simply
erroneous and should be deleted.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#22Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#21)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

On Fri, Mar 18, 2016 at 8:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

David Steele <david@pgmasters.net> writes:

On 3/17/16 7:00 PM, Tom Lane wrote:

The message I saw was post-1-March. If it was in fact submitted in
time for 2016-03, then we owe it a review.

I meant to add the CF record and forgot:
https://commitfest.postgresql.org/9/480
It was added 2016-01-13 by Michael Paquier.

OK, so it looks like David's 10-Mar patch was actually just a repost of
Michael's 28-Jan patch, which was already in the queue to be reviewed in
2016-03 (and hasn't yet been). So the addition to 2016-09 was simply
erroneous and should be deleted.

My mistake I guess. I should have mentioned as well on this thread
that I registered it.
--
Michael

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#23David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#22)
Re: Request - repeat value of \pset title during \watch interations

On Thursday, March 17, 2016, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Fri, Mar 18, 2016 at 8:16 AM, Tom Lane <tgl@sss.pgh.pa.us
<javascript:;>> wrote:

David Steele <david@pgmasters.net <javascript:;>> writes:

On 3/17/16 7:00 PM, Tom Lane wrote:

The message I saw was post-1-March. If it was in fact submitted in
time for 2016-03, then we owe it a review.

I meant to add the CF record and forgot:
https://commitfest.postgresql.org/9/480
It was added 2016-01-13 by Michael Paquier.

OK, so it looks like David's 10-Mar patch was actually just a repost of
Michael's 28-Jan patch, which was already in the queue to be reviewed in
2016-03 (and hasn't yet been). So the addition to 2016-09 was simply
erroneous and should be deleted.

My mistake I guess. I should have mentioned as well on this thread
that I registered it.

And I didn't look hard enough on the CF site...sorry 'bout that. All good
now and I've learned for next time.

David J.

#24Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#16)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

On Thu, Mar 10, 2016 at 1:40 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

Adding -hackers for consideration in the Commitfest.

I don't much like how this patch uses the arbitrary constant 50 in no
fewer than 5 locations.

Also, it seems like we could arrange for head_title to be "" rather
than NULL when myopt.title is NULL. Then instead of this:

+        if (head_title)
+            snprintf(title, strlen(myopt.title) + 50,
+                     _("Watch every %lds\t%s\n%s"),
+                     sleep, asctime(localtime(&timer)), head_title);
+        else
+            snprintf(title, 50, _("Watch every %lds\t%s"),
+                     sleep, asctime(localtime(&timer)));

...we could just the first branch of that if all the time.

         if (res == -1)
+        {
+            pg_free(title);
+            pg_free(head_title);
             return false;
+        }

Instead of repeating the cleanup code, how about making this break;
then, change the return statement at the bottom of the function to
return (res != -1).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#25Michael Paquier
michael.paquier@gmail.com
In reply to: Robert Haas (#24)
1 attachment(s)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

On Sat, Mar 19, 2016 at 11:42 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Mar 10, 2016 at 1:40 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

Adding -hackers for consideration in the Commitfest.

I don't much like how this patch uses the arbitrary constant 50 in no
fewer than 5 locations.

Also, it seems like we could arrange for head_title to be "" rather
than NULL when myopt.title is NULL. Then instead of this:

+        if (head_title)
+            snprintf(title, strlen(myopt.title) + 50,
+                     _("Watch every %lds\t%s\n%s"),
+                     sleep, asctime(localtime(&timer)), head_title);
+        else
+            snprintf(title, 50, _("Watch every %lds\t%s"),
+                     sleep, asctime(localtime(&timer)));

...we could just the first branch of that if all the time.

OK, why not.

if (res == -1)
+        {
+            pg_free(title);
+            pg_free(head_title);
return false;
+        }

Instead of repeating the cleanup code, how about making this break;
then, change the return statement at the bottom of the function to
return (res != -1).

OK.

And the patch attached gives the following output:
With title:
=# \watch 1
Watch every 1s Sun Mar 20 22:28:38 2016
popo
a
---
1
(1 row)

And without title:
Watch every 1s Sun Mar 20 22:29:31 2016

a
---
1
(1 row)
--
Michael

Attachments:

psql_watch_title-v3.patchbinary/octet-stream; name=psql_watch_title-v3.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index eef6e4b..ce1f688 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,10 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	char		 *head_title = NULL;
+	int			  title_len;
+	int			  res = 0;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,9 +3037,25 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title_len = strlen(myopt.title);
+		title = pg_malloc(title_len + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+	{
+		title_len = 0;
+		title = pg_malloc(50);
+		head_title = pg_strdup("");
+	}
+
 	for (;;)
 	{
-		int			res;
 		time_t		timer;
 		long		i;
 
@@ -3045,8 +3064,9 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
-				 sleep, asctime(localtime(&timer)));
+		snprintf(title, title_len + 50,
+				 _("Watch every %lds\t%s%s"),
+				 sleep, asctime(localtime(&timer)), head_title);
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3056,10 +3076,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * PSQLexecWatch handles the case where we can no longer repeat the
 		 * query, and returns 0 or -1.
 		 */
-		if (res == 0)
+		if (res == 0 || res == -1)
 			break;
-		if (res == -1)
-			return false;
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,7 +3102,9 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
-	return true;
+	pg_free(title);
+	pg_free(head_title);
+	return res == 0;
 }
 
 /*
#26Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#25)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

On Sun, Mar 20, 2016 at 9:31 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

And the patch attached gives the following output:
With title:
=# \watch 1
Watch every 1s Sun Mar 20 22:28:38 2016
popo
a
---
1
(1 row)

And without title:
Watch every 1s Sun Mar 20 22:29:31 2016

a
---
1
(1 row)

And does everybody agree that this is a desirable change?

As for the patch itself, you could replace all this:

+       /*
+        * Take into account any title present in the user setup as a part of
+        * what is printed for each iteration by using it as a header.
+        */
+       if (myopt.title)
+       {
+               title_len = strlen(myopt.title);
+               title = pg_malloc(title_len + 50);
+               head_title = pg_strdup(myopt.title);
+       }
+       else
+       {
+               title_len = 0;
+               title = pg_malloc(50);
+               head_title = pg_strdup("");
+       }

...with:

head_title = pg_strdup(myopt.title != NULL ? myopt.title : "");
title_len = strlen(head_title);
title = pg_malloc(title_len + 50);

Better yet, include the + 50 in title_len, and then you don't need to
reference the number 50 again further down.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#27David G. Johnston
david.g.johnston@gmail.com
In reply to: Robert Haas (#26)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

On Mon, Mar 21, 2016 at 8:03 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sun, Mar 20, 2016 at 9:31 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

And the patch attached gives the following output:
With title:
=# \watch 1
Watch every 1s Sun Mar 20 22:28:38 2016
popo
a
---
1
(1 row)

​This doesn't show the blank line above "popo" that the prior example
had...​

And without title:
Watch every 1s Sun Mar 20 22:29:31 2016

a
---
1
(1 row)

​Unchanged from present behavior - but its not obvious that the watch line
is center-aligned​

And does everybody agree that this is a desirable change?

​Adding the title is desirable. While I'm inclined to bike-shed this
anything that gets it in I can live with and so I'm content letting the
author/committer decide where exactly things (including whitespace) appear.

​It is a bit odd that the "Watch every %s...." gets centered if the result
is wide but that the title remains left-aligned.

The minimally invasive change would be the following:

optional title<
watch<
(blank line)
headers
head-body divider
body
optional footer

Though I like the idea of:

optional title
(blank line - if Title present)
headers
head-body divider
body
optional footer
watch

​David J.​

#28Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#27)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

On Mon, Mar 21, 2016 at 11:17 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

And does everybody agree that this is a desirable change?

Adding the title is desirable. While I'm inclined to bike-shed this
anything that gets it in I can live with and so I'm content letting the
author/committer decide where exactly things (including whitespace) appear.

It is a bit odd that the "Watch every %s...." gets centered if the result is
wide but that the title remains left-aligned.

Well, the title isn't normally centered, but yeah, that is odd. Yeah,
that is odd. Come to think of it, I think I might have expected the
title to appear *above* "Watch every %s", not below it. That might
decrease the oddness.

As for letting the committer decide, I don't care about this
personally at all, so I'm only looking at it to be nice to the people
who do. Whatever is the consensus is OK with me. I just don't want
to get yelled at later for committing something here, so it would be
nice to see a few votes for whatever we're gonna do here.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#28)
1 attachment(s)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

Robert Haas <robertmhaas@gmail.com> writes:

Well, the title isn't normally centered, but yeah, that is odd. Yeah,
that is odd. Come to think of it, I think I might have expected the
title to appear *above* "Watch every %s", not below it. That might
decrease the oddness.

AFAICS, it appears *beside* it with this patch. It's only below if the
terminal is narrow enough that it wraps to there.

As for letting the committer decide, I don't care about this
personally at all, so I'm only looking at it to be nice to the people
who do. Whatever is the consensus is OK with me. I just don't want
to get yelled at later for committing something here, so it would be
nice to see a few votes for whatever we're gonna do here.

I'm still of the opinion that what would make the most sense is to replace
the "Watch every Ns" text with the user-given title, if there is one.
I ran that up the flagpole already and didn't get a lot of salutes, but
it seems to respond to your concern that the user title ought to be first.

Regardless of that, I concur with your complaints about coding style, in
particular with the need to repeat the magic constant 50 in several
places. Also, I think the patch makes do_watch return the wrong result
code for the (typical) case where we exit because of query cancel not
PSQLexecWatch failure.

So on the whole, I'd do it as attached.

regards, tom lane

Attachments:

psql_watch_title-v4.patchtext/x-diff; charset=us-ascii; name=psql_watch_title-v4.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index eef6e4b..a309109 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** static bool
*** 3020,3026 ****
  do_watch(PQExpBuffer query_buf, long sleep)
  {
  	printQueryOpt myopt = pset.popt;
! 	char		title[50];
  
  	if (!query_buf || query_buf->len <= 0)
  	{
--- 3020,3029 ----
  do_watch(PQExpBuffer query_buf, long sleep)
  {
  	printQueryOpt myopt = pset.popt;
! 	const char	 *user_title;
! 	char		 *title;
! 	int			  title_len;
! 	int			  res = 0;
  
  	if (!query_buf || query_buf->len <= 0)
  	{
*************** do_watch(PQExpBuffer query_buf, long sle
*** 3034,3042 ****
  	 */
  	myopt.topt.pager = 0;
  
  	for (;;)
  	{
- 		int			res;
  		time_t		timer;
  		long		i;
  
--- 3037,3055 ----
  	 */
  	myopt.topt.pager = 0;
  
+ 	/*
+ 	 * If there's a title in the user configuration, make sure we have room
+ 	 * for it in the title buffer.
+ 	 */
+ 	user_title = myopt.title;
+ 	if (user_title)
+ 		title_len = strlen(user_title) + 50;
+ 	else
+ 		title_len = 50;
+ 	title = pg_malloc(title_len);
+ 
  	for (;;)
  	{
  		time_t		timer;
  		long		i;
  
*************** do_watch(PQExpBuffer query_buf, long sle
*** 3045,3052 ****
  		 * of completion of the command?
  		 */
  		timer = time(NULL);
! 		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
! 				 sleep, asctime(localtime(&timer)));
  		myopt.title = title;
  
  		/* Run the query and print out the results */
--- 3058,3071 ----
  		 * of completion of the command?
  		 */
  		timer = time(NULL);
! 		if (user_title)
! 			snprintf(title, title_len,
! 					 "%s\t%s",
! 					 user_title, asctime(localtime(&timer)));
! 		else
! 			snprintf(title, title_len,
! 					 _("Watch every %lds\t%s"),
! 					 sleep, asctime(localtime(&timer)));
  		myopt.title = title;
  
  		/* Run the query and print out the results */
*************** do_watch(PQExpBuffer query_buf, long sle
*** 3056,3065 ****
  		 * PSQLexecWatch handles the case where we can no longer repeat the
  		 * query, and returns 0 or -1.
  		 */
! 		if (res == 0)
  			break;
- 		if (res == -1)
- 			return false;
  
  		/*
  		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
--- 3075,3082 ----
  		 * PSQLexecWatch handles the case where we can no longer repeat the
  		 * query, and returns 0 or -1.
  		 */
! 		if (res == 0 || res == -1)
  			break;
  
  		/*
  		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
*************** do_watch(PQExpBuffer query_buf, long sle
*** 3084,3090 ****
  		sigint_interrupt_enabled = false;
  	}
  
! 	return true;
  }
  
  /*
--- 3101,3108 ----
  		sigint_interrupt_enabled = false;
  	}
  
! 	pg_free(title);
! 	return (res >= 0);
  }
  
  /*
#30David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#29)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

On Mon, Mar 21, 2016 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Well, the title isn't normally centered, but yeah, that is odd. Yeah,
that is odd. Come to think of it, I think I might have expected the
title to appear *above* "Watch every %s", not below it. That might
decrease the oddness.

AFAICS, it appears *beside* it with this patch. It's only below if the
terminal is narrow enough that it wraps to there.

As for letting the committer decide, I don't care about this
personally at all, so I'm only looking at it to be nice to the people
who do. Whatever is the consensus is OK with me. I just don't want
to get yelled at later for committing something here, so it would be
nice to see a few votes for whatever we're gonna do here.

I'm still of the opinion that what would make the most sense is to replace
the "Watch every Ns" text with the user-given title, if there is one.
I ran that up the flagpole already and didn't get a lot of salutes, but
it seems to respond to your concern that the user title ought to be first.

Regardless of that, I concur with your complaints about coding style, in
particular with the need to repeat the magic constant 50 in several
places. Also, I think the patch makes do_watch return the wrong result
code for the (typical) case where we exit because of query cancel not
PSQLexecWatch failure.

So on the whole, I'd do it as attached.


I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:

if (user_title)​
​snprintf(title, title_len, "%s\t%s (%ld​s)", user_title,
asctime(localtime(&timer)), sleep)

"""
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

col1
-----
1
"""

David J.

#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#30)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

"David G. Johnston" <david.g.johnston@gmail.com> writes:

I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

Meh ... seems a bit awkward to me. Couldn't you include " (5s)" in the
title, if you want that info? If it's variable, you could still
accommodate that:

regression=# \set delay 5
regression=# \pset title 'My Title (':delay' s)'
Title is "My Title (5 s)".
regression=# select repeat('xyzzy',12) \watch :delay
My Title (5 s) Mon Mar 21 13:39:25 2016

repeat
--------------------------------------------------------------
xyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzy
(1 row)

But I don't care enough to veto it.
Anyone else have an opinion?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#32David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#31)
Re: Request - repeat value of \pset title during \watch interations

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com <javascript:;>> writes:

I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

Meh ... seems a bit awkward to me. Couldn't you include " (5s)" in the
title, if you want that info? If it's variable, you could still
accommodate that:

Actually, only if it's a variable that you setup and repeat and you show.
A bit cumbersome and mixes the parts that are title and those that are
present only because you are watching.

regression=# \set delay 5
regression=# \pset title 'My Title (':delay' s)'
Title is "My Title (5 s)".
regression=# select repeat('xyzzy',12) \watch :delay

David J.

#33Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#32)
Re: Request - repeat value of \pset title during \watch interations

On Mon, Mar 21, 2016 at 2:09 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

Meh ... seems a bit awkward to me. Couldn't you include " (5s)" in the
title, if you want that info? If it's variable, you could still
accommodate that:

Actually, only if it's a variable that you setup and repeat and you show. A
bit cumbersome and mixes the parts that are title and those that are present
only because you are watching.

Ah, come on. This doesn't really seem like an issue we should spend
more time quibbling about. I think Tom's version is fine.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#34David G. Johnston
david.g.johnston@gmail.com
In reply to: Robert Haas (#33)
Re: Request - repeat value of \pset title during \watch interations

On Monday, March 21, 2016, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Mar 21, 2016 at 2:09 PM, David G. Johnston
<david.g.johnston@gmail.com <javascript:;>> wrote:

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us <javascript:;>>

wrote:

"David G. Johnston" <david.g.johnston@gmail.com <javascript:;>> writes:

I'd rather not omit sleep but removing "Watch every" is fine

(preferred

actually), so:
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

Meh ... seems a bit awkward to me. Couldn't you include " (5s)" in the
title, if you want that info? If it's variable, you could still
accommodate that:

Actually, only if it's a variable that you setup and repeat and you

show. A

bit cumbersome and mixes the parts that are title and those that are

present

only because you are watching.

Ah, come on. This doesn't really seem like an issue we should spend
more time quibbling about. I think Tom's version is fine.

Tom doesn't care enough to veto and you don't really care...

I'll admit it's awkward because it's abbreviated but if someone enters
\watch 5 and then sees (5s) in the title I think they can put two and two
together.

If the watched query takes a long to run, or there is a disruption, knowing
when the last one ran and how often it is supposed to run is useful info to
have at ones fingertips. I have done this myself occasionally so I'm not
speaking from theory. But I won't complain if its removed.

David J.

#35Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: David G. Johnston (#34)
Re: Request - repeat value of \pset title during \watch interations

David G. Johnston wrote:

Tom doesn't care enough to veto and you don't really care...

I'll admit it's awkward because it's abbreviated but if someone enters
\watch 5 and then sees (5s) in the title I think they can put two and two
together.

If the watched query takes a long to run, or there is a disruption, knowing
when the last one ran and how often it is supposed to run is useful info to
have at ones fingertips. I have done this myself occasionally so I'm not
speaking from theory. But I won't complain if its removed.

I like David's UI better FWIW.

(I'll also use this opportunity to complain again about not being able
to use floating point sleep time.)

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#36Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#34)
Re: Request - repeat value of \pset title during \watch interations

"David G. Johnston" <david.g.johnston@gmail.com> writes:

I'll admit it's awkward because it's abbreviated but if someone enters
\watch 5 and then sees (5s) in the title I think they can put two and two
together.

Where I find this to be awkward is that the format is randomly different
between the user-title and no-user-title cases.

What about just discarding the old format entirely, and printing one of
these two things:

Timestamp (every Ns)

User Given Title <tab> Timestamp (every Ns)

If the watched query takes a long to run, or there is a disruption, knowing
when the last one ran and how often it is supposed to run is useful info to
have at ones fingertips.

That's not unreasonable. I just want it to look less weirdly different
between the two cases.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#35)
Re: Request - repeat value of \pset title during \watch interations

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

(I'll also use this opportunity to complain again about not being able
to use floating point sleep time.)

That's not unreasonable either, though it seems like material for a
separate patch.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#38David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#36)
Re: Request - repeat value of \pset title during \watch interations

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com <javascript:;>> writes:

I'll admit it's awkward because it's abbreviated but if someone enters
\watch 5 and then sees (5s) in the title I think they can put two and two
together.

Where I find this to be awkward is that the format is randomly different
between the user-title and no-user-title cases.

What about just discarding the old format entirely, and printing one of
these two things:

Timestamp (every Ns)

User Given Title <tab> Timestamp (every Ns)

This works for me.

David J.

#39Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#38)
Re: [GENERAL] Request - repeat value of \pset title during \watch interations

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

What about just discarding the old format entirely, and printing one of
these two things:

Timestamp (every Ns)

User Given Title <tab> Timestamp (every Ns)

This works for me.

If I don't hear objections PDQ, I'm going to update the docs and commit
it like that.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#40Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#39)
Re: [HACKERS] Request - repeat value of \pset title during \watch interations

Tom Lane wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

What about just discarding the old format entirely, and printing one of
these two things:

Timestamp (every Ns)

User Given Title <tab> Timestamp (every Ns)

This works for me.

If I don't hear objections PDQ, I'm going to update the docs and commit
it like that.

It works for me too.

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

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#41Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#39)
Re: Request - repeat value of \pset title during \watch interations

On Tue, Mar 22, 2016 at 6:25 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

If I don't hear objections PDQ, I'm going to update the docs and commit
it like that.

Thanks!
--
Michael

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

#42David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#39)
Re: Request - repeat value of \pset title during \watch interations

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com <javascript:;>> writes:

On Monday, March 21, 2016, Tom Lane <tgl@sss.pgh.pa.us <javascript:;>>

wrote:

What about just discarding the old format entirely, and printing one of
these two things:

Timestamp (every Ns)

User Given Title <tab> Timestamp (every Ns)

This works for me.

If I don't hear objections PDQ, I'm going to update the docs and commit
it like that.

Saw it go in. Thank You.

David J.