Re: psql - add ability to test whether a variable exists

From: Robins Tharakan <tharakan(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql - add ability to test whether a variable exists
Date: 2017-09-20 06:37:46
Message-ID: CAEP4nAwUqpAdpMmmQbpt7CbY+PKT-PRSskZyvrjYvwzVPyb=Yg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Fabien,

I was able to test the functionality (which seemed to work fine) and fed in
my comment to assist anyone else reviewing this patch (and intentionally
let it's state as 'Needs Review').

While trying to provide my feedback, on hindsight I should have been more
detailed about what I didn't test. Being my first review, I didn't
understand that not checking a box meant 'failure'. For e.g. I read the
sgml changes, which felt okay but didn't click 'Passed' because my env
wasn't setup properly.

I've set this back to 'Needs Review' because clearly needs it.
Apologies for the noise here.

-
robins

On 20 September 2017 at 16:18, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:

>
> Hello Robins,
>
> Thanks for the review.
>
> The following review has been posted through the commitfest application:
>> make installcheck-world: not tested
>> Implements feature: tested, failed
>>
>
> Where ?
>
> Spec compliant: not tested
>> Documentation: tested, failed
>>
>
> Where ? I just regenerated the html doc on the patch without a glitch.
>
> The patch applies cleanly and compiles + installs fine (although am unable
>> to do installcheck-world on my Cygwin setup). This is how the patch works
>> on my setup.
>>
>> $ /opt/postgres/reviewpatch/bin/psql -U postgres -h localhost
>> psql (11devel, server 9.6.1)
>> Type "help" for help.
>>
>> postgres=# \set i 1
>> postgres=# \if :{?i}
>> postgres=# \echo 'testing'
>> testing
>> postgres=# \endif
>> postgres=# \if :{?id}
>> postgres(at)# \echo 'testing'
>> \echo command ignored; use \endif or Ctrl-C to exit current \if block
>> postgres(at)# \endif
>> postgres=#
>>
>
> ISTM that this is the expected behavior.
>
> In the first case, "i" is defined, so the test is true and the echo echoes.
>
> In the second case, "id" is undefined, the test is false and the echo is
> skipped.
>
> I do not understand why you conclude that the feature "failed".
>
> --
> Fabien.
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robins Tharakan 2017-09-20 06:38:08 Re: psql - add ability to test whether a variable exists
Previous Message Rosser Schwarz 2017-09-20 06:26:21 Re: Patch: add --if-exists to pg_recvlogical