Re: Global temporary tables

Initial pass review follows.

Relation name "SESSION" is odd. I guess you're avoiding "global" because the data is session-scoped, not globally temporary. But I'm not sure "session" fits either - after all regular temp tables are also session temporary tables. I won't focus on naming further beyond asking that it be consistent though, right now there's a mix of "global" in some places and "session" in others.

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 . 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? 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 that patch. 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.

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.

A number of places make a change like this:

 

+ rel->rd_rel->relpersistence == RELPERSISTENCE_SESSION

and I'd like to see a test macro or inline static for it since it's repeated so much. Mostly to make the intent clear: "is this a relation with temporary backend-scoped data?"

This test:

+ if (blkno == BTREE_METAPAGE && PageIsNew(BufferGetPage(buf)) && IsSessionRelationBackendId(rel->rd_backend))
+ _bt_initmetapage(BufferGetPage(buf), P_NONE, 0);

seems sensible but I'm wondering if there's a way to channel initialization of global-temp objects through a bit more of a common path, so it reads more obviously as a common test applying to all global-temp tables. "Global temp table not initialized in session yet? Initialize it." So we don't have to have every object type do an object type specific test for "am I actually uninitialized?" in all paths it might hit. I guess I expected to see something more like a

if (RelGlobalTempUninitialized(rel))

but maybe I've been doing too much Java ;)

A similar test reappears here for sequences:

+ if (rel->rd_rel->relpersistence == RELPERSISTENCE_SESSION && PageIsNew(page))

Why is this written differently?

Sequence initialization ignores sequence startval/firstval settings. Why?

- else if (newrelpersistence == RELPERSISTENCE_PERMANENT)
+ else if (newrelpersistence != RELPERSISTENCE_TEMP)

Doesn't this change the test outcome for RELPERSISTENCE_UNLOGGED?

In PreCommit_on_commit_actions, in the the ONCOMMIT_DELETE_ROWS case, is there any way to still respect the XACT_FLAGS_ACCESSEDTEMPNAMESPACE flag if the oid is for a backend-temp table not a global-temp table?

+ bool isLocalBuf = SmgrIsTemp(smgr) && relpersistence == RELPERSISTENCE_TEMP;

I agree that there's no reason to be able to differentiate between local-temp and session-temp relfilenodes at the filesystem level.