On 17 October 2010 16:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> Hmm, it's harder than I thought. The crash is because each time it
>> finds a label it hasn't seen before it adds it to the array of cached
>> values without checking the array bounds. That array is only as big as
>> the number of elements in the enum the first time it was called.
>
> [ scratches head... ] And where does it get that number of elements
> from, if not by doing the same work that would allow it to fill the
> array completely? Something seems ill-designed here.
>
Hmm. That's coming from a new column added to pg_type (typnlabels).
Perhaps that's not safe though. Are there potential race conditions
there?
>> Allowing the array to grow would prevent crashes, but not protect
>> again returning incorrect answers.
>
> Well, one of the questions here is exactly how wrong the answers can
> be. Offhand, it seems to me that comparisons of two existing entries
> can never be falsified by adding a new entry, so I'm not seeing that
> there could be any real problem. If we allowed rearrangement of the
> sort order of existing entries, it'd be problematic.
>
>> Perhaps it should just read and cache all the values the first time it
>> is called. Then if it ever fails to find a value in the array, it
>> knows that the enum must have grown, and it can rebuild the whole
>> array.
>
> This is kept in typcache, right? ISTM the right thing to do is arrange
> to invalidate the cached array when a cache flush event occurs, and
> rebuild the whole array on next use. Then you just throw an error if
> you're passed a value that isn't there.
>
Makes sense.
Regards,
Dean
> regards, tom lane
>