Re: Calling PrepareTempTablespaces in BufFileCreateTemp

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Melanie Plageman <melanieplageman(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Calling PrepareTempTablespaces in BufFileCreateTemp
Date: 2019-04-23 20:05:59
Message-ID: 16713.1556049959@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Peter Geoghegan <pg(at)bowt(dot)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

Attachment Content-Type Size
move-temp-tablespace-setup-calls-1.patch text/x-diff 4.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-04-23 20:07:05 Re: Trouble with FETCH_COUNT and combined queries in psql
Previous Message Ibrar Ahmed 2019-04-23 19:22:25 How and at what stage to stop FDW to generate plan with JOIN.