[PATCH v1] Add \echo_stderr to psql

ne 21. 4. 2019 v 20:31 odesílatel David Fetter <david@fetter.org> napsal:

Folks,

Any interest in this?

has sense

Pavel

Any interest in this?

Yep, although I'm not sure of the suggested command name. More
suggestions:
\stderr ...
\err ...
\error ...
\warn ...
\warning ...

--
Fabien.

On Sun, Apr 21, 2019 at 09:31:16PM +0200, Fabien COELHO wrote:

Any interest in this?

Yep, although I'm not sure of the suggested command name. More suggestions:
\stderr ...
\err ...
\error ...
\warn ...
\warning ...

Naming Things is one of the two[1]The others are cache coherency and off-by-one -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 hard problems in CS.

I'm happy with whatever the community consensus comes out to be.

Best,
David.

[1]: The others are cache coherency and off-by-one -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

\warn ...
\warning ...

These two seem about the best to me, drawing from the perl warn command.

I suppose we could go the bash &2 route here, but I don't want to.

Hello Corey,

\warn ...
\warning ...

These two seem about the best to me, drawing from the perl warn command.

Yep, I was thinking of perl & gmake. Maybe the 4 letter option is better
because its the same length as "echo".

I suppose we could go the bash &2 route here, but I don't want to.

I agree on this one.

--
Fabien.

On Mon, Apr 22, 2019 at 09:04:08AM +0200, Fabien COELHO wrote:

Hello Corey,

\warn ...
\warning ...

These two seem about the best to me, drawing from the perl warn command.

Yep, I was thinking of perl & gmake. Maybe the 4 letter option is better
because its the same length as "echo".

I suppose we could go the bash &2 route here, but I don't want to.

I agree on this one.

Please find attached v2, name is now \warn.

How might we test this portably?

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

\warn ...
\warning ...

These two seem about the best to me, drawing from the perl warn command.

Yep, I was thinking of perl & gmake. Maybe the 4 letter option is better
because its the same length as "echo".

I suppose we could go the bash &2 route here, but I don't want to.

I agree on this one.

Please find attached v2, name is now \warn.

How might we test this portably?

TAP testing? see pgbench which has tap test which can test stdout & stderr
by calling utility command_checks_all, the same could be done with psql.

--
Fabien.

Hello David,

Please find attached v2, name is now \warn.

Patch applies cleanly, compiles, "make check ok", although there are no
tests. Doc gen ok.

Code is pretty straightforward.

I'd put the commands in alphabetical order (echo, qecho, warn) instead of
e/w/q in the condition.

The -n trick does not appear in the help lines, ISTM that it could fit, so
maybe it could be added, possibly something like:

\echo [-n] [TEXT] write string to stdout, possibly without trailing newline

and same for \warn and \qecho?

How might we test this portably?

Hmmm... TAP tests are expected to be portable. Attached a simple POC,
which could be extended to test many more things which are currently out
of coverage (src/bin/psql stuff is covered around 40% only).

--
Fabien.

On Sat, Apr 27, 2019 at 04:05:20PM +0200, Fabien COELHO wrote:

Hello David,

Please find attached v2, name is now \warn.

Patch applies cleanly, compiles, "make check ok", although there are no
tests. Doc gen ok.

Code is pretty straightforward.

I'd put the commands in alphabetical order (echo, qecho, warn) instead of
e/w/q in the condition.

Done.

The -n trick does not appear in the help lines, ISTM that it could fit, so
maybe it could be added, possibly something like:

\echo [-n] [TEXT] write string to stdout, possibly without trailing newline

and same for \warn and \qecho?

Makes sense, but I put it there just for \echo to keep lines short.

How might we test this portably?

Hmmm... TAP tests are expected to be portable. Attached a simple POC, which
could be extended to test many more things which are currently out of
coverage (src/bin/psql stuff is covered around 40% only).

Thanks for putting this together. I've added this test, and agree that
increasing coverage is important for another patch.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Hello David,

About v3. Applies, compiles, global & local make check are ok. doc gen ok.

I'd put the commands in alphabetical order (echo, qecho, warn) instead of
e/w/q in the condition.

Done.

Cannot see it:

+ else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "warn") == 0 || strcmp(cmd, "qecho") == 0)

The -n trick does not appear in the help lines, ISTM that it could fit, so
maybe it could be added, possibly something like:

\echo [-n] [TEXT] write string to stdout, possibly without trailing newline

and same for \warn and \qecho?

Makes sense, but I put it there just for \echo to keep lines short.

I think that putting together the 3 echo variants help makes sense, but
maybe someone will object about breaking the abc order.

How might we test this portably?

Hmmm... TAP tests are expected to be portable. Attached a simple POC, which
could be extended to test many more things which are currently out of
coverage (src/bin/psql stuff is covered around 40% only).

Thanks for putting this together. I've added this test, and agree that
increasing coverage is important for another patch.

Yep.

--
Fabien.

On Sat, Apr 27, 2019 at 10:09:27PM +0200, Fabien COELHO wrote:

Hello David,

About v3. Applies, compiles, global & local make check are ok. doc gen ok.

I'd put the commands in alphabetical order (echo, qecho, warn) instead of
e/w/q in the condition.

Done.

Cannot see it:

+ else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "warn") == 0 || strcmp(cmd, "qecho") == 0)

My mistake. I didn't think the order in which they were compared
mattered much, but it makes sense on further reflection to keep things
tidy in the code.

The -n trick does not appear in the help lines, ISTM that it could fit, so
maybe it could be added, possibly something like:

\echo [-n] [TEXT] write string to stdout, possibly without trailing newline

and same for \warn and \qecho?

Makes sense, but I put it there just for \echo to keep lines short.

I think that putting together the 3 echo variants help makes sense, but
maybe someone will object about breaking the abc order.

Here's the alphabetical version.

How might we test this portably?

Hmmm... TAP tests are expected to be portable. Attached a simple POC, which
could be extended to test many more things which are currently out of
coverage (src/bin/psql stuff is covered around 40% only).

Thanks for putting this together. I've added this test, and agree that
increasing coverage is important for another patch.

Yep.

Speaking of which, I'd like to see about getting your patch against
Testlib.pm in so more tests of psql can also go in. It's not a new
feature /per se/, and it doesn't break any current scripts, so I'd
make the argument that it's OK for them to go in and possibly even be
back-patched.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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.

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 <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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.

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 <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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.

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.

(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 <coelho@cri.ensmp.fr> 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

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 <coelho@cri.ensmp.fr> 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 <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate