Neil Conway <neilc@samurai.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> I'm inclined to apply this but I can see where a person not comfortable
>> with flex might feel differently. Opinions?
> Looks good to me. The psql cleanup is nice, and ISTM that much of the
> flex code is comments or flex boilerplate anyway, so the actual LOC
> increase isn't too bad.
I just realized that something I had planned to look at later is
actually an essential step if this is to be applied. I had wanted
to see if lexing of backslash command tokens could be folded into the
flex lexer as well, but thought I could leave it for later.
The case where this doesn't preserve the previous behavior is if the
expansion of a psql variable includes both a backslash command and some
part of its arguments. As patched, HandleSlashCommand will only look to
the original input string and not see the contents of any psql variables
(because those are only seen inside the lexer, I'm not physically
inserting them into the line buffer as the old code did). Imagine
for example
\set foo '\c mydb'
blah :foo bar
The existing code would interpret this as
blah \c mydb bar
but my patch as it stands would behave very strangely --- the \c command
would see bar as its argument and then 'mydb' would be regurgitated
after HandleSlashCommand finishes.
The existing code is pretty darn inconsistent on this point anyway when
you look at it carefully. AFAICS a variable reference is handled quite
differently in the arguments of a backslash command than elsewhere; it
can't supply general text but only a single option value. The same
variable expanded before control reaches HandleSlashCommand is going to
look indistinguishable from hand-entered text. If we did translate it
into a flex thing the behavior would be different in corner cases like
this.
Peter, any comments on this? Is the current behavior actually
intentional, or did it just fall out of the implementation techniques?
regards, tom lane
