Re: I'm amazed we never caught this before ...

On Sat, 6 Jan 2001, Tom Lane wrote:

> The stringToNode routines (backend/nodes/read.c and readfuncs.c) depend
> on a static variable that is the next-input pointer for lsptok().
>
> Guess what happens if stringToNode is invoked recursively. (Hint:
> it's not good.)
>
> It's apparently not easy for this to happen, but I have a reproducible
> test case involving relcache flush while reading the rule action
> parsetree for a view. readDatum() needs to be able to look up the
> pg_type entry for the value it's reading, and that lookup could result
> in relcache flush, which would lead to an attempt to read the rule
> actions for any locked relcache entries --- like, say, the one we
> are currently trying to load.
>
> I propose fixing this by making stringToNode save and restore the
> entry-time value of the static pointer. (Alternatively, we could
> explicitly pass a pointer-to-pointer-to-char to lsptok() and every
> single one of the readfuncs, but that seems like way too much
> notational clutter.)
>
> I am not sure that is enough to fix the problem completely. It might
> be best to avoid calling get_typbyval() from readDatum, which could
> be done if we re-ordered the fields of a CONST dump so that the byval
> field appears before we need to read the const value itself. That
> means an initdb, but we've already forced an initdb for beta2 ...
>
> Marc, can you hold off wrapping beta2 for a few hours? I think this
> is a 'must fix'.

No probs ... I got the update to mk-release from Peter, so let me know
when you are ready and we'll go with beta2 ...