Re: Regress tests reveal *serious* psql bug

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> In some earlier developmental stage I had the variable delimiter runtime
> definable since it became clear that the traditional $ wouldn't work.
> Later on someone pointed out that the SQL syntax for this is the colon
> deal. (And you were in that discussion, if I am not completely off.)

Yah. I think at the time we were only thinking of colon as a user-
definable operator (well, it's also a predefined operator, but not a
very essential one). I plead guilty to forgetfulness in not remembering
that it was also an essential component of the array grammar. Still,
there it is. I think this raises the bar to the point where we must
have a transparent backward-compatible approach to psql variables.
I was not all that thrilled about blowing off colon as an operator,
and blowing off array subscripts *too* is just too far above my
threshold of pain.

> Actually the colon deal only applies to embedded SQL, so ecpg should have
> the same problem, but I'm not familiar with it, so I don't know how it
> copes.

Good question. Michael?

> The fact is that (besides the garden-variety bugs) this is indeed a
> problem of definition. I'm not sure if the following is valid by any
> standard or even makes sense, but how do you interpret something like
> this:
> SELECT arrtest.biggest_value_pos, arrtest.a[1:biggest_value_pos] ... ;
> There's no way you can disambiguate this unless you redefine everything.

Huh? There was nothing in the least ambiguous about it, until you
redefined psql. It's still not ambiguous in any other pgsql interface,
except possibly ecpg...

>> A minimum requirement is that psql should *not* substitute for :x unless
>> x is the name of a psql variable that the user has explicitly defined.

> Then psql becomes no better than csh, and that's certainly one of the
> worse things one could do.

csh isn't one of my favorite programming languages either, but failure
to detect undefined substitution variables is a pretty venial sin
compared to silently transforming *valid* queries into wrong queries.
The former will not bring villagers with pitchforks to your doorstep,
but the latter will.

Furthermore, if you are trying to help the substitution-variable
programmer detect his mistakes, then silently substituting (wrong)
empty values is not my idea of helpfulness. You could maybe make a
defensible case for rejecting the whole query with ":x is not defined".
What you have chosen is the worst of all possible worlds, because it
breaks existing scripts that are ignorant of the new feature without
doing anything particularly helpful for people who *are* using the new
feature.

Finally, if the would-be user of psql variables misspells :foo as
:fop, I think he's much more likely to realize he's made a mistake if
psql passes :fop as-is to the backend rather than quietly discarding it.

> Putting blame on other people's shoulders I would suggest changing the
> array syntax to arr[1][2] like everyone else has,

Say what? That's the syntax for a 2-D array, not the syntax for
an array slice.

> The best idea I have to get this working _now_ would be to once again make
> the variable delimiter run-time configurable so that you can set it to
> : or $ or # or whatever your query doesn't use, completely off by default

If it's off by default, that would eliminate the backwards-compatibility
problem --- but I still think the potential dual use of whatever
character you happen to pick would just be a gotcha waiting to bite
anyone who uses the feature. We still ought to think about tightening
up the substitution rules so they are less likely to trap the unwary.

> If I'm going to hack around in that code, one related question: what
> should the deal be regarding variable interpolation into quoted
> strings? Yes/No/Maybe?

No bloody way, IMHO --- that increases the odds of unwanted
substitutions by many orders of magnitude. If you want to allow
substitutions into quoted strings, there should be a very special
syntax for it, perhaps along the lines of
'a quoted ':foo' string'
which could translate to 'a quoted foobar string' if :foo expands
to foobar.

regards, tom lane