Re: Variable substitution in psql backtick expansion

On Sun, May 21, 2017 at 11:37 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>> This patch has trivial implementation - and there are not any objection to
>> used new variable names.
>>
>> 1. there was not any problem with patching, compiling
>> 2. all regress tests passed
>> 3. no problems with doc build
>>
>> I'll mark this patch as ready for commiters
>
> Thanks for the review.

I am attempting to understand the status of this patch. It looks like
the patch that was the original subject of this thread was committed
as f833c847b8fa4782efab45c8371d3cee64292d9b on April 1st by Tom, who
was its author. Subsequently, a new patch not obviously related to the
subject line was proposed by Fabien Coelho, and that patch was
subsequently marked Ready for Committer by Pavel Stehule. Meanwhile,
objections were raised by Tom, who seems to think that we should make
\if accept an expression language before we consider this change.
AFAICT, there's no patch for that.

Personally, I have mixed feelings about Tom's objection. On the one
hand, it seems a bit petty to reject this patch on the grounds that
the marginally-related feature of having \if accept an expression
doesn't exist yet. It's hard enough to get things into PostgreSQL
already; we shouldn't make it harder without a very fine reason. On
the other hand, given that there is no client-side expression language
yet, the only way to use this seems to be to send a query to the
server, and if you're going to do that, you may as well use select
current_setting('server_version_num') instead of referencing a psql
variable containing the same value. Maybe the client version number
has some use, though; I think that's not otherwise available right
now.

Some comments on the patch itself:

- Documentation needs attention from a native English speaker.
- Is it a good idea/practical to prevent the new variables from being
modified by the user?
- I think Pavel's upthread suggestion of prefixing the client
variables with "client" to match the way the server variables are
named is a good idea.

I guess the bottom line is that I'm willing to fix this up and commit
it if we've got consensus on it, but I read this thread as Fabien and
Pavel voting +1 on this patch, Tom as voting -1, and a bunch of other
people (including me) not taking a strong position. In my view,
that's not enough consensus to proceed. Anybody else want to vote, or
change their vote?

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