Re: [PATCH v4] Add \warn to psql
On Mon, Jul 8, 2019 at 8:35 PM Bruce Momjian wrote: > On Mon, Jul 8, 2019 at 11:29:00PM -0400, Tom Lane wrote: > > Bruce Momjian writes: > > > On Fri, Jul 5, 2019 at 11:29:03PM +0200, David Fetter wrote: > > >>> I fixed that, but I'm wondering if we should back-patch that fix > > >>> or leave the back branches alone. > > > > >> +0.5 for back-patching. > > > > > Uh, if this was done in a major release I am thinking we have to > mention > > > this as an incompatibility, which means we should probably not > backpatch > > > it. > > > > How is "clearly doesn't match the documentation" not a bug? > > Uh, it is a bug, but people might be expecting the existing behavior > without consulting the documentation, and we don't expect people to be > testing minor releases. > > Anyway, it seems to be have been applied only to head so far. > I would leave it at that. Won't Fix for released versions (neither code nor documentation) as we describe the intended usage so people do the right thing (which is highly likely anyway - though something like "\echo :content_to_echo -n" wouldn't surprise me) but those that learned through trial and error only experience a behavior change on a major release as they would expect. This doesn't seem important enough to warrant breaking the general rule. Though I'd give a +1 to v12; at least for me Beta is generally fair game. David J.
Re: [PATCH v4] Add \warn to psql
On Mon, Jul 8, 2019 at 11:29:00PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Fri, Jul 5, 2019 at 11:29:03PM +0200, David Fetter wrote: > >>> I fixed that, but I'm wondering if we should back-patch that fix > >>> or leave the back branches alone. > > >> +0.5 for back-patching. > > > Uh, if this was done in a major release I am thinking we have to mention > > this as an incompatibility, which means we should probably not backpatch > > it. > > How is "clearly doesn't match the documentation" not a bug? Uh, it is a bug, but people might be expecting the existing behavior without consulting the documentation, and we don't expect people to be testing minor releases. Anyway, it seems to be have been applied only to head so far. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [PATCH v4] Add \warn to psql
Bruce Momjian writes: > On Fri, Jul 5, 2019 at 11:29:03PM +0200, David Fetter wrote: >>> I fixed that, but I'm wondering if we should back-patch that fix >>> or leave the back branches alone. >> +0.5 for back-patching. > Uh, if this was done in a major release I am thinking we have to mention > this as an incompatibility, which means we should probably not backpatch > it. How is "clearly doesn't match the documentation" not a bug? regards, tom lane
Re: [PATCH v4] Add \warn to psql
On Fri, Jul 5, 2019 at 11:29:03PM +0200, David Fetter wrote: > > While I was fooling with it I noticed that the existing code for -n > > is buggy. The documentation says clearly that only the first > > argument is a candidate to be -n: > > > > If the first argument is an unquoted -n the > > trailing > > newline is not written. > > > > but the actual implementation allows any argument to be recognized as > > -n: > > > > regression=# \echo this -n should not be -n like this > > this should not be like thisregression=# > > > > I fixed that, but I'm wondering if we should back-patch that fix > > or leave the back branches alone. > > +0.5 for back-patching. Uh, if this was done in a major release I am thinking we have to mention this as an incompatibility, which means we should probably not backpatch it. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: [PATCH v4] Add \warn to psql
On Fri, Jul 05, 2019 at 12:38:02PM -0400, Tom Lane wrote: > I wrote: > > David Fetter writes: > >> [ v7-0001-Add-warn-to-psql.patch ] > > > I took a look at this. I have no quibble with the proposed feature, > > and the implementation is certainly simple enough. But I'm unconvinced > > about the proposed test scaffolding. > > I pushed this with the simplified test methodology. Thanks! > While I was fooling with it I noticed that the existing code for -n > is buggy. The documentation says clearly that only the first > argument is a candidate to be -n: > > If the first argument is an unquoted -n the > trailing > newline is not written. > > but the actual implementation allows any argument to be recognized as > -n: > > regression=# \echo this -n should not be -n like this > this should not be like thisregression=# > > I fixed that, but I'm wondering if we should back-patch that fix > or leave the back branches alone. +0.5 for back-patching. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: [PATCH v4] Add \warn to psql
Fabien COELHO writes: > I agree that using TAP test if another simpler option is available is not > a good move. > However, in the current state, as soon as there is some variation a test > is removed and coverage is lost, but they could be kept if the check could > be against a regexp. I'm fairly suspicious of using TAP tests just to get a regexp match. The thing I don't like about TAP tests for this is that they won't notice if the test case prints extra stuff beyond what you were expecting --- at least, not without care that I don't think we usually take. I've thought for some time that we should steal an idea from MySQL and extend pg_regress so that individual lines of an expected-file could have regexp match patterns rather than being just exact matches. I'm not really sure how to do that without reimplementing diff(1) for ourselves :-(, but that would be a very large step forward if we could find a reasonable implementation. Anyway, my opinion about having TAP test(s) for psql remains that it'll be a good idea as soon as somebody submits a test that adds a meaningful amount of code coverage that way (and the coverage can't be gotten more simply). But we don't need a patch that is just trying to get the camel's nose under the tent. regards, tom lane
Re: [PATCH v4] Add \warn to psql
I wrote: > David Fetter writes: >> [ v7-0001-Add-warn-to-psql.patch ] > I took a look at this. I have no quibble with the proposed feature, > and the implementation is certainly simple enough. But I'm unconvinced > about the proposed test scaffolding. I pushed this with the simplified test methodology. While I was fooling with it I noticed that the existing code for -n is buggy. The documentation says clearly that only the first argument is a candidate to be -n: If the first argument is an unquoted -n the trailing newline is not written. but the actual implementation allows any argument to be recognized as -n: regression=# \echo this -n should not be -n like this this should not be like thisregression=# I fixed that, but I'm wondering if we should back-patch that fix or leave the back branches alone. regards, tom lane
Re: [PATCH v4] Add \warn to psql
Hello Tom, The point is that there would be at least *one* TAP tests so that many other features of psql, although not all, can be tested. [...] Yeah, but the point I was trying to make is that that's mostly down to laziness. Not always. I agree that using TAP test if another simpler option is available is not a good move. However, in the current state, as soon as there is some variation a test is removed and coverage is lost, but they could be kept if the check could be against a regexp. I see no reason that we couldn't be covering a lot of these features in src/test/regress/sql/psql.sql, with far less overhead. The interactive aspects of psql can't be tested that way ... but since this patch doesn't actually provide any way to test those, it's not much of a proof-of-concept. The PoC is checking against a set of regexp instead of expecting an exact output. Ok, it does not solve all possible test scenarii, that is life. IOW, the blocking factor here is not "does src/bin/psql/t/ exist", it's "has somebody written a test that moves the coverage needle meaningfully". I'm not big on adding a bunch of overhead first and just hoping somebody will do something to make it worthwhile later. I do intend to add coverage once a psql TAP test is available, as I have done with pgbench. Ok, some of the changes are still in the long CF queue, but at least pgbench coverage is around 90%. I also intend to direct submitted patches to use the TAP infra when appropriate, instead of "no tests, too bad". -- Fabien.
Re: [PATCH v4] Add \warn to psql
Fabien COELHO writes: > The point is that there would be at least *one* TAP tests so that many > other features of psql, although not all, can be tested. I have been > reviewing quite a few patches without tests because of this lack of > infrastructure, and no one patch is ever going to justify a TAP test on > its own. It has to start somewhere. Currently psql coverage is abysmal, > around 40% of lines & functions are called by the whole non regression > tests, despite the hundreds of psql-relying tests. Yeah, but the point I was trying to make is that that's mostly down to laziness. I see no reason that we couldn't be covering a lot of these features in src/test/regress/sql/psql.sql, with far less overhead. The interactive aspects of psql can't be tested that way ... but since this patch doesn't actually provide any way to test those, it's not much of a proof-of-concept. IOW, the blocking factor here is not "does src/bin/psql/t/ exist", it's "has somebody written a test that moves the coverage needle meaningfully". I'm not big on adding a bunch of overhead first and just hoping somebody will do something to make it worthwhile later. regards, tom lane
Re: [PATCH v4] Add \warn to psql
Hello Tom, I took a look at this. I have no quibble with the proposed feature, and the implementation is certainly simple enough. But I'm unconvinced about the proposed test scaffolding. Spinning up a new PG instance is a *hell* of a lot of overhead to pay for testing something that could be tested as per attached. Admittedly, the attached doesn't positively prove which pipe each output string went down, but that does not strike me as a concern large enough to justify adding a TAP test for. Sure. The point is that there would be at least *one* TAP tests so that many other features of psql, although not all, can be tested. I have been reviewing quite a few patches without tests because of this lack of infrastructure, and no one patch is ever going to justify a TAP test on its own. It has to start somewhere. Currently psql coverage is abysmal, around 40% of lines & functions are called by the whole non regression tests, despite the hundreds of psql-relying tests. Pg is around 80% coverage overall. Basically, I really thing that one psql dedicated TAP test should be added, not for \warn per se, but for other features. I'd be happier about adding TAP infrastructure if it looked like it'd be usable to test some of the psql areas that are unreachable by the existing test methodology, particularly tab-complete.c and prompt.c. But I don't see anything here that looks like it'll work for that. The tab complete and prompt are special interactive cases and probably require special infrastructure to make a test believe it is running against a tty while it is not. The point of this proposal is not to address these special needs, but to lay a basic infra. I don't like what you did to command_checks_all, Yeah, probably my fault, not David. either --- it could hardly say "bolted on after the fact" more clearly if you'd written that in text. If we need an input-stream argument, let's just add it in a rational place and adjust the callers. There aren't that many of 'em, nor has the subroutine been around all that long. I wanted to avoid breaking the function signature of it is used by some external packages. Not caring is an option. -- Fabien.
Re: [PATCH v4] Add \warn to psql
David Fetter writes: > [ v7-0001-Add-warn-to-psql.patch ] I took a look at this. I have no quibble with the proposed feature, and the implementation is certainly simple enough. But I'm unconvinced about the proposed test scaffolding. Spinning up a new PG instance is a *hell* of a lot of overhead to pay for testing something that could be tested as per attached. Admittedly, the attached doesn't positively prove which pipe each output string went down, but that does not strike me as a concern large enough to justify adding a TAP test for. I'd be happier about adding TAP infrastructure if it looked like it'd be usable to test some of the psql areas that are unreachable by the existing test methodology, particularly tab-complete.c and prompt.c. But I don't see anything here that looks like it'll work for that. I don't like what you did to command_checks_all, either --- it could hardly say "bolted on after the fact" more clearly if you'd written that in text. If we need an input-stream argument, let's just add it in a rational place and adjust the callers. There aren't that many of 'em, nor has the subroutine been around all that long. regards, tom lane diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 4bcf0cc..9b4060f 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -4180,6 +4180,29 @@ drop table psql_serial_tab; \pset format aligned \pset expanded off \pset border 1 +-- \echo and allied features +\echo this is a test +this is a test +\echo -n this is a test without newline +this is a test without newline\echo more test +more test +\set foo bar +\echo foo = :foo +foo = bar +\qecho this is a test +this is a test +\qecho -n this is a test without newline +this is a test without newline\qecho more test +more test +\qecho foo = :foo +foo = bar +\warn this is a test +this is a test +\warn -n this is a test without newline +this is a test without newline\warn more test +more test +\warn foo = :foo +foo = bar -- tests for \if ... \endif \if true select 'okay'; diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 26f436a..2bae6c5 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -771,6 +771,24 @@ drop table psql_serial_tab; \pset expanded off \pset border 1 +-- \echo and allied features + +\echo this is a test +\echo -n this is a test without newline +\echo more test +\set foo bar +\echo foo = :foo + +\qecho this is a test +\qecho -n this is a test without newline +\qecho more test +\qecho foo = :foo + +\warn this is a test +\warn -n this is a test without newline +\warn more test +\warn foo = :foo + -- tests for \if ... \endif \if true
Re: [PATCH v4] Add \warn to psql
I guess that you have a verbose ~/.psqlrc.
Can you try with adding -X to psql option when calling psql from the tap
test?
Ah, true. This patch works for me:
diff --git a/src/bin/psql/t/001_psql.pl b/src/bin/psql/t/001_psql.pl
index 32dd43279b..637baa94c9 100644
--- a/src/bin/psql/t/001_psql.pl
+++ b/src/bin/psql/t/001_psql.pl
@@ -20,7 +20,7 @@ sub psql
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($opts, $stat, $in, $out, $err, $name) = @_;
- my @cmd = ('psql', split /\s+/, $opts);
+ my @cmd = ('psql', '-X', split /\s+/, $opts);
$node->command_checks_all(\@cmd, $stat, $out, $err, $name, $in);
return;
}
Please find attached :)
Good. Works for me, even with a verbose .psqlrc. Switched back to ready.
--
Fabien.
Re: [PATCH v4] Add \warn to psql
On Wed, May 01, 2019 at 10:05:44AM +0300, Arthur Zakirov wrote:
> (Unfortunately I accidentally sent my previous two messages using my personal
> email address because of my email client configuration. This address is not
> verified by PostgreSQL.org services and messages didn't reach hackers mailing
> lists, so I recent latest message).
>
> On Tue, Apr 30, 2019 at 4:46 PM Fabien COELHO wrote:
> > > Unfortunately new TAP test doesn't pass on my machine. I'm not good at
> > > Perl
> > > and didn't get the reason of the failure quickly.
> >
> > I guess that you have a verbose ~/.psqlrc.
> >
> > Can you try with adding -X to psql option when calling psql from the tap
> > test?
>
> Ah, true. This patch works for me:
>
> diff --git a/src/bin/psql/t/001_psql.pl b/src/bin/psql/t/001_psql.pl
> index 32dd43279b..637baa94c9 100644
> --- a/src/bin/psql/t/001_psql.pl
> +++ b/src/bin/psql/t/001_psql.pl
> @@ -20,7 +20,7 @@ sub psql
> {
> local $Test::Builder::Level = $Test::Builder::Level + 1;
> my ($opts, $stat, $in, $out, $err, $name) = @_;
> - my @cmd = ('psql', split /\s+/, $opts);
> + my @cmd = ('psql', '-X', split /\s+/, $opts);
> $node->command_checks_all(\@cmd, $stat, $out, $err, $name, $in);
> return;
> }
Please find attached :)
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From 66daae6f73e704ba05dec83b70eebabf9470d768 Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Sun, 21 Apr 2019 11:08:40 -0700
Subject: [PATCH v7] Add \warn to psql
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.20.1"
This is a multi-part message in MIME format.
--2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
- Does what it says on the label
- Adds TAP tests for same
- In passing, expose the -n option for \echo, \qecho, and \warn in \? output
create mode 100755 src/bin/psql/t/001_psql.pl
--2.20.1
Content-Type: text/x-patch; name="v7-0001-Add-warn-to-psql.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v7-0001-Add-warn-to-psql.patch"
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b86764003d..e474efb521 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3220,6 +3220,25 @@ testdb= \setenv LESS -imx4F
+
+\warn text [ ... ]
+
+
+Prints the arguments to the standard error, separated by one
+space and followed by a newline. This can be useful to
+intersperse information in the output of scripts when not polluting
+standard output is desired. For example:
+
+
+= \warn `date`
+Sun Apr 28 21:00:10 PDT 2019
+
+If the first argument is an unquoted -n the trailing
+newline is not written.
+
+
+
+
\watch [ seconds ]
diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore
index c2862b12d6..d324c1c1fa 100644
--- a/src/bin/psql/.gitignore
+++ b/src/bin/psql/.gitignore
@@ -3,3 +3,4 @@
/sql_help.c
/psql
+/tmp_check
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 69bb297fe7..9473ab01cb 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -60,8 +60,15 @@ uninstall:
clean distclean:
rm -f psql$(X) $(OBJS) lex.backup
+ rm -rf tmp_check
# files removed here are supposed to be in the distribution tarball,
# so do not clean them in the clean/distclean rules
maintainer-clean: distclean
rm -f sql_help.h sql_help.c psqlscanslash.c
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 8254d61099..f6fedb45b7 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -319,7 +319,7 @@ exec_command(const char *cmd,
status = exec_command_ef_ev(scan_state, active_branch, query_buf, true);
else if (strcmp(cmd, "ev") == 0)
status = exec_command_ef_ev(scan_state, active_branch, query_buf, false);
- else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0)
+ else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0 || strcmp(cmd, "warn") == 0)
status = exec_command_echo(scan_state, active_branch, cmd);
else if (strcmp(cmd, "elif") == 0)
status = exec_command_elif(scan_state, cstack, query_buf);
@@ -1114,7 +1114,7 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch,
}
/*
- * \echo and \qecho -- echo arguments to stdout or query output
+ * \echo, \qecho, and \warn -- echo arguments to stdout, query output, or stderr
*/
static backslashResult
exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
@@ -1129,6 +1129,8 @@ exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
if (strcmp(cmd, "qecho") == 0)
fout = pset.queryFout;
+
Re: [PATCH v4] Add \warn to psql
(Unfortunately I accidentally sent my previous two messages using my personal
email address because of my email client configuration. This address is not
verified by PostgreSQL.org services and messages didn't reach hackers mailing
lists, so I recent latest message).
On Tue, Apr 30, 2019 at 4:46 PM Fabien COELHO wrote:
> > Unfortunately new TAP test doesn't pass on my machine. I'm not good at Perl
> > and didn't get the reason of the failure quickly.
>
> I guess that you have a verbose ~/.psqlrc.
>
> Can you try with adding -X to psql option when calling psql from the tap
> test?
Ah, true. This patch works for me:
diff --git a/src/bin/psql/t/001_psql.pl b/src/bin/psql/t/001_psql.pl
index 32dd43279b..637baa94c9 100644
--- a/src/bin/psql/t/001_psql.pl
+++ b/src/bin/psql/t/001_psql.pl
@@ -20,7 +20,7 @@ sub psql
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($opts, $stat, $in, $out, $err, $name) = @_;
- my @cmd = ('psql', split /\s+/, $opts);
+ my @cmd = ('psql', '-X', split /\s+/, $opts);
$node->command_checks_all(\@cmd, $stat, $out, $err, $name, $in);
return;
}
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Re: [PATCH v4] Add \warn to psql
About v5: applies, compiles, global & local make check ok, doc gen ok. Very minor comment: \qecho is just before \o in the embedded help, where it should be just after. Sorry I did not see it on the preceding submission. Unfortunately new TAP test doesn't pass on my machine. I'm not good at Perl and didn't get the reason of the failure quickly. I guess that you have a verbose ~/.psqlrc. Can you try with adding -X to psql option when calling psql from the tap test? -- Fabien.
Re: [PATCH v4] Add \warn to psql
Hello David, About v5: applies, compiles, global & local make check ok, doc gen ok. Very minor comment: \qecho is just before \o in the embedded help, where it should be just after. Sorry I did not see it on the preceding submission. Done. Patch v6 applies, compiles, global & local make check ok, doc gen ok. This is okay for me, marked as ready. -- Fabien.
Re: [PATCH v4] Add \warn to psql
On Mon, Apr 29, 2019 at 08:30:18AM +0200, Fabien COELHO wrote:
>
> Hello David,
>
> About v5: applies, compiles, global & local make check ok, doc gen ok.
>
> Very minor comment: \qecho is just before \o in the embedded help, where it
> should be just after. Sorry I did not see it on the preceding submission.
Done.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From c2cc57537124edac1a0ec5ae2d991d94529c8bc0 Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Sun, 21 Apr 2019 11:08:40 -0700
Subject: [PATCH v6] Add \warn to psql
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.20.1"
This is a multi-part message in MIME format.
--2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
- Does what it says on the label
- Adds TAP tests for same
- In passing, expose the -n option for \echo, \qecho, and \warn in \? output
create mode 100644 src/bin/psql/t/001_psql.pl
--2.20.1
Content-Type: text/x-patch; name="v6-0001-Add-warn-to-psql.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v6-0001-Add-warn-to-psql.patch"
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b86764003d..e474efb521 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3220,6 +3220,25 @@ testdb= \setenv LESS -imx4F
+
+\warn text [ ... ]
+
+
+Prints the arguments to the standard error, separated by one
+space and followed by a newline. This can be useful to
+intersperse information in the output of scripts when not polluting
+standard output is desired. For example:
+
+
+= \warn `date`
+Sun Apr 28 21:00:10 PDT 2019
+
+If the first argument is an unquoted -n the trailing
+newline is not written.
+
+
+
+
\watch [ seconds ]
diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore
index c2862b12d6..d324c1c1fa 100644
--- a/src/bin/psql/.gitignore
+++ b/src/bin/psql/.gitignore
@@ -3,3 +3,4 @@
/sql_help.c
/psql
+/tmp_check
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 69bb297fe7..9473ab01cb 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -60,8 +60,15 @@ uninstall:
clean distclean:
rm -f psql$(X) $(OBJS) lex.backup
+ rm -rf tmp_check
# files removed here are supposed to be in the distribution tarball,
# so do not clean them in the clean/distclean rules
maintainer-clean: distclean
rm -f sql_help.h sql_help.c psqlscanslash.c
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 8254d61099..f6fedb45b7 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -319,7 +319,7 @@ exec_command(const char *cmd,
status = exec_command_ef_ev(scan_state, active_branch, query_buf, true);
else if (strcmp(cmd, "ev") == 0)
status = exec_command_ef_ev(scan_state, active_branch, query_buf, false);
- else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0)
+ else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0 || strcmp(cmd, "warn") == 0)
status = exec_command_echo(scan_state, active_branch, cmd);
else if (strcmp(cmd, "elif") == 0)
status = exec_command_elif(scan_state, cstack, query_buf);
@@ -1114,7 +1114,7 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch,
}
/*
- * \echo and \qecho -- echo arguments to stdout or query output
+ * \echo, \qecho, and \warn -- echo arguments to stdout, query output, or stderr
*/
static backslashResult
exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
@@ -1129,6 +1129,8 @@ exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
if (strcmp(cmd, "qecho") == 0)
fout = pset.queryFout;
+ else if (strcmp(cmd, "warn") == 0)
+ fout = stderr;
else
fout = stdout;
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d6d41b51d5..581e51246a 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -206,11 +206,12 @@ slashUsage(unsigned short int pager)
fprintf(output, _("Input/Output\n"));
fprintf(output, _(" \\copy ... perform SQL COPY with data stream to the client host\n"));
- fprintf(output, _(" \\echo [STRING] write string to standard output\n"));
+ fprintf(output, _(" \\echo [-n] [STRING]write string to standard output (-n for no newline)\n"));
fprintf(output, _(" \\i FILEexecute commands from file\n"));
fprintf(output, _(" \\ir FILE as \\i, but relative to location of current script\n"));
fprintf(output, _(" \\o [FILE] send all query results to file or |pipe\n"));
- fprintf(output, _(" \\qecho [STRING]write
Re: [PATCH v4] Add \warn to psql
Hello David, About v5: applies, compiles, global & local make check ok, doc gen ok. Very minor comment: \qecho is just before \o in the embedded help, where it should be just after. Sorry I did not see it on the preceding submission. -- Fabien.
Re: [PATCH v4] Add \warn to psql
On Sun, Apr 28, 2019 at 08:22:09PM +0200, Fabien COELHO wrote:
>
> Hello David,
>
> About v4: applies, compiles, global & local "make check" ok. Doc gen ok.
>
> Code & help look ok.
>
> About the doc: I do not understand why the small program listing contains an
> "\echo :variable".
It no longer does.
> Also, the new entry should probably be between the \w &
> \watch entries instead of between \echo & \ef.
Moved.
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From ede29ec28e833c2c86638bbc08f3400fe6f7f3f9 Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Sun, 21 Apr 2019 11:08:40 -0700
Subject: [PATCH v5] Add \warn to psql
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2.20.1"
This is a multi-part message in MIME format.
--2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
- Does what it says on the label
- Adds TAP tests for same
- In passing, expose the -n option for \echo, \qecho, and \warn in \? output
create mode 100644 src/bin/psql/t/001_psql.pl
--2.20.1
Content-Type: text/x-patch; name="v5-0001-Add-warn-to-psql.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="v5-0001-Add-warn-to-psql.patch"
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b86764003d..e474efb521 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3220,6 +3220,25 @@ testdb= \setenv LESS -imx4F
+
+\warn text [ ... ]
+
+
+Prints the arguments to the standard error, separated by one
+space and followed by a newline. This can be useful to
+intersperse information in the output of scripts when not polluting
+standard output is desired. For example:
+
+
+= \warn `date`
+Sun Apr 28 21:00:10 PDT 2019
+
+If the first argument is an unquoted -n the trailing
+newline is not written.
+
+
+
+
\watch [ seconds ]
diff --git a/src/bin/psql/.gitignore b/src/bin/psql/.gitignore
index c2862b12d6..d324c1c1fa 100644
--- a/src/bin/psql/.gitignore
+++ b/src/bin/psql/.gitignore
@@ -3,3 +3,4 @@
/sql_help.c
/psql
+/tmp_check
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 69bb297fe7..9473ab01cb 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -60,8 +60,15 @@ uninstall:
clean distclean:
rm -f psql$(X) $(OBJS) lex.backup
+ rm -rf tmp_check
# files removed here are supposed to be in the distribution tarball,
# so do not clean them in the clean/distclean rules
maintainer-clean: distclean
rm -f sql_help.h sql_help.c psqlscanslash.c
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 8254d61099..f6fedb45b7 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -319,7 +319,7 @@ exec_command(const char *cmd,
status = exec_command_ef_ev(scan_state, active_branch, query_buf, true);
else if (strcmp(cmd, "ev") == 0)
status = exec_command_ef_ev(scan_state, active_branch, query_buf, false);
- else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0)
+ else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0 || strcmp(cmd, "warn") == 0)
status = exec_command_echo(scan_state, active_branch, cmd);
else if (strcmp(cmd, "elif") == 0)
status = exec_command_elif(scan_state, cstack, query_buf);
@@ -1114,7 +1114,7 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch,
}
/*
- * \echo and \qecho -- echo arguments to stdout or query output
+ * \echo, \qecho, and \warn -- echo arguments to stdout, query output, or stderr
*/
static backslashResult
exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
@@ -1129,6 +1129,8 @@ exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd)
if (strcmp(cmd, "qecho") == 0)
fout = pset.queryFout;
+ else if (strcmp(cmd, "warn") == 0)
+ fout = stderr;
else
fout = stdout;
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d6d41b51d5..bc98f01be7 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -206,11 +206,12 @@ slashUsage(unsigned short int pager)
fprintf(output, _("Input/Output\n"));
fprintf(output, _(" \\copy ... perform SQL COPY with data stream to the client host\n"));
- fprintf(output, _(" \\echo [STRING] write string to standard output\n"));
+ fprintf(output, _(" \\echo [-n] [STRING]write string to standard output (-n for no newline)\n"));
fprintf(output, _(" \\i FILEexecute commands from file\n"));
fprintf(output, _(" \\ir FILE as \\i, but relative to location of current script\n"));
+ fprintf(output, _(" \\qecho [-n] [STRING]
Re: [PATCH v4] Add \warn to psql
Hello David, About v4: applies, compiles, global & local "make check" ok. Doc gen ok. Code & help look ok. About the doc: I do not understand why the small program listing contains an "\echo :variable". Also, the new entry should probably be between the \w & \watch entries instead of between \echo & \ef. -- Fabien.
