Variable substitution in psql backtick expansion

On Fri, Mar 31, 2017 at 2:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Awhile back in the discussion about the \if feature for psql,
I'd pointed out that you shouldn't really need very much in
the way of boolean-expression evaluation smarts, because you
ought to be able to use a backtick shell escape:

\if `expr :foo \> :bar`
\echo :foo is greater than :bar
\endif

Now that the basic feature is in, I went to play around with this,
and was disappointed to find out that it doesn't work. The reason
is not far to seek: we do not do variable substitution within the
text between backticks. psqlscanslash.l already foresaw that some
day we'd want to do that:

/*
* backticked text: copy everything until next backquote, then evaluate.
*
* XXX Possible future behavioral change: substitute for :VARIABLE?
*/

I think today is that day, because it's going to make a material
difference to the usability of this feature.

I propose extending backtick processing so that

1. :VARIABLE is replaced by the contents of the variable

2. :'VARIABLE' is replaced by the contents of the variable,
single-quoted according to Unix shell conventions. (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

This doesn't look like it would take very much new code to do.

Thoughts?

In short, +1.
--
Michael

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

On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

single-quoted according to Unix shell conventions. (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

+1
Having been bit by format '%L' prepending an 'E' to any string that happens
to have a backslash in it, I'm in favor of this difference.

Any reason we wouldn't do :"VARIABLE" as well? People might expect it given
its use elsewhere, and it would make possible things like

SELECT '$HOME/lamentable application name dir/bin/myprog' as myprog \gset
`:"myprog" arg1 arg2`

both for expanding $HOME and keeping the lamentable dir path as one arg.

Tom Lane wrote:

Thoughts?

ISTM that expr is too painful to use to be seen as the
idiomatic way of achieving comparison in psql.

Among its disadvantages, it won't work on windows, and its
interface is hard to work with due to the necessary
quoting of half its operators, and the mandatory spacing
between arguments.

Also the quoting rules and command line syntax
depend on the underlying shell.
Isn't it going to be tricky to produce code that works
across different families of shells, like bourne and csh?

I think that users would rather have the option to just put
an SQL expression behind \if. That implies a working connection
to evaluate, which expr doesn't, but that's no
different from the other backslash commands that read
the database.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

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

2017-03-31 15:00 GMT+02:00 Daniel Verite <daniel@manitou-mail.org>:

Tom Lane wrote:

Thoughts?

ISTM that expr is too painful to use to be seen as the
idiomatic way of achieving comparison in psql.

Among its disadvantages, it won't work on windows, and its
interface is hard to work with due to the necessary
quoting of half its operators, and the mandatory spacing
between arguments.

Also the quoting rules and command line syntax
depend on the underlying shell.
Isn't it going to be tricky to produce code that works
across different families of shells, like bourne and csh?

I think that users would rather have the option to just put
an SQL expression behind \if. That implies a working connection
to evaluate, which expr doesn't, but that's no
different from the other backslash commands that read
the database.

some basic expression can be done on client side like defvar, serverver,
... but generic expression should be evaluated in server - I am not sure,
if it is what we would - when I have PLpgSQL.

In psql scripting I expecting really simple expressions - but it should to
work everywhere - using bash is not good enough.

Regards

Pavel

Corey Huinker <corey.huinker@gmail.com> writes:

On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

single-quoted according to Unix shell conventions. (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

Any reason we wouldn't do :"VARIABLE" as well?

Well, what would that mean exactly? The charter of :'FOO', I think,
is to get the string value through shell parsing unscathed. I'm a
lot less clear on what :"FOO" ought to do. If it has any use then
I'd imagine that that includes allowing $shell_variable references in
the string to become expanded. But then you have a situation where some
shell special characters in the string are supposed to take effect but
others presumably still aren't. Figuring out the exact semantics would
take some specific use-cases, and more time than I really have available
right now.

In short, that's something I thought was best left as a later
refinement, rather than doing a rush job we might regret.

People might expect it given
its use elsewhere, and it would make possible things like

SELECT '$HOME/lamentable application name dir/bin/myprog' as myprog \gset
`:"myprog" arg1 arg2`

You could get about 80% of the way there with `":myprog" arg1 arg2`,
since backtick processing doesn't have any rule that would prevent
:myprog from being expanded inside shell double quotes.

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

"Daniel Verite" <daniel@manitou-mail.org> writes:

ISTM that expr is too painful to use to be seen as the
idiomatic way of achieving comparison in psql.

I'm not proposing it as the best permanent solution, just saying
that having this in v10 is a lot better than having nothing in v10.
And we certainly aren't going to get any more-capable boolean
expression syntax into v10 at this point.

Among its disadvantages, it won't work on windows, and its
interface is hard to work with due to the necessary
quoting of half its operators, and the mandatory spacing
between arguments.
Also the quoting rules and command line syntax
depend on the underlying shell.

All true, but that's true of just about any use of backtick
or \! commands. Shall we remove those features because they
are hard to use 100% portably?

Isn't it going to be tricky to produce code that works
across different families of shells, like bourne and csh?

Probably 95% of our users do not really care about that,
because they're only trying to produce scripts that will
work in their own environment.

I think that users would rather have the option to just put
an SQL expression behind \if. That implies a working connection
to evaluate, which expr doesn't, but that's no
different from the other backslash commands that read
the database.

Well, it also implies being in a non-aborted transaction,
which seems like more of a problem to me for \if scripting.
Certainly there would be value in an option like that, but
it's not any more of a 100% solution than the `expr` one is.

Also, I didn't think I'd have to point it out, but expr(1)
is hardly the only command you can call from a backtick
substitution. There are *lots* of potential use-cases
here ... but not so many if you can't plug any variable
material into the shell command.

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

Corey Huinker <corey.huinker@gmail.com> writes:

On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

single-quoted according to Unix shell conventions. (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

+1

Here's a proposed patch for this. I used the existing appendShellString()
logic, which already knows how to quote stuff safely on both Unix and
Windows. A small problem is that appendShellString() rejects LF and CR
characters. As written, it just printed an error and did exit(1), which
is more or less OK for existing callers but seemed like a pretty bad idea
for psql. I refactored it to get the behavior proposed in the patch,
which is that we print an error and decline to substitute the variable's
value, leading to executing the backtick command with the :'variable'
text still in place. This is more or less the same thing that happens
for encoding-check failures in the other variable-substitution cases,
so it didn't seem too unreasonable.

Perhaps it would be preferable to prevent execution of the backtick
command and/or fail the calling metacommand, but that seems to require
some very invasive refactoring (or else magic global variables), which
I didn't have the appetite for.

regards, tom lane

Bonjour Daniel,

I think that users would rather have the option to just put
an SQL expression behind \if.

Note that this is already available indirectly, as show in the
documentation.

SELECT some-boolean-expression AS okay \gset
\if :okay
\echo boolean expression was true
\else
\echo boolean expression was false
\endif

--
Fabien.

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

2017-04-02 8:53 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Bonjour Daniel,

I think that users would rather have the option to just put

an SQL expression behind \if.

Note that this is already available indirectly, as show in the
documentation.

SELECT some-boolean-expression AS okay \gset
\if :okay
\echo boolean expression was true
\else
\echo boolean expression was false
\endif

For this case can be nice to have function that returns server version as
number

some like version_num() .. 10000

comments?

Regards

Pavel

Hello Pavel,

For this case can be nice to have function that returns server version
as number some like version_num() .. 10000

The server side information can be queried:

SELECT current_setting(‘server_version_num’)
AS server_version_num \gset
-- 90602

However client side is not so clean:

\echo :VERSION
PostgreSQL 10devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

Probably some :VERSION_NUM would make some sense. See attached PoC patch.
Would it make sense?

--
Fabien.

For this case can be nice to have function that returns server version as
number

some like version_num() .. 10000

Another possible trick to get out of a script which does not support \if,
relying on the fact that the unexpected command is simply ignored:

-- exit version below 10
\if false
\echo 'script requires version 10 or better'
\q
\endif

Also possible but less informative on errors:

\set ON_ERROR_STOP on
\if false \endif

--
Fabien.

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

2017-04-02 9:45 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

For this case can be nice to have function that returns server version as

number some like version_num() .. 10000

The server side information can be queried:

SELECT current_setting(‘server_version_num’)
AS server_version_num \gset
-- 90602

However client side is not so clean:

\echo :VERSION
PostgreSQL 10devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

Probably some :VERSION_NUM would make some sense. See attached PoC patch.
Would it make sense?

It has sense

Pavel

2017-04-02 9:45 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

For this case can be nice to have function that returns server version as

number some like version_num() .. 10000

The server side information can be queried:

SELECT current_setting(‘server_version_num’)
AS server_version_num \gset
-- 90602

However client side is not so clean:

\echo :VERSION
PostgreSQL 10devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

Probably some :VERSION_NUM would make some sense. See attached PoC patch.
Would it make sense?

Maybe better name for you CLIENT_VERSION_NUM

Can be SERVER_VERSION_NUM taken from connection info?

Regards

Pavel

Hello Pavel,

\echo :VERSION
PostgreSQL 10devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

Probably some :VERSION_NUM would make some sense. See attached PoC patch.
Would it make sense?

Maybe better name for you CLIENT_VERSION_NUM

If it was starting from nothing I would tend to agree with you, but there
is already an existing :VERSION variable, so it seemed logical to keep on
and create variants with the same prefix.

Can be SERVER_VERSION_NUM taken from connection info?

Probably it could. It seems a little less straightforward than defining a
client-side string at compile time. The information is displayed when the
connection is established, so the information is there somewhere.

psql (10devel, server 9.6.2)

--
Fabien.

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

Can be SERVER_VERSION_NUM taken from connection info?

Here is a version with that, quite easy as well as the information was
already there for other checks.

I have not updated "help.c" because the initial VERSION was not presented
there in the first place.

--
Fabien.

Can be SERVER_VERSION_NUM taken from connection info?

Here is a version with that, quite easy as well as the information was
already there for other checks.

I have not updated "help.c" because the initial VERSION was not presented
there in the first place.

Here is a v3 to fix the documentation example. Sorry for the noise.

--
Fabien.

Fabien COELHO wrote:

Note that this is already available indirectly, as show in the
documentation.

SELECT some-boolean-expression AS okay \gset
\if :okay
\echo boolean expression was true
\else
\echo boolean expression was false
\endif

Yes, the question was whether we leave it as that for v10,
or if it's worth a last-minute improvement for usability,
assuming it's doable, similarly to what $subject does to backtick
expansion for external evaluation.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

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

Hello Daniel,

SELECT some-boolean-expression AS okay \gset
\if :okay

Yes, the question was whether we leave it as that for v10,
or if it's worth a last-minute improvement for usability,
assuming it's doable, similarly to what $subject does to backtick
expansion for external evaluation.

My 0.02 ￜ about server-side expressions: ISTM that there is nothing
obvious/easy to do to include these:

- how would it work, both with \set ... and \if ...?

- should it be just simple expressions or may it allow complex
queries?

- how would error detection and handling work from a script?

- should it have some kind of continuation, as expressions are
likely to be longer than a constant?

- how would they interact with possible client-side expressions?

(on this point, I think that client-side is NOT needed for "psql".
It makes sense for "pgbench" in a benchmarking context where the
client must interact with the server in some special meaningful
way, but for simple scripting the performance requirement and
logic is not the same, so server-side could be enough).

Basically quite a few questions which would not find an instantaneous
answer and associated patch.

However I agree with you that there may be minimal usability things to add
before 10, similar to Tom's backtick variable substitution.

Having some access to the client version as suggested by Pavel looks like
a good idea for the kind of script which may rely on conditionals...

Maybe other things, not sure what, though. Maybe other client settings
could be exported as variables, but the version looks like the main which
is currently missing.

Maybe a way to know what is the client current status? eg in transaction,
transaction has aborted, things like that?

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

2017-04-02 13:13 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

\echo :VERSION

PostgreSQL 10devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

Probably some :VERSION_NUM would make some sense. See attached PoC patch.
Would it make sense?

Maybe better name for you CLIENT_VERSION_NUM

If it was starting from nothing I would tend to agree with you, but there
is already an existing :VERSION variable, so it seemed logical to keep on
and create variants with the same prefix.

you have true - so VERSION_NUM should be client side version

Can be SERVER_VERSION_NUM taken from connection info?

Probably it could. It seems a little less straightforward than defining a
client-side string at compile time. The information is displayed when the
connection is established, so the information is there somewhere.

It is not too hard

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 94a3cfce90..d1ae81646f 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3320,16 +3320,21 @@ checkWin32Codepage(void)
 void
 SyncVariables(void)
 {
+   char        buffer[100];
+
    /* get stuff from connection */
    pset.encoding = PQclientEncoding(pset.db);
    pset.popt.topt.encoding = pset.encoding;
    pset.sversion = PQserverVersion(pset.db);

+   snprintf(buffer, 100, "%d", pset.sversion);
+
    SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
    SetVariable(pset.vars, "USER", PQuser(pset.db));
    SetVariable(pset.vars, "HOST", PQhost(pset.db));
    SetVariable(pset.vars, "PORT", PQport(pset.db));
    SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding));
+   SetVariable(pset.vars, "SVERSION_NUM", buffer);

/* send stuff to it, too */
PQsetErrorVerbosity(pset.db, pset.verbosity);

Regards

Pavel