On Thu, Apr 25, 2019 at 11:53 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Apr 25, 2019 at 12:45:03PM -0400, Tom Lane wrote:
> > I still remain concerned that invoking catalog lookups from fd.c is a darn
> > bad idea, even if we have a fallback for it to work (for some value of
> > "work") in non-transactional states. It's not really hard to envision
> > that kind of thing leading to infinite recursion. I think it's safe
> > right now, because catalog fetches shouldn't lead to any temp-file
> > access, but that's sort of a rickety assumption isn't it?
>
> Introducing catalog lookups into fd.c which is not a layer designed
> for that is a choice that I find strange, and I fear that it may bite
> in the future. I think that the choice proposed upthread to add
> an assertion on TempTablespacesAreSet() when calling a function
> working on temporary data is just but fine, and that we should just
> make sure that the gist code calls PrepareTempTablespaces()
> correctly. So [1] is a proposal I find much more acceptable than the
> other one.
Well the one thing I wish to point out explicitly is just taking fd.c
changes from [1], and running make check hits no assertions and
doesn't flag issue exist for gistbuildbuffers.c. Means its missing
coverage and in future same can happen as well.
[1]: https://postgr.es/m/11777.1556133426@sss.pgh.pa.us