I wrote:
> So maybe a reasonable compromise is to add the Assert(s) in fd.c as
> per previous patch, but *also* add PrepareTempTablespaces in
> BufFileCreateTemp, so that at least users of buffile.c are insulated
> from the issue. buffile.c is still kind of low-level, but it's not
> part of core infrastructure in the same way as fd.c, so probably I could
> hold my nose for this solution from the system-structural standpoint.
Actually, after digging around in the related code some more, I'm having
second thoughts about those Asserts. PrepareTempTablespaces is pretty
clear about what it thinks the contract is:
/*
* Can't do catalog access unless within a transaction. This is just a
* safety check in case this function is called by low-level code that
* could conceivably execute outside a transaction. Note that in such a
* scenario, fd.c will fall back to using the current database's default
* tablespace, which should always be OK.
*/
if (!IsTransactionState())
return;
If we just add the discussed assertions and leave this bit alone,
the net effect would be that any tempfile usage outside a transaction
would suffer an assertion failure, *even if* it had called
PrepareTempTablespaces. There doesn't seem to be any such usage in
the core code, but do we really want to forbid the case? It seems
like fd.c shouldn't be imposing such a restriction, if it never has
before.
So now I'm feeling more favorable about the idea of adding a
PrepareTempTablespaces call to BufFileCreateTemp, and just stopping
with that. If we want to do more, I feel like it requires a
significant amount of rethinking about what the expectations are for
fd.c, and some rejiggering of PrepareTempTablespaces's API too.
I'm not sufficiently excited about this issue to do that.
regards, tom lane
