On Fri, Apr 26, 2019 at 8:05 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > The version that I posted left it to GetNextTempTableSpace to assert > that. That seemed cleaner to me than an Assert that has to depend > on interXact. > > Running `make check` with [1] applied and one of the calls to PrepareTempTablespaces commented out, I felt like I deserved more as a developer than the assertion in this case.
Assertions are especially good to protect against regressions, but, in this case, I'm just trying to use an API that is being provided. Assertions don't give me a nice, easy-to-understand test failure. I see that there was a crash halfway through make check and now I have to figure out why. If that is the default way for developers to find out that they are missing something when using the API, it would be nice if it gave me some sort of understandable diff or error message. I also think that if there is a step that a caller should always take before calling a function, then there needs to be a very compelling reason not to move that step into the function itself. So, just to make sure I understand this case: PrepareTempTablespaces should not be called in BufFileCreateTemp because it is not concerned with temp tablespaces. OpenTemporaryFile is concerned with temp tablespaces, so any reference to those should be there. However, PrepareTempTablespaces should not be called in OpenTemporaryFile because it is in fd.c and no functions that make up part of the file descriptor API should do catalog lookups. Is this correct? [1] https://www.postgresql.org/message-id/11777.1556133426%40sss.pgh.pa.us -- Melanie Plageman
