Re: Regress tests reveal *serious* psql bug

I looked into the cause of the current failure of the array regress
test, enough to realize that there is both a garden-variety code bug
and a serious definitional problem there. The issue is that the new
psql converts

SELECT arrtest.a[1:3],
arrtest.b[1:1][1:2][1:2],
arrtest.c[1:2],
arrtest.d[1:1][1:2]
FROM arrtest;

into

SELECT arrtest.a[1],
arrtest.b[1][1][1],
arrtest.c[1],
arrtest.d[1][1]]
FROM arrtest

--- or at least it tries to do so; on one machine I have handy, psql
actually dumps core while running the array test. (It looks like that
is because line mainloop.c:259 underestimates the amount of memory it
needs to malloc for the changed string, but I haven't worked through the
details. I suspect the extra ']' is the result of an off-by-one kind of
bug in this same block of code.)

The reason *why* it is doing this is that it thinks that ":3" and so
forth are variables that it ought to substitute for, and since it has
no definition for them, it happily substitutes empty strings.

After fixing the outright bugs, we could make the array test work by
changing "[1:3]" to "[1\:3]" and so forth, but I think that that is the
wrong way to deal with it. I believe that psql's variable feature needs
to be redefined, instead.

I certainly don't feel that the regress tests are graven on stone
tablets; but when an allegedly unrelated feature change breaks one,
I think we need to treat that as a danger signal. If psql variables
break a regress test, they will likely break existing user applications
as well.

The case at hand is particularly nasty because if psql is allowed to
continue to behave this way, it will silently transform valid queries
into other valid queries with different results. ("array[1:3]" and
"array[1]" don't mean the same thing.) I don't think that's acceptable.

I suggest that psql's variable facility needs to be redefined so that
it is not possible to trigger it accidentally in scripts that don't
even know what psql variables are.

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.

I would also suggest tightening up the allowed names of variables;
conventional practice is that variable names have to start with a
letter, and I think psql ought to follow that convention. (That
wouldn't in itself stop the array-subscript problem, though, since
an array subscript could be a simple field reference.)

It might even be a good idea to require psql variables to contain only
upper-case letters, so that they'd be less likely to be confused with
SQL field names (which are usually lower case or at least mixed case)
--- but I'm not convinced about that one.

regards, tom lane