Corey Huinker <corey.huinker@gmail.com> writes:
> On Wed, Feb 22, 2017 at 4:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.
Dunno, that sounds a lot like an "if the only tool I have is a hammer,
then this must be a nail" argument. pset should not accrete every single
global variable in psql just because it's there. Actually, there's a
pretty fair amount of stuff in it already that should not be there by any
reasonable interpretation of what it's for. Inventing a PsqlFileState or
similar struct might be a good idea to help pull some of that cruft out of
pset and get it back to having a reasonably clearly defined purpose of
holding "current settings".
So I think that if you're intent on this being a global variable, it might
as well be a standalone global variable. I was wondering more about
whether we shouldn't be passing the condition-stack top pointer around
to places that need to know about conditional execution. get_prompt would
be one if we decide that the prompt might need to reflect this (a question
that still seems undecided to me --- I think we'd be better off with "this
command was ignored" warning messages). I'm failing to follow why
GetVariable would need to care.
regards, tom lane