Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)

From: Fabien COELHO <fabien(dot)coelho(at)mines-paristech(dot)fr>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Daniel Verite <daniel(at)manitou-mail(dot)org>, PostgreSQL <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)
Date: 2017-02-01 13:33:43
Message-ID: alpine.DEB.2.20.1702011425410.31128@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello,

> What do I need to do to get TAP tests running?

I misunderstood. You need to configure with "--enable-tap-tests".

>> There is a spurious empty line added at the very end of "mainloop.h":
>>
>> +
>> #endif /* MAINLOOP_H */
>
> Not in my diff, but that's been coming and going in your diff reviews.

Strange. Maybe this is linked to the warning displayed with "git apply"
when I apply the diff.

>> I would suggest to add a short one line comment before each test to
>> explain what is being tested, like "-- test \elif execution", "-- test
>> \else execution"...
>
> Where are you suggesting this?

In "regress/sql/psql.sql", in front of each group which starts a test.

>> Debatable suggestion about "psql_branch_empty":
>
> The name isn't great. Maybe psql_branch_stack_empty()?

Yep, maybe, or "empty_stack" or "stack_is_empty" or IDK...

> "psql_branch_end_state": it is a pop, it could be named "psql_branch_pop"
>
> Yeah, we either need to go fully with telling the programmer that it's a
> stack (push/pop/empty) or (begin_branch/end_branch/not_branching). I'm
> inclined to go full-stack, as it were.

Anything consistent is ok. I'm fine with calling a stack a stack:-)

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2017-02-01 13:48:34 Re: Deadlock in XLogInsert at AIX
Previous Message Rushabh Lathia 2017-02-01 13:32:30 Re: Gather Merge