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

From: Fabien COELHO <coelho(at)cri(dot)ensmp(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>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Erik Rijkers <er(at)xs4all(dot)nl>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, PostgreSQL <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org
Subject: Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)
Date: 2017-03-24 20:10:52
Message-ID: alpine.DEB.2.20.1703242047000.8361@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Corey,

> I wished for the same thing, happy to use one if it is made known to me.
> I pulled that pattern from somewhere else in the code, and given that the
> max number of args for a command is around 4, I'm not too worried about
> scaling.

If there are expressions one day like pgbench, the number of arguments
becomes arbitrary. Have you looked at PQExpBuffer?

>> However there is an impact on testing because of all these changes. ISTM
>> that test cases should reflect this situation and test that \cd, \edit, ...
>> are indeed ignored properly and taking account there expected args...
>
> I think one grand
>
> \if false
> \a
> \c some_connect_string
> ...
> \z some_table_name
> \endif
> should do the trick,

Yes. Maybe some commands could be on the same line as well.

> but it wouldn't detect memory leaks.

No miracle...

>> There seems to be pattern repetition for _ev _ef and _sf _sv. Would it
>> make sense to create a function instead of keeping the initial copy-paste?
>
> Yes, and a few things like that, but I wanted this patch to keep as much
> code as-is as possible.

If you put the generic function at the same place, one would be more or
less kept and the other would be just removed?

"git diff --patience -w" gives a rather convenient output for looking at
the patch.

>> I would suggest to put together all if-related backslash command, so that
>> the stack management is all in one function instead of 4.
>
> I recognize the urge to group them together, but would there be any actual
> code sharing between them? Wouldn't I be either re-checking the string
> "cmd" again, or otherwise setting an enum that I immediately re-check
> inside the all_branching_commands() function?

I do not see that as a significant issue, especially compared to the
benefit of having the automaton transition management in a single place.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-24 20:21:49 Re: Page Scan Mode in Hash Index
Previous Message Amit Khandekar 2017-03-24 20:04:35 Re: UPDATE of partition key