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>, Greg Stark <stark(at)mit(dot)edu>, Erik Rijkers <er(at)xs4all(dot)nl>, Robert Haas <robertmhaas(at)gmail(dot)com>, Daniel Verite <daniel(at)manitou-mail(dot)org>, 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-02-23 21:46:31
Message-ID: alpine.DEB.2.20.1702232148170.21598@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Corey,

> v16 is everything v15 promised to be.

My 0.02€:

Patch applies, make check ok, psql make check ok as well.

>> Welcome to v15, highlights:
>> - all conditional data structure management moved to conditional.h and
>> conditional.c

Indeed.

I cannot say that I find it better, but (1) Tom did required it and (2) it
still works:-)

If the stack stuff had stayed in "fe_utils", it would have been easy to
reuse them in pgbench where they might be useful... But who cares?

>> - conditional state lives in mainloop.c and is passed to
>> HandleSlashCommands, exec_command and get_prompt as needed

Same.

>> - no more pset.active_branch, uses conditional_active(conditional_stack)
>> instead

Same.

>> - PsqlScanState no longer has branching state

Indeed.

>> - Implements the %R '@' prompt on false branches.

I'm not sure that '@' is the best choice, but this is just one char.

I noticed that it takes precedence over '!'. Why not. ISTM that orthogonal
features are not shown independently, but this is a preexisting state, and
it allows to have a shorter prompt, so why not.

Anyway, the '%R' documentation needs to be updated.

>> - Variable expansion is never suppressed even in false blocks,
>> regression test edited to reflect this.

It could be nice to keep test cases that show what may happen?

The various simplifications required result in the feature being more
error prone for the user. Maybe the documentation could add some kind of
warning about that?

>> - ConditionalStack could morph into PsqlFileState without too much
>> work.

Probably.

Code details:

Add space after comma when calling send_query.

I'm not sure why you removed the comments before \if in the doc example.
Maybe keep a one liner?

Why not reuse the pop loop trick to "destroy" the stack?

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nico Williams 2017-02-23 21:55:03 Re: Idea on how to simplify comparing two sets
Previous Message Andrew Dunstan 2017-02-23 21:45:00 Re: btree_gin and btree_gist for enums