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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, 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-22 21:00:50
Message-ID: 16856.1487797250@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Corey Huinker <corey(dot)huinker(at)gmail(dot)com> writes:
>> but if you think that it should be somewhere else, please advise Corey
>> about where to put it.

> Just a reminder that I'm standing by for advice.

Sorry, I'd lost track of this thread.

> The issue at hand is whether the if-state should be a part of the
> PsqlScanState, or if it should be a separate state variable owned by
> MainLoop() and passed to HandleSlashCommands(), ... or some other solution.

My reaction to putting it in PsqlScanState is pretty much "over my dead
body". That's just trashing any pretense of an arms-length separation
between psql and the lexer proper. We only recently got done sweating
blood to create that separation, why would we toss it away already?

If you're concerned about the notational overhead of passing two arguments
rather than one, my druthers would be to invent a new struct type, perhaps
named along the lines of PsqlFileState or PsqlInputState, and pass that
around. One of its fields would be a PsqlScanState pointer, the rest
would be for if-state and whatever else we think we need in per-input-file
state.

However, that way is doubling down on the assumption that the if-state is
exactly one-to-one with input file levels, isn't it? We might be better
off to just live with the separate arguments to preserve some flexibility
in that regard. The v12 patch doesn't look that awful in terms of what
it's adding to argument lists.

One thing I'm wondering is why the "active_branch" bool is in "pset"
and not in the conditional stack. That seems, at best, pretty grotty.
_psqlSettings is meant for reasonably persistent state.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-02-22 21:14:14 Re: Should we cacheline align PGXACT?
Previous Message Nikolay Shaplov 2017-02-22 20:41:39 Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM