Re: Global temporary tables

On Fri, 16 Aug 2019 at 15:30, Konstantin Knizhnik <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