On Thu, Apr 25, 2019 at 11:53 PM Michael Paquier <mich...@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.1556133...@sss.pgh.pa.us
