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

Поиск
Список
Период
Сортировка
От Corey Huinker
Тема Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands:\quit_if, \quit_unless)
Дата
Msg-id CADkLM=cxaB-GjPScrhSqcTfwEHD5JLtOOQ6PRqv8dWBY1q0c0Q@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
On Wed, Feb 22, 2017 at 4:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Corey Huinker <corey.huinker@gmail.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.

Judging by the volume of the 50-or-so active threads on this list, I figured you were busy.
 
> 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?

Good to know that history.
 

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.

I wasn't, my reviewer was. I thought about the super-state structure like you described, and decided I was happy with two state params.
 
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.

The rationale for tying if-state to file levels is not so much of anything if-then related, but rather of the mess we'd create for whatever poor soul decided to undertake \while loops down the road, and the difficulties they'd have trying to unwind/rewind jump points in file(s)...keeping it inside one file makes things simpler for the coding and the coder.
 
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.

With the if-stack moved to MainLoop(), nearly all the active_branch checks could be against a variable that lives in MainLoop(), with two big exceptions: GetVariable() needs to know when NOT to expand a variable because it's in a false-block, and get_prompt will need to know when it's in a false block for printing the '@' prompt hint or equivalent, and pset is the only global around I know of to do that. I can move nearly all the is-this-branch-active checks to structures inside of MainLoop(), and that does strike me as cleaner, but there will still have to be that gross bit where we update pset.active_branch so that the prompt and GetVariable() are clued in.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] Should we cacheline align PGXACT?
Следующее
От: Tom Lane
Дата:
Сообщение: Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)