Re: Global temporary tables

On 16.08.2019 11:37, Craig Ringer wrote:
>
>
> On Fri, 16 Aug 2019 at 15:30, Konstantin Knizhnik
> <k(dot)knizhnik(at)postgrespro(dot)ru <mailto:k(dot)knizhnik(at)postgrespro(dot)ru>> wrote:
>
> I forget or do not notice some of your questions, would you be
> so kind as to repeat them?
>
>
> Sent early by accident.
>
> Repeating questions:

Sorry, but I have answered them (my e-mail from 13.08)!
Looks like you have looed at wrong version of the patch:
global_shared_temp-1.patch instead of global_private_temp-1.patch which
implements global tables accessed through local buffers.

>
>
> Why do you need to do all this indirection with changing RelFileNode
> to RelFileNodeBackend in the bufmgr, changing BufferGetTag etc?
> Similarly, your changes of RelFileNodeBackendIsTemp
> to RelFileNodeBackendIsLocalTemp . I'm guessing you did it the way you
> did instead to lay the groundwork for cross-backend sharing, but if so
> it should IMO be in your second patch that adds support for using
> shared_buffers for temp tables, not in the first patch that adds a
> minimal global temp tables implementation. Maybe my understanding of
> the existing temp table mechanics is just insufficient as I
> see RelFileNodeBackendIsTemp is already used in some aspects of
> existing temp relation handling.
>

Sorry, are you really speaking about global_private_temp-1.patch?
This patch doesn't change bufmgr file at all.
May be you looked at another patch - global_shared_temp-1.patch
which is accessing shared tables though shared buffers and so have to
change buffer tag to include backend ID in it.

> Did you look into my suggestion of extending the relmapper so that
> global temp tables would have a relfilenode of 0 like pg_class etc,
> and use a backend-local map of oid-to-relfilenode mappings?
>
> Similarly, TruncateSessionRelations probably shouldn't need to exist
> in this patch in its current form; there's no shared_buffers use to
> clean and the same file cleanup mechanism should handle both
> session-temp and local-temp relfilenodes.

In global_private_temp-1.patch TruncateSessionRelations does nothing
with shared buffers, it just delete relation files.

>
> Sequence initialization ignores sequence startval/firstval settings. Why?
> +               value[SEQ_COL_LASTVAL-1] = Int64GetDatumFast(1); /*
> start sequence with 1 */
>
>
>

I am handling only case of implicitly created sequences for
SERIAL/BIGSERIAL columns.
Is it possible to explicitly specify initial value and step for them?
If so, this place should definitely be rewritten.

> Doesn't this change the test outcome for RELPERSISTENCE_UNLOGGED?:
> - else if (newrelpersistence == RELPERSISTENCE_PERMANENT)
> + else if (newrelpersistence != RELPERSISTENCE_TEMP)
>

RELPERSISTENCE_UNLOGGED case is handle in previous IF branch.
-

Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company