Re: Global temporary tables

On 16.08.2019 11:37, Craig Ringer wrote:
>
>
>     On Fri, 16 Aug 2019 at 15:30, Konstantin Knizhnik
>     <k.knizhnik@postgrespro.ru <mailto:k.knizhnik@postgrespro.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