Thread: BUG #5448: psql \set does not terminate if variable is referenced recursively

The following bug has been logged online:

Bug reference:      5448
Logged by:          Francis
Email address:      fmarkham@gmail.com
PostgreSQL version: 8.4.3
Operating system:   Ubuntu linux 10.04
Description:        psql \set does not terminate if variable is referenced
recursively
Details:

psql \set does not terminate if a variable is referenced recursively.  For
example, the following will hang the psql client in a nasty way:

db=# \set n 1
db=# \set n (:n + 1)

"Francis" <fmarkham@gmail.com> wrote:

> psql \set does not terminate if a variable is referenced
> recursively.  For example, the following will hang the psql client
> in a nasty way:
>
> db=# \set n 1
> db=# \set n (:n + 1)

It seem to me that the above doesn't hang the psql client, but a
subsequent reference to :n does.  It doesn't have to be a direct
self-reference, either; any circular reference seems to do it.  It
doesn't respond to Ctrl+C during this recursion.

pgbench=# \set n 1
pgbench=# \set x (:n+1)
pgbench=# select :x;
 ?column?
----------
        2
(1 row)

pgbench=# \set n 5
pgbench=# select :x;
 ?column?
----------
        6
(1 row)

pgbench=# \set n (:x+1)
pgbench=# select :x;
[CAUTION: psql sucked CPU time and ate RAM until I killed it]

-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> "Francis" <fmarkham@gmail.com> wrote:
>> psql \set does not terminate if a variable is referenced
>> recursively.  For example, the following will hang the psql client
>> in a nasty way:
>>
>> db=# \set n 1
>> db=# \set n (:n + 1)

> It seem to me that the above doesn't hang the psql client, but a
> subsequent reference to :n does.

As near as I can tell, it's actually trying to do a recursive expansion
of the variable, which of course doesn't terminate.  On my machine it
fails after awhile with

out of dynamic memory in yy_scan_buffer()
xmalloc: out of virtual memory

but you could easily endure a lot of swapping before you get to that,
if your machine isn't carefully configured for amount of swap vs real
RAM vs max allowed process size.

I suppose the only fix is to keep track of which variables are being
actively expanded and refuse to attempt a recursive expansion.
However our options as to what to actually *do* when we detect recursion
are a bit limited.  We could print a message and treat the inner
expansion as empty --- is that good enough?

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We could print a message and treat the inner
> expansion as empty --- is that good enough?

If there's any way to throw an error, that seems better.  I can
imagine someone getting confused by silent failure here.  On the
other hand, since this probably doesn't happen very often, maybe
it's not worth the effort.  I could even see an argument for just
testing for Ctrl+C before each expansion and otherwise leaving it as
is.

-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> We could print a message and treat the inner
>> expansion as empty --- is that good enough?

> If there's any way to throw an error, that seems better.  I can
> imagine someone getting confused by silent failure here.  On the
> other hand, since this probably doesn't happen very often, maybe
> it's not worth the effort.  I could even see an argument for just
> testing for Ctrl+C before each expansion and otherwise leaving it as
> is.

The problem is there's no real support inside psql for "throwing an
error" --- we have to unwind all the state manually.  In particular,
what this problem requires is backing out the stack of flex buffers
representing pending variable expansions.  So I think we need to add
an explicit recursion test and suppress further expansion of the
variable when we see it, but there's no very simple way to just abandon
the current command altogether.

We can definitely print a message though, and I agree that just silently
suppressing recursion would be confusing.

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:

> I think we need to add an explicit recursion test and suppress
> further expansion of the variable when we see it

> We can definitely print a message

Sounds perfect to me.

-Kevin

On 2010-05-05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> The problem is there's no real support inside psql for "throwing an
> error" --- we have to unwind all the state manually.  In particular,
> what this problem requires is backing out the stack of flex buffers
> representing pending variable expansions.  So I think we need to add
> an explicit recursion test and suppress further expansion of the
> variable when we see it, but there's no very simple way to just abandon
> the current command altogether.

having not examined the code I immagine something like this could work.

fputs(stgerr,"recursive expansion\n");
raise(SIGINT);
return "";

> We can definitely print a message though, and I agree that just silently
> suppressing recursion would be confusing.

On Wed, May 5, 2010 at 1:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The problem is there's no real support inside psql for "throwing an
> error" --- we have to unwind all the state manually. =A0In particular,
> what this problem requires is backing out the stack of flex buffers
> representing pending variable expansions. =A0So I think we need to add
> an explicit recursion test and suppress further expansion of the
> variable when we see it, but there's no very simple way to just abandon
> the current command altogether.

I notIced this when working on Pavel's patch to make :'foo' and :"foo"
do interpolation with literal/identifier quoting, and thought that it
would benefit from some refactoring, but didn't want to bother with it
at the time.  It's seems like kind of a pain for the amount of benefit
we'd likely to get out of it, but I think we'd probably be happier in
the long run.

http://en.wikipedia.org/wiki/Frog_boiling

--=20
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company