Re: Calling PrepareTempTablespaces in BufFileCreateTemp
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2019-04-23T20:05:59Z
Lists: pgsql-hackers
Attachments
- move-temp-tablespace-setup-calls-1.patch (text/x-diff) patch
I wrote: > Peter Geoghegan <pg@bowt.ie> writes: >> That doesn't seem like a particularly good or complete answer, though. >> Perhaps it should simply be called within BufFileCreateTemp(). The >> BufFile/fd.c layering is confusing in a number of ways IMV. > I don't actually see why BufFileCreateTemp should do it; if > we're to add a call, seems like OpenTemporaryFile is the place, > as that's what is really concerned with the temp tablespace(s). > I'm in favor of doing this, I think, as it sure looks to me like > gistInitBuildBuffers() is calling BufFileCreateTemp without any > closely preceding PrepareTempTablespaces. So we already have an > instance of Melanie's bug in core. It'd be difficult to notice > because of the silent-fallback-to-default-tablespace behavior. Here's a draft patch for that. It's slightly ugly that this adds a dependency on commands/tablespace to fd.c, which is a pretty low-level module. I think wanting to avoid that layering violation might've been the reason for doing things the way they are. However, this gets rid of tablespace dependencies in some other files that are only marginally higher-level, like tuplesort.c, so I'm not sure how strong that objection is. There are three functions in fd.c that have a dependency on the temp tablespace info having been set up: OpenTemporaryFile GetTempTablespaces GetNextTempTableSpace This patch makes the first of those automatically set up the info if it's not done yet. The second one has always had an assertion that the caller did it already, and now the third one does too. An about equally plausible change would be to make all three call PrepareTempTablespaces, but there are so few callers of the second and third that I'm not sure that'd be better. Thoughts? regards, tom lane
Commits
-
Make BufFileCreateTemp() ensure that temp tablespaces are set up.
- 93f03dad824f 12.0 landed