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