Re: Parallell hashjoin sometimes ignores temp_tablespaces

On Fri, Jul 3, 2020 at 4:16 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > Thanks. pushed!
>
> Sorry for not having paid more attention earlier, but this patch is
> quite broken. If it weren't misguided it'd still be wrong, because
> this isn't the only spot in PrepareTempTablespaces that inserts
> InvalidOid into the output list.
>
> But, in fact, it's intentional that we represent the DB's default
> tablespace by InvalidOid in that list. Some callers of
> GetNextTempTableSpace need that to be the case, either for
> permissions-checking reasons or because they're going to store the
> result into a temp table's pg_class.reltablespace, where that
> representation is *required*.
>

Hmm. I guess I must've been careless in checking other callers.

I see that this is perhaps underdocumented, since while
> GetNextTempTableSpace's comment mentions the behavior, there's
> no comment about it with the data structure proper.
>

Yeah, it could definitely do with that. It was too many steps of
indirections away to me to pick that up.

It looks to me like the actual bug here is that whoever added
> GetTempTablespaces() and made sharedfileset.c depend on it
> did not get the memo about what to do with InvalidOid.
> It's possible that we could safely make GetTempTablespaces()
> do the substitution, but that would be making fd.c assume more
> about the usage of GetTempTablespaces() than I think it ought to.
> I feel like we oughta fix sharedfileset.c, instead.
>

This seems to be in dc6c4c9dc2a -- adding Thomas.

A quick look -- to do things right, we will need to know the database
default tablespace in this case right? Which I guess isn't there because
the shared fileset isn't tied to a database. But perhaps it's as easy as
something like the attached, just overwriting the oid?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>