Thread: [Proposal] Global temporary tables

Dear Hackers,

This propose a way to develop global temporary tables in PostgreSQL.

I noticed that there is an "Allow temporary tables to exist as empty by default in all sessions" in the postgresql todolist.

https://wiki.postgresql.org/wiki/Todo

In recent years, PG community had many discussions about global temp table (GTT) support. Previous discussion covered the following topics: 

(1) The main benefit or function: GTT offers features like “persistent schema, ephemeral data”, which avoids catalog bloat and reduces catalog vacuum. 

(2) Whether follows ANSI concept of temporary tables

(3) How to deal with statistics, single copy of schema definition, relcache

(4) More can be seen in https://www.postgresql.org/message-id/73954ab7-44d3-b37b-81a3-69bdcbb446f7%40postgrespro.ru

(5) A recent implementation and design from Konstantin Knizhnik covered many functions of GTT: https://www.postgresql.org/message-id/attachment/103265/global_private_temp-1.patch

However, as pointed by Konstantin himself, the implementation still needs functions related to CLOG, vacuum, and MVCC visibility.

Just to clarify.
I have now proposed several different solutions for GTT:

Shared vs. private buffers for GTT:
1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.

Access to GTT at replica:
1. Access is prohibited (as for original temp tables). No changes at all.
2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules are used for GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32
and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen, so XID horizon never moved).

So except the limitation mentioned above (which I do not consider as critical) there is only one problem which was not addressed: maintaining statistics for GTT.
If all of the following conditions are true:

1) GTT are used in joins
2) There are indexes defined for GTT
3) Size and histogram of GTT in different backends can significantly vary.
4) ANALYZE was explicitly called for GTT

then query execution plan built in one backend will be also used for other backends where it can be inefficient.
I also do not consider this problem as "show stopper" for adding GTT to Postgres.

I still do not understand the opinion of community which functionality of GTT is considered to be most important.
But the patch with local buffers and no replica support is small enough to become good starting point.

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

On Fri, Oct 11, 2019 at 9:50 AM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
> Just to clarify.
> I have now proposed several different solutions for GTT:
>
> Shared vs. private buffers for GTT:
> 1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
> 2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.

I vote for #1. I think parallel query for temp objects may be a
desirable feature, but I don't think it should be the job of a patch
implementing GTTs to make it happen. In fact, I think it would be an
actively bad idea, because I suspect that if we do eventually support
temp relations for parallel query, we're going to want a solution that
is shared between regular temp tables and global temp tables, not
separate solutions for each.

> Access to GTT at replica:
> 1. Access is prohibited (as for original temp tables). No changes at all.
> 2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
> 3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules are
usedfor GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32
 
> and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen, so
XIDhorizon never moved).
 

I again vote for #1. A GTT is defined to allow data to be visible only
within one session -- so what does it even mean for the data to be
accessible on a replica?

> So except the limitation mentioned above (which I do not consider as critical) there is only one problem which was
notaddressed: maintaining statistics for GTT.
 
> If all of the following conditions are true:
>
> 1) GTT are used in joins
> 2) There are indexes defined for GTT
> 3) Size and histogram of GTT in different backends can significantly vary.
> 4) ANALYZE was explicitly called for GTT
>
> then query execution plan built in one backend will be also used for other backends where it can be inefficient.
> I also do not consider this problem as "show stopper" for adding GTT to Postgres.

I think that's *definitely* a show stopper.

> I still do not understand the opinion of community which functionality of GTT is considered to be most important.
> But the patch with local buffers and no replica support is small enough to become good starting point.

Well, it seems we now have two patches for this feature. I guess we
need to figure out which one is better, and whether it's possible for
the two efforts to be merged, rather than having two different teams
hacking on separate code bases.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 25.10.2019 18:01, Robert Haas wrote:
> On Fri, Oct 11, 2019 at 9:50 AM Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> Just to clarify.
>> I have now proposed several different solutions for GTT:
>>
>> Shared vs. private buffers for GTT:
>> 1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
>> 2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.
> I vote for #1. I think parallel query for temp objects may be a
> desirable feature, but I don't think it should be the job of a patch
> implementing GTTs to make it happen. In fact, I think it would be an
> actively bad idea, because I suspect that if we do eventually support
> temp relations for parallel query, we're going to want a solution that
> is shared between regular temp tables and global temp tables, not
> separate solutions for each.

Sorry, may be I do not not understand you.
It seems to me that there is only one thing preventing usage of 
temporary tables in parallel plans: private buffers.
If global temporary tables are accessed as normal tables though shared 
buffers then them can be used in parallel queries
and no extra support is required for it.
At least I have checked that parallel queries are correctly worked for 
my implementation of GTT with shared buffers.
So I do not understand about which "separate solutions" you are talking 
about.

I can agree that private buffers may be  good starting point for GTT 
implementation, because it is less invasive and GTT access speed is 
exactly the same as of normal temp tables.
But I do not understand your argument why it is "actively bad idea".

>> Access to GTT at replica:
>> 1. Access is prohibited (as for original temp tables). No changes at all.
>> 2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
>> 3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules are
usedfor GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32
 
>> and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen, so
XIDhorizon never moved).
 
> I again vote for #1. A GTT is defined to allow data to be visible only
> within one session -- so what does it even mean for the data to be
> accessible on a replica?

There are sessions at replica (in case of hot standby), aren't there?

>
>> So except the limitation mentioned above (which I do not consider as critical) there is only one problem which was
notaddressed: maintaining statistics for GTT.
 
>> If all of the following conditions are true:
>>
>> 1) GTT are used in joins
>> 2) There are indexes defined for GTT
>> 3) Size and histogram of GTT in different backends can significantly vary.
>> 4) ANALYZE was explicitly called for GTT
>>
>> then query execution plan built in one backend will be also used for other backends where it can be inefficient.
>> I also do not consider this problem as "show stopper" for adding GTT to Postgres.
> I think that's *definitely* a show stopper.
Well, if both you and Pavel think that it is really "show stopper", then 
this problem really has to be addressed.
I slightly confused about this opinion, because Pavel has told me 
himself that 99% of users never create indexes for temp tables
or run "analyze" for them. And without it, this problem is not a problem 
at all.

>> I still do not understand the opinion of community which functionality of GTT is considered to be most important.
>> But the patch with local buffers and no replica support is small enough to become good starting point.
> Well, it seems we now have two patches for this feature. I guess we
> need to figure out which one is better, and whether it's possible for
> the two efforts to be merged, rather than having two different teams
> hacking on separate code bases.

I am open for cooperations.
Source code of all my patches is available.

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

> 2019年10月26日 上午12:22，Konstantin Knizhnik <k.knizhnik@postgrespro.ru> 写道：
>
>
>
> On 25.10.2019 18:01, Robert Haas wrote:
>> On Fri, Oct 11, 2019 at 9:50 AM Konstantin Knizhnik
>> <k.knizhnik@postgrespro.ru> wrote:
>>> Just to clarify.
>>> I have now proposed several different solutions for GTT:
>>>
>>> Shared vs. private buffers for GTT:
>>> 1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
>>> 2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.
>> I vote for #1. I think parallel query for temp objects may be a
>> desirable feature, but I don't think it should be the job of a patch
>> implementing GTTs to make it happen. In fact, I think it would be an
>> actively bad idea, because I suspect that if we do eventually support
>> temp relations for parallel query, we're going to want a solution that
>> is shared between regular temp tables and global temp tables, not
>> separate solutions for each.
>
> Sorry, may be I do not not understand you.
> It seems to me that there is only one thing preventing usage of temporary tables in parallel plans: private buffers.
> If global temporary tables are accessed as normal tables though shared buffers then them can be used in parallel
queries
> and no extra support is required for it.
> At least I have checked that parallel queries are correctly worked for my implementation of GTT with shared buffers.
> So I do not understand about which "separate solutions" you are talking about.
>
> I can agree that private buffers may be  good starting point for GTT implementation, because it is less invasive and
GTTaccess speed is exactly the same as of normal temp tables. 
> But I do not understand your argument why it is "actively bad idea".
>
>>> Access to GTT at replica:
>>> 1. Access is prohibited (as for original temp tables). No changes at all.
>>> 2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
>>> 3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules
areused for GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32 
>>> and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen, so
XIDhorizon never moved). 
>> I again vote for #1. A GTT is defined to allow data to be visible only
>> within one session -- so what does it even mean for the data to be
>> accessible on a replica?
>
> There are sessions at replica (in case of hot standby), aren't there?
>
>>
>>> So except the limitation mentioned above (which I do not consider as critical) there is only one problem which was
notaddressed: maintaining statistics for GTT. 
>>> If all of the following conditions are true:
>>>
>>> 1) GTT are used in joins
>>> 2) There are indexes defined for GTT
>>> 3) Size and histogram of GTT in different backends can significantly vary.
>>> 4) ANALYZE was explicitly called for GTT
>>>
>>> then query execution plan built in one backend will be also used for other backends where it can be inefficient.
>>> I also do not consider this problem as "show stopper" for adding GTT to Postgres.
>> I think that's *definitely* a show stopper.
> Well, if both you and Pavel think that it is really "show stopper", then this problem really has to be addressed.
> I slightly confused about this opinion, because Pavel has told me himself that 99% of users never create indexes for
temptables 
> or run "analyze" for them. And without it, this problem is not a problem at all.
>
>>> I still do not understand the opinion of community which functionality of GTT is considered to be most important.
>>> But the patch with local buffers and no replica support is small enough to become good starting point.
>> Well, it seems we now have two patches for this feature. I guess we
>> need to figure out which one is better, and whether it's possible for
>> the two efforts to be merged, rather than having two different teams
>> hacking on separate code bases.
>
> I am open for cooperations.
> Source code of all my patches is available.
We are also willing to cooperate to complete this feature.
Let me prepare the code(merge code to pg12) and up to community, then see how we work together.

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

On Fri, Oct 25, 2019 at 11:14 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> > Access to GTT at replica:
>> > 1. Access is prohibited (as for original temp tables). No changes at all.
>> > 2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
>> > 3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules
areused for GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32
 
>> > and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen,
soXID horizon never moved).
 
>>
>> I again vote for #1. A GTT is defined to allow data to be visible only
>> within one session -- so what does it even mean for the data to be
>> accessible on a replica?
>
> why not? there are lot of sessions on replica servers. One usage of temp tables is fixing estimation errors. You can
createtemp table with partial query result, run ANALYZE and evaluate other steps. Now this case is not possible on
replicaservers.
 
>
> One motivation for GTT  is decreasing port costs from Oracle. But other motivations, like do more complex
calculationson replica are valid and valuable.
 

Hmm, I think I was slightly confused when I wrote my previous
response. I now see that what was under discussion was not making data
from the master visible on the standbys, which really wouldn't make
any sense, but rather allowing standby sessions to also use the GTT,
each with its own local copy of the data. I don't think that's a bad
feature, but look how invasive the required changes are. Not allowing
rollbacks seems dead on arrival; an abort would be able to leave the
table and index mutually inconsistent.  A separate XID space would be
a real solution, perhaps, but it would be *extremely* complicated and
invasive to implement.

One thing that I've learned over and over again as a developer is that
you get a lot more done if you tackle one problem at a time. GTTs are
a sufficiently-large problem all by themselves; a major reworking of
the way XIDs work might be a good project to undertake at some point,
but it doesn't make any sense to incorporate that into the GTT
project, which is otherwise about a mostly-separate set of issues.
Let's not try to solve more problems at once than strictly necessary.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Oct 25, 2019 at 12:22 PM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
> On 25.10.2019 18:01, Robert Haas wrote:
> > On Fri, Oct 11, 2019 at 9:50 AM Konstantin Knizhnik
> > <k.knizhnik@postgrespro.ru> wrote:
> >> Just to clarify.
> >> I have now proposed several different solutions for GTT:
> >>
> >> Shared vs. private buffers for GTT:
> >> 1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
> >> 2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.
> > I vote for #1. I think parallel query for temp objects may be a
> > desirable feature, but I don't think it should be the job of a patch
> > implementing GTTs to make it happen. In fact, I think it would be an
> > actively bad idea, because I suspect that if we do eventually support
> > temp relations for parallel query, we're going to want a solution that
> > is shared between regular temp tables and global temp tables, not
> > separate solutions for each.
>
> Sorry, may be I do not not understand you.
> It seems to me that there is only one thing preventing usage of
> temporary tables in parallel plans: private buffers.
> If global temporary tables are accessed as normal tables though shared
> buffers then them can be used in parallel queries
> and no extra support is required for it.
> At least I have checked that parallel queries are correctly worked for
> my implementation of GTT with shared buffers.
> So I do not understand about which "separate solutions" you are talking
> about.
>
> I can agree that private buffers may be  good starting point for GTT
> implementation, because it is less invasive and GTT access speed is
> exactly the same as of normal temp tables.
> But I do not understand your argument why it is "actively bad idea".

Well, it sounds like you're talking about ending up in a situation
where local temporary tables are still in private buffers, but global
temporary table data is in shared buffers. I think that would be
inconsistent. And it would mean that when somebody wanted to make
local temporary tables accessible in parallel query, they'd have to
write a patch for that.  In other words, I don't support dividing the
patches like this:

Patch #1: Support global temporary tables + allow global temporary
tables to used by parallel query
Patch #2: Allow local temporary tables to be used by parallel query

I support dividing them like this:

Patch #1: Support global temporary tables
Patch #2: Allow (all kinds of) temporary tables to be used by parallel query

The second division looks a lot cleaner to me, although as always I
might be missing something.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 28.10.2019 15:07, Robert Haas wrote:
> On Fri, Oct 25, 2019 at 11:14 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>> Access to GTT at replica:
>>>> 1. Access is prohibited (as for original temp tables). No changes at all.
>>>> 2. Tuples of temp tables are marked with forzen XID.  Minimal changes, rollbacks are not possible.
>>>> 3. Providing special XIDs for GTT at replica. No changes in CLOG are required, but special MVCC visibility rules
areused for GTT. Current limitation: number of transactions accessing GTT at replica is limited by 2^32
 
>>>> and bitmap of correspondent size has to be maintained (tuples of GTT are not proceeded by vacuum and not frozen,
soXID horizon never moved).
 
>>> I again vote for #1. A GTT is defined to allow data to be visible only
>>> within one session -- so what does it even mean for the data to be
>>> accessible on a replica?
>> why not? there are lot of sessions on replica servers. One usage of temp tables is fixing estimation errors. You can
createtemp table with partial query result, run ANALYZE and evaluate other steps. Now this case is not possible on
replicaservers.
 
>>
>> One motivation for GTT  is decreasing port costs from Oracle. But other motivations, like do more complex
calculationson replica are valid and valuable.
 
> Hmm, I think I was slightly confused when I wrote my previous
> response. I now see that what was under discussion was not making data
> from the master visible on the standbys, which really wouldn't make
> any sense, but rather allowing standby sessions to also use the GTT,
> each with its own local copy of the data. I don't think that's a bad
> feature, but look how invasive the required changes are. Not allowing
> rollbacks seems dead on arrival; an abort would be able to leave the
> table and index mutually inconsistent.  A separate XID space would be
> a real solution, perhaps, but it would be *extremely* complicated and
> invasive to implement.

Sorry, but both statements are not true.
As I mentioned before, I have implemented both solutions.

I am not sure how vital is lack of aborts for transactions working with 
GTT at replica.
Some people said that there is no sense in aborts of read-only 
transactions at replica (despite to the fact that them are saving 
intermediate results in GTT).
Some people said something similar with your's "dead on arrival".
But inconsistency is not possible: if such transaction is really 
aborted, then backend is terminated and nobody can see this inconsistency.

Concerning second alternative: you can check yourself that it is not 
*extremely* complicated and invasive.
I extracted changes which are related with handling transactions at 
replica and attached them to this mail.
It is just 500 lines (including diff contexts). Certainly there are some 
limitation of this implementation: number of  transactions working with 
GTT at replica is limited by 2^32
and since GTT tuples are not frozen, analog of GTT CLOG kept in memory 
is never truncated.

>
> One thing that I've learned over and over again as a developer is that
> you get a lot more done if you tackle one problem at a time. GTTs are
> a sufficiently-large problem all by themselves; a major reworking of
> the way XIDs work might be a good project to undertake at some point,
> but it doesn't make any sense to incorporate that into the GTT
> project, which is otherwise about a mostly-separate set of issues.
> Let's not try to solve more problems at once than strictly necessary.
>
I agree with it and think that implementation of GTT with private 
buffers and no replica access is good starting point.

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

On 28.10.2019 15:13, Robert Haas wrote:
> On Fri, Oct 25, 2019 at 12:22 PM Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> On 25.10.2019 18:01, Robert Haas wrote:
>>> On Fri, Oct 11, 2019 at 9:50 AM Konstantin Knizhnik
>>> <k.knizhnik@postgrespro.ru> wrote:
>>>> Just to clarify.
>>>> I have now proposed several different solutions for GTT:
>>>>
>>>> Shared vs. private buffers for GTT:
>>>> 1. Private buffers. This is least invasive patch, requiring no changes in relfilenodes.
>>>> 2. Shared buffers. Requires changing relfilenode but supports parallel query execution for GTT.
>>> I vote for #1. I think parallel query for temp objects may be a
>>> desirable feature, but I don't think it should be the job of a patch
>>> implementing GTTs to make it happen. In fact, I think it would be an
>>> actively bad idea, because I suspect that if we do eventually support
>>> temp relations for parallel query, we're going to want a solution that
>>> is shared between regular temp tables and global temp tables, not
>>> separate solutions for each.
>> Sorry, may be I do not not understand you.
>> It seems to me that there is only one thing preventing usage of
>> temporary tables in parallel plans: private buffers.
>> If global temporary tables are accessed as normal tables though shared
>> buffers then them can be used in parallel queries
>> and no extra support is required for it.
>> At least I have checked that parallel queries are correctly worked for
>> my implementation of GTT with shared buffers.
>> So I do not understand about which "separate solutions" you are talking
>> about.
>>
>> I can agree that private buffers may be  good starting point for GTT
>> implementation, because it is less invasive and GTT access speed is
>> exactly the same as of normal temp tables.
>> But I do not understand your argument why it is "actively bad idea".
> Well, it sounds like you're talking about ending up in a situation
> where local temporary tables are still in private buffers, but global
> temporary table data is in shared buffers. I think that would be
> inconsistent. And it would mean that when somebody wanted to make
> local temporary tables accessible in parallel query, they'd have to
> write a patch for that.  In other words, I don't support dividing the
> patches like this:
>
> Patch #1: Support global temporary tables + allow global temporary
> tables to used by parallel query
> Patch #2: Allow local temporary tables to be used by parallel query
>
> I support dividing them like this:
>
> Patch #1: Support global temporary tables
> Patch #2: Allow (all kinds of) temporary tables to be used by parallel query
>
> The second division looks a lot cleaner to me, although as always I
> might be missing something.
>
Logically it may be good decision. But piratically support of parallel 
access to GTT requires just accessing their data through shared buffer.
But in case of local temp tables we need also need to some how share 
table's metadata between parallel workers. It seems to be much more 
complicated if ever possible.

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

On Mon, Oct 28, 2019 at 9:48 AM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
> Logically it may be good decision. But piratically support of parallel
> access to GTT requires just accessing their data through shared buffer.
> But in case of local temp tables we need also need to some how share
> table's metadata between parallel workers. It seems to be much more
> complicated if ever possible.

Why? The backends all share a snapshot, and can load whatever they
need from the system catalogs.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Oct 28, 2019 at 9:37 AM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
> Sorry, but both statements are not true.

Well, I think they are true.

> I am not sure how vital is lack of aborts for transactions working with
> GTT at replica.
> Some people said that there is no sense in aborts of read-only
> transactions at replica (despite to the fact that them are saving
> intermediate results in GTT).
> Some people said something similar with your's "dead on arrival".
> But inconsistency is not possible: if such transaction is really
> aborted, then backend is terminated and nobody can see this inconsistency.

Aborting the current transaction is a very different thing from
terminating the backend.

Also, the idea that there is no sense in aborts of read-only
transactions on a replica seems totally wrong. Suppose that you insert
a row into the table and then you go to insert a row in each index,
but one of the index inserts fails - duplicate key, out of memory
error, I/O error, whatever. Now the table and the index are
inconsistent. Normally, we're protected against this by MVCC, but if
you use a solution that breaks MVCC by using the same XID for all
transactions, then it can happen.

> Concerning second alternative: you can check yourself that it is not
> *extremely* complicated and invasive.
> I extracted changes which are related with handling transactions at
> replica and attached them to this mail.
> It is just 500 lines (including diff contexts). Certainly there are some
> limitation of this implementation: number of  transactions working with
> GTT at replica is limited by 2^32
> and since GTT tuples are not frozen, analog of GTT CLOG kept in memory
> is never truncated.

I admit that this patch is not lengthy, but there remains the question
of whether it is correct. It's possible that the problem isn't as
complicated as I think it is, but I do think there are quite a number
of reasons why this patch wouldn't be considered acceptable...

> I agree with it and think that implementation of GTT with private
> buffers and no replica access is good starting point.

...but given that we seem to agree on this point, perhaps it isn't
necessary to argue about those things right now.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 28.10.2019 19:40, Robert Haas wrote:
> Aborting the current transaction is a very different thing from
> terminating the backend.
>
> Also, the idea that there is no sense in aborts of read-only
> transactions on a replica seems totally wrong. Suppose that you insert
> a row into the table and then you go to insert a row in each index,
> but one of the index inserts fails - duplicate key, out of memory
> error, I/O error, whatever. Now the table and the index are
> inconsistent. Normally, we're protected against this by MVCC, but if
> you use a solution that breaks MVCC by using the same XID for all
> transactions, then it can happen.


Certainly I understand the difference between abort of transaction and 
termination of backend.
I do not say that it is good solution. And definitely aborts can happen 
for read-only transactions.
I just wanted to express one moment: transaction aborts are caused by 
two reasons:
- expected programming errors: deadlocks, conversion errors, unique 
constraint violation,...
- unexpected system errors: disk space exhaustion, out of memory, I/O 
errors...

Usually at replica with read-only transactions we do not have to deal 
with errors of first kind.
So transaction may be aborted, but such abort most likely means that 
something is wrong with the system
and restart of backend is not so bad solution in this situation.

In any case, I do not insist on this "frozen XID" approach.
The only advantage of this approach is that it is very simple to 
implement: correspondent patch contains just 80 lines of code
and actually it requires just 5 (five) one-line changes.
I didn't agree with your statement just because restart of backend makes 
it not possible to observe any inconsistencies in the database.

> ...but given that we seem to agree on this point, perhaps it isn't
> necessary to argue about those things right now.
>
Ok.
I attached new patch for GTT with local (private) buffer and no replica 
access.
It provides GTT for all built-in indexes


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

>
>> So except the limitation mentioned above (which I do not consider as critical) there is only one problem which was not addressed: maintaining statistics for GTT.
>> If all of the following conditions are true:
>>
>> 1) GTT are used in joins
>> 2) There are indexes defined for GTT
>> 3) Size and histogram of GTT in different backends can significantly vary.
>> 4) ANALYZE was explicitly called for GTT
>>
>> then query execution plan built in one backend will be also used for other backends where it can be inefficient.
>> I also do not consider this problem as "show stopper" for adding GTT to Postgres.
> I think that's *definitely* a show stopper.
Well, if both you and Pavel think that it is really "show stopper", then
this problem really has to be addressed.
I slightly confused about this opinion, because Pavel has told me
himself that 99% of users never create indexes for temp tables
or run "analyze" for them. And without it, this problem is not a problem
at all.

Users doesn't do ANALYZE on temp tables in 99%. It's true. But second fact is so users has lot of problems. It's very similar to wrong statistics on persistent tables. When data are small, then it is not problem for users, although from my perspective it's not optimal. When data are not small, then the problem can be brutal. Temporary tables are not a exception. And users and developers are people - we know only about fatal problems. There are lot of unoptimized queries, but because the problem is not fatal, then it is not reason for report it. And lot of people has not any idea how fast the databases can be. The knowledges of  users and app developers are sad book.

Pavel

It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in backend's catalog cache, but not in pg_statistic table itself.
To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent cache.
I wonder if there are some pitfalls of such approach?

New patch for GTT is attached.

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

On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in
backend'scatalog cache, but not in pg_statistic table itself.
 
> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent cache.
> I wonder if there are some pitfalls of such approach?

That sounds pretty hackish. You'd have to be very careful, for
example, that if the tables were dropped or re-analyzed, all of the
old entries got removed -- and then it would still fail if any code
tried to access the statistics directly from the table, rather than
via the caches. My assumption is that the statistics ought to be
stored in some backend-private data structure designed for that
purpose, and that the code that needs the data should be taught to
look for it there when the table is a GTT.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 01.11.2019 18:26, Robert Haas wrote:
> On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in
backend'scatalog cache, but not in pg_statistic table itself.
 
>> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent
cache.
>> I wonder if there are some pitfalls of such approach?
> That sounds pretty hackish. You'd have to be very careful, for
> example, that if the tables were dropped or re-analyzed, all of the
> old entries got removed --

I have checked it:
- when table is reanalyzed, then cache entries are replaced.
- when table is dropped, then cache entries are removed.

> and then it would still fail if any code
> tried to access the statistics directly from the table, rather than
> via the caches. My assumption is that the statistics ought to be
> stored in some backend-private data structure designed for that
> purpose, and that the code that needs the data should be taught to
> look for it there when the table is a GTT.

Yes, if you do "select * from pg_statistic" then you will not see 
statistic for GTT in this case.
But I do not think that it is so critical. I do not believe that anybody 
is trying to manually interpret values in this table.
And optimizer is retrieving statistic through sys-cache mechanism and so 
is able to build correct plan in this case.

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

On Sat, Nov 2, 2019 at 6:31 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> pá 1. 11. 2019 v 17:09 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:
>>
>> On 01.11.2019 18:26, Robert Haas wrote:
>> > On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
>> > <k.knizhnik@postgrespro.ru> wrote:
>> >> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in
backend'scatalog cache, but not in pg_statistic table itself. 
>> >> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent
cache.
>> >> I wonder if there are some pitfalls of such approach?
>> > That sounds pretty hackish. You'd have to be very careful, for
>> > example, that if the tables were dropped or re-analyzed, all of the
>> > old entries got removed --
>>
>> I have checked it:
>> - when table is reanalyzed, then cache entries are replaced.
>> - when table is dropped, then cache entries are removed.
>>
>> > and then it would still fail if any code
>> > tried to access the statistics directly from the table, rather than
>> > via the caches. My assumption is that the statistics ought to be
>> > stored in some backend-private data structure designed for that
>> > purpose, and that the code that needs the data should be taught to
>> > look for it there when the table is a GTT.
>>
>> Yes, if you do "select * from pg_statistic" then you will not see
>> statistic for GTT in this case.
>> But I do not think that it is so critical. I do not believe that anybody
>> is trying to manually interpret values in this table.
>> And optimizer is retrieving statistic through sys-cache mechanism and so
>> is able to build correct plan in this case.
>
>
> Years ago, when I though about it, I wrote patch with similar design. It's working, but surely it's ugly.
>
> I have another idea. Can be pg_statistics view instead a table?
>
> Some like
>
> SELECT * FROM pg_catalog.pg_statistics_rel
> UNION ALL
> SELECT * FROM pg_catalog.pg_statistics_gtt();
>
> Internally - when stat cache is filled, then there can be used pg_statistics_rel and pg_statistics_gtt() directly.
WhatI remember, there was not possibility to work with queries, only with just relations. 

It'd be a loss if you lose the ability to see the statistics, as there
are valid use cases where you need to see the stats, eg. understanding
why you don't get the plan you wanted.  There's also at least one
extension [1] that allows you to backup and use restored statistics,
so there are definitely people interested in it.

[1]: https://github.com/ossc-db/pg_dbms_stats

On Sat, Nov 2, 2019 at 8:23 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> so 2. 11. 2019 v 8:18 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
>>
>> On Sat, Nov 2, 2019 at 6:31 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> >
>> > pá 1. 11. 2019 v 17:09 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:
>> >>
>> >> On 01.11.2019 18:26, Robert Haas wrote:
>> >> > On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
>> >> > <k.knizhnik@postgrespro.ru> wrote:
>> >> >> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it
inbackend's catalog cache, but not in pg_statistic table itself. 
>> >> >> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent
cache.
>> >> >> I wonder if there are some pitfalls of such approach?
>> >> > That sounds pretty hackish. You'd have to be very careful, for
>> >> > example, that if the tables were dropped or re-analyzed, all of the
>> >> > old entries got removed --
>> >>
>> >> I have checked it:
>> >> - when table is reanalyzed, then cache entries are replaced.
>> >> - when table is dropped, then cache entries are removed.
>> >>
>> >> > and then it would still fail if any code
>> >> > tried to access the statistics directly from the table, rather than
>> >> > via the caches. My assumption is that the statistics ought to be
>> >> > stored in some backend-private data structure designed for that
>> >> > purpose, and that the code that needs the data should be taught to
>> >> > look for it there when the table is a GTT.
>> >>
>> >> Yes, if you do "select * from pg_statistic" then you will not see
>> >> statistic for GTT in this case.
>> >> But I do not think that it is so critical. I do not believe that anybody
>> >> is trying to manually interpret values in this table.
>> >> And optimizer is retrieving statistic through sys-cache mechanism and so
>> >> is able to build correct plan in this case.
>> >
>> >
>> > Years ago, when I though about it, I wrote patch with similar design. It's working, but surely it's ugly.
>> >
>> > I have another idea. Can be pg_statistics view instead a table?
>> >
>> > Some like
>> >
>> > SELECT * FROM pg_catalog.pg_statistics_rel
>> > UNION ALL
>> > SELECT * FROM pg_catalog.pg_statistics_gtt();
>> >
>> > Internally - when stat cache is filled, then there can be used pg_statistics_rel and pg_statistics_gtt() directly.
WhatI remember, there was not possibility to work with queries, only with just relations. 
>>
>> It'd be a loss if you lose the ability to see the statistics, as there
>> are valid use cases where you need to see the stats, eg. understanding
>> why you don't get the plan you wanted.  There's also at least one
>> extension [1] that allows you to backup and use restored statistics,
>> so there are definitely people interested in it.
>>
>> [1]: https://github.com/ossc-db/pg_dbms_stats
>
>
> I don't think - the extensions can use UNION and the content will be same as caches used by planner.

Yes, I agree that changing pg_statistics to be a view as you showed
would fix the problem.  I was answering Konstantin's point:

>> >> But I do not think that it is so critical. I do not believe that anybody
>> >> is trying to manually interpret values in this table.
>> >> And optimizer is retrieving statistic through sys-cache mechanism and so
>> >> is able to build correct plan in this case.

which is IMHO a wrong assumption.

On 02.11.2019 10:19, Julien Rouhaud wrote:
> On Sat, Nov 2, 2019 at 6:31 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> pá 1. 11. 2019 v 17:09 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:
>>> On 01.11.2019 18:26, Robert Haas wrote:
>>>> On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
>>>> <k.knizhnik@postgrespro.ru> wrote:
>>>>> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in
backend'scatalog cache, but not in pg_statistic table itself.
 
>>>>> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent
cache.
>>>>> I wonder if there are some pitfalls of such approach?
>>>> That sounds pretty hackish. You'd have to be very careful, for
>>>> example, that if the tables were dropped or re-analyzed, all of the
>>>> old entries got removed --
>>> I have checked it:
>>> - when table is reanalyzed, then cache entries are replaced.
>>> - when table is dropped, then cache entries are removed.
>>>
>>>> and then it would still fail if any code
>>>> tried to access the statistics directly from the table, rather than
>>>> via the caches. My assumption is that the statistics ought to be
>>>> stored in some backend-private data structure designed for that
>>>> purpose, and that the code that needs the data should be taught to
>>>> look for it there when the table is a GTT.
>>> Yes, if you do "select * from pg_statistic" then you will not see
>>> statistic for GTT in this case.
>>> But I do not think that it is so critical. I do not believe that anybody
>>> is trying to manually interpret values in this table.
>>> And optimizer is retrieving statistic through sys-cache mechanism and so
>>> is able to build correct plan in this case.
>>
>> Years ago, when I though about it, I wrote patch with similar design. It's working, but surely it's ugly.
>>
>> I have another idea. Can be pg_statistics view instead a table?
>>
>> Some like
>>
>> SELECT * FROM pg_catalog.pg_statistics_rel
>> UNION ALL
>> SELECT * FROM pg_catalog.pg_statistics_gtt();
>>
>> Internally - when stat cache is filled, then there can be used pg_statistics_rel and pg_statistics_gtt() directly.
WhatI remember, there was not possibility to work with queries, only with just relations.
 
> It'd be a loss if you lose the ability to see the statistics, as there
> are valid use cases where you need to see the stats, eg. understanding
> why you don't get the plan you wanted.  There's also at least one
> extension [1] that allows you to backup and use restored statistics,
> so there are definitely people interested in it.
>
> [1]: https://github.com/ossc-db/pg_dbms_stats
It seems to have completely no sense to backup and restore statistic for 
temporary tables which life time is limited to life time of backend,
doesn't it?

pá 1. 11. 2019 v 17:09 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:

On 01.11.2019 18:26, Robert Haas wrote:
> On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it in backend's catalog cache, but not in pg_statistic table itself.
>> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent cache.
>> I wonder if there are some pitfalls of such approach?
> That sounds pretty hackish. You'd have to be very careful, for
> example, that if the tables were dropped or re-analyzed, all of the
> old entries got removed --

I have checked it:
- when table is reanalyzed, then cache entries are replaced.
- when table is dropped, then cache entries are removed.

> and then it would still fail if any code
> tried to access the statistics directly from the table, rather than
> via the caches. My assumption is that the statistics ought to be
> stored in some backend-private data structure designed for that
> purpose, and that the code that needs the data should be taught to
> look for it there when the table is a GTT.

Yes, if you do "select * from pg_statistic" then you will not see
statistic for GTT in this case.
But I do not think that it is so critical. I do not believe that anybody
is trying to manually interpret values in this table.
And optimizer is retrieving statistic through sys-cache mechanism and so
is able to build correct plan in this case.

Years ago, when I though about it, I wrote patch with similar design. It's working, but surely it's ugly.

I have another idea. Can be pg_statistics view instead a table?

Some like

SELECT * FROM pg_catalog.pg_statistics_rel

UNION ALL

SELECT * FROM pg_catalog.pg_statistics_gtt();

And pg_catalog.pg_statistics_gtt() is set returning functions?
I afraid that it is not acceptable solution from performance point of view: pg_statictic table is accessed by keys (<relid>,<attpos>,<inh>)
If it can not be done using index scan, then it can cause significant performance slow down.

Internally - when stat cache is filled, then there can be used pg_statistics_rel and pg_statistics_gtt() directly. What I remember, there was not possibility to work with queries, only with just relations.

Or crazy idea - today we can implement own types of heaps. Is possible to create engine where result can be combination of some shared data and local data. So union will be implemented on heap level.

This implementation can be simple, just scanning pages from shared buffers and from local buffers. For these tables we don't need complex metadata. It's crazy idea, and I think so union with table function should be best.

Frankly speaking, implementing special heap access method for pg_statistic just to handle case of global temp tables seems to be overkill
from my point of view. It requires a lot coding (or at least copying a lot of code from heapam). Also, as I wrote above, we need also index for efficient lookup of statistic.

On Sat, Nov 2, 2019 at 4:09 PM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
>
> On 02.11.2019 10:19, Julien Rouhaud wrote:
> > On Sat, Nov 2, 2019 at 6:31 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> >> pá 1. 11. 2019 v 17:09 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:
> >>> On 01.11.2019 18:26, Robert Haas wrote:
> >>>> On Fri, Nov 1, 2019 at 11:15 AM Konstantin Knizhnik
> >>>> <k.knizhnik@postgrespro.ru> wrote:
> >>>>> It seems to me that I have found quite elegant solution for per-backend statistic for GTT: I just inserting it
inbackend's catalog cache, but not in pg_statistic table itself. 
> >>>>> To do it I have to add InsertSysCache/InsertCatCache functions which insert pinned entry in the correspondent
cache.
> >>>>> I wonder if there are some pitfalls of such approach?
> >>>> That sounds pretty hackish. You'd have to be very careful, for
> >>>> example, that if the tables were dropped or re-analyzed, all of the
> >>>> old entries got removed --
> >>> I have checked it:
> >>> - when table is reanalyzed, then cache entries are replaced.
> >>> - when table is dropped, then cache entries are removed.
> >>>
> >>>> and then it would still fail if any code
> >>>> tried to access the statistics directly from the table, rather than
> >>>> via the caches. My assumption is that the statistics ought to be
> >>>> stored in some backend-private data structure designed for that
> >>>> purpose, and that the code that needs the data should be taught to
> >>>> look for it there when the table is a GTT.
> >>> Yes, if you do "select * from pg_statistic" then you will not see
> >>> statistic for GTT in this case.
> >>> But I do not think that it is so critical. I do not believe that anybody
> >>> is trying to manually interpret values in this table.
> >>> And optimizer is retrieving statistic through sys-cache mechanism and so
> >>> is able to build correct plan in this case.
> >>
> >> Years ago, when I though about it, I wrote patch with similar design. It's working, but surely it's ugly.
> >>
> >> I have another idea. Can be pg_statistics view instead a table?
> >>
> >> Some like
> >>
> >> SELECT * FROM pg_catalog.pg_statistics_rel
> >> UNION ALL
> >> SELECT * FROM pg_catalog.pg_statistics_gtt();
> >>
> >> Internally - when stat cache is filled, then there can be used pg_statistics_rel and pg_statistics_gtt() directly.
WhatI remember, there was not possibility to work with queries, only with just relations. 
> > It'd be a loss if you lose the ability to see the statistics, as there
> > are valid use cases where you need to see the stats, eg. understanding
> > why you don't get the plan you wanted.  There's also at least one
> > extension [1] that allows you to backup and use restored statistics,
> > so there are definitely people interested in it.
> >
> > [1]: https://github.com/ossc-db/pg_dbms_stats
> It seems to have completely no sense to backup and restore statistic for
> temporary tables which life time is limited to life time of backend,
> doesn't it?

In general yes I agree, but it doesn't if the goal is to understand
why even after an analyze on the temporary table your query is still
behaving poorly.  It can be useful to allow reproduction or just give
someone else the statistics to see what's going on.

Dear Hackers


I attached the patch of GTT implementationI base on PG12.
The GTT design came from my first email.
Some limitations in patch will be eliminated in later versions.

Later, I will comment on Konstantin's patch and make some proposals for cooperation.
Looking forward to your feedback.

Thanks.

Zeng Wenjing





> 2019年10月29日 上午12:40，Robert Haas <robertmhaas@gmail.com> 写道：
>
> On Mon, Oct 28, 2019 at 9:37 AM Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> Sorry, but both statements are not true.
>
> Well, I think they are true.
>
>> I am not sure how vital is lack of aborts for transactions working with
>> GTT at replica.
>> Some people said that there is no sense in aborts of read-only
>> transactions at replica (despite to the fact that them are saving
>> intermediate results in GTT).
>> Some people said something similar with your's "dead on arrival".
>> But inconsistency is not possible: if such transaction is really
>> aborted, then backend is terminated and nobody can see this inconsistency.
>
> Aborting the current transaction is a very different thing from
> terminating the backend.
>
> Also, the idea that there is no sense in aborts of read-only
> transactions on a replica seems totally wrong. Suppose that you insert
> a row into the table and then you go to insert a row in each index,
> but one of the index inserts fails - duplicate key, out of memory
> error, I/O error, whatever. Now the table and the index are
> inconsistent. Normally, we're protected against this by MVCC, but if
> you use a solution that breaks MVCC by using the same XID for all
> transactions, then it can happen.
>
>> Concerning second alternative: you can check yourself that it is not
>> *extremely* complicated and invasive.
>> I extracted changes which are related with handling transactions at
>> replica and attached them to this mail.
>> It is just 500 lines (including diff contexts). Certainly there are some
>> limitation of this implementation: number of  transactions working with
>> GTT at replica is limited by 2^32
>> and since GTT tuples are not frozen, analog of GTT CLOG kept in memory
>> is never truncated.
>
> I admit that this patch is not lengthy, but there remains the question
> of whether it is correct. It's possible that the problem isn't as
> complicated as I think it is, but I do think there are quite a number
> of reasons why this patch wouldn't be considered acceptable...
>
>> I agree with it and think that implementation of GTT with private
>> buffers and no replica access is good starting point.
>
> ...but given that we seem to agree on this point, perhaps it isn't
> necessary to argue about those things right now.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

On 06.11.2019 16:24, 曾文旌(义从) wrote:
> Dear Hackers
>
>
> I attached the patch of GTT implementationI base on PG12.
> The GTT design came from my first email.
> Some limitations in patch will be eliminated in later versions.
>
> Later, I will comment on Konstantin's patch and make some proposals for cooperation.
> Looking forward to your feedback.
>
> Thanks.
>
> Zeng Wenjing
>

Thank you for this patch.
My first comments:

1.  I have ported you patch to the latest Postgres version (my patch is 
attached).
2. You patch is supporting only B-Tree index for GTT. All other indexes 
(hash, gin, gist, brin,...) are not currently supported.
3. I do not understand the reason for the following limitation:
"We allow to create index on global temp table only this session use it"

First of all it seems to significantly reduce usage of global temp tables.
Why do we need GTT at all? Mostly because we need to access temporary 
data in more than one backend. Otherwise we can just use normal table.
If temp table is expected to be larger enough, so that we need to create 
index for it, then it is hard to believe that it will be needed only in 
one backend.

May be the assumption is that all indexes has to be created before GTT 
start to be used.
But right now this check is not working correctly in any case - if you 
insert some data into the table, then
you can not create index any more:

postgres=# create global temp table gtt(x integer primary key, y integer);
CREATE TABLE
postgres=# insert into gtt values (generate_series(1,100000), 
generate_series(1,100000));
INSERT 0 100000
postgres=# create index on gtt(y);
ERROR:  can not create index when have one or more backend attached this 
global temp table

I wonder why do you need such restriction?


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

> 2019年11月7日 上午12:08，Konstantin Knizhnik <k.knizhnik@postgrespro.ru> 写道：
>
>
>
> On 06.11.2019 16:24, 曾文旌(义从) wrote:
>> Dear Hackers
>>
>>
>> I attached the patch of GTT implementationI base on PG12.
>> The GTT design came from my first email.
>> Some limitations in patch will be eliminated in later versions.
>>
>> Later, I will comment on Konstantin's patch and make some proposals for cooperation.
>> Looking forward to your feedback.
>>
>> Thanks.
>>
>> Zeng Wenjing
>>
>
> Thank you for this patch.
> My first comments:
>
> 1.  I have ported you patch to the latest Postgres version (my patch is attached).
> 2. You patch is supporting only B-Tree index for GTT. All other indexes (hash, gin, gist, brin,...) are not currently
supported.
Currently I only support btree index.
I noticed that your patch supports more index types, which is where I'd like to work with you.

> 3. I do not understand the reason for the following limitation:
> "We allow to create index on global temp table only this session use it"
>
> First of all it seems to significantly reduce usage of global temp tables.
> Why do we need GTT at all? Mostly because we need to access temporary data in more than one backend. Otherwise we can
justuse normal table. 
> If temp table is expected to be larger enough, so that we need to create index for it, then it is hard to believe
thatit will be needed only in one backend. 
>
> May be the assumption is that all indexes has to be created before GTT start to be used.
Yes, Currently, GTT's index is only supported and created in an empty table state, and other sessions are not using it.
There has two improvements pointer:
1 Index can create on GTT(A) when the GTT(A)  in the current session is not empty, requiring the GTT table to be empty
inthe other session. 
Index_build needs to be done in the current session just like a normal table. This improvement is relatively easy.

2 Index can create on GTT(A)  when more than one session are using this GTT(A).
Because when I'm done creating an index of the GTT in this session and setting it to be an valid index, it's not true
forthe GTT in other sessions. 
Indexes on gtt in other sessions require "rebuild_index" before using it.
I don't have a better solution right now, maybe you have some suggestions.


> But right now this check is not working correctly in any case - if you insert some data into the table, then
> you can not create index any more:
>
> postgres=# create global temp table gtt(x integer primary key, y integer);
> CREATE TABLE
> postgres=# insert into gtt values (generate_series(1,100000), generate_series(1,100000));
> INSERT 0 100000
> postgres=# create index on gtt(y);
> ERROR:  can not create index when have one or more backend attached this global temp table
>
> I wonder why do you need such restriction?
>
>
> --
> Konstantin Knizhnik
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
> <global_temporary_table_v1-pg13.patch>

On 07.11.2019 12:30, 曾文旌(义从) wrote:
>
>> May be the assumption is that all indexes has to be created before GTT start to be used.
> Yes, Currently, GTT's index is only supported and created in an empty table state, and other sessions are not using
it.
> There has two improvements pointer:
> 1 Index can create on GTT(A) when the GTT(A)  in the current session is not empty, requiring the GTT table to be
emptyin the other session.
 
> Index_build needs to be done in the current session just like a normal table. This improvement is relatively easy.
>
> 2 Index can create on GTT(A)  when more than one session are using this GTT(A).
> Because when I'm done creating an index of the GTT in this session and setting it to be an valid index, it's not true
forthe GTT in other sessions.
 
> Indexes on gtt in other sessions require "rebuild_index" before using it.
> I don't have a better solution right now, maybe you have some suggestions.
It is possible to create index on demand:

Buffer
_bt_getbuf(Relation rel, BlockNumber blkno, int access)
{
     Buffer        buf;

     if (blkno != P_NEW)
     {
         /* Read an existing block of the relation */
         buf = ReadBuffer(rel, blkno);
         /* Session temporary relation may be not yet initialized for 
this backend. */
         if (blkno == BTREE_METAPAGE && 
GlobalTempRelationPageIsNotInitialized(rel, BufferGetPage(buf)))
         {
             Relation heap = RelationIdGetRelation(rel->rd_index->indrelid);
             ReleaseBuffer(buf);
             DropRelFileNodeLocalBuffers(rel->rd_node, MAIN_FORKNUM, blkno);
             btbuild(heap, rel, BuildIndexInfo(rel));
             RelationClose(heap);
             buf = ReadBuffer(rel, blkno);
             LockBuffer(buf, access);
         }
         else
         {
             LockBuffer(buf, access);
             _bt_checkpage(rel, buf);
         }
     }
     ...


This code initializes B-Tree and load data in it when GTT index is 
access and is not initialized yet.
It looks a little bit hacker but it works.

I also wonder why you are keeping information about GTT in shared 
memory. Looks like the only information we really need to share is 
table's metadata.
But it is already shared though catalog. All other GTT related 
information is private to backend so I do not see reasons to place it in 
shared memory.

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

> 2019年11月8日 上午12:32，Konstantin Knizhnik <k.knizhnik@postgrespro.ru> 写道：
>
>
>
> On 07.11.2019 12:30, 曾文旌(义从) wrote:
>>
>>> May be the assumption is that all indexes has to be created before GTT start to be used.
>> Yes, Currently, GTT's index is only supported and created in an empty table state, and other sessions are not using
it.
>> There has two improvements pointer:
>> 1 Index can create on GTT(A) when the GTT(A)  in the current session is not empty, requiring the GTT table to be
emptyin the other session. 
>> Index_build needs to be done in the current session just like a normal table. This improvement is relatively easy.
>>
>> 2 Index can create on GTT(A)  when more than one session are using this GTT(A).
>> Because when I'm done creating an index of the GTT in this session and setting it to be an valid index, it's not
truefor the GTT in other sessions. 
>> Indexes on gtt in other sessions require "rebuild_index" before using it.
>> I don't have a better solution right now, maybe you have some suggestions.
> It is possible to create index on demand:
>
> Buffer
> _bt_getbuf(Relation rel, BlockNumber blkno, int access)
> {
>     Buffer        buf;
>
>     if (blkno != P_NEW)
>     {
>         /* Read an existing block of the relation */
>         buf = ReadBuffer(rel, blkno);
>         /* Session temporary relation may be not yet initialized for this backend. */
>         if (blkno == BTREE_METAPAGE && GlobalTempRelationPageIsNotInitialized(rel, BufferGetPage(buf)))
>         {
>             Relation heap = RelationIdGetRelation(rel->rd_index->indrelid);
>             ReleaseBuffer(buf);
>             DropRelFileNodeLocalBuffers(rel->rd_node, MAIN_FORKNUM, blkno);
>             btbuild(heap, rel, BuildIndexInfo(rel));
>             RelationClose(heap);
>             buf = ReadBuffer(rel, blkno);
>             LockBuffer(buf, access);
>         }
>         else
>         {
>             LockBuffer(buf, access);
>             _bt_checkpage(rel, buf);
>         }
>     }
>     ...
In my opinion, it is not a good idea to trigger a btbuild with a select or DML, the cost of which depends on the amount
ofdata in the GTT. 

>
>
> This code initializes B-Tree and load data in it when GTT index is access and is not initialized yet.
> It looks a little bit hacker but it works.
>
> I also wonder why you are keeping information about GTT in shared memory. Looks like the only information we really
needto share is table's metadata. 
> But it is already shared though catalog. All other GTT related information is private to backend so I do not see
reasonsto place it in shared memory. 
The shared hash structure tracks which backend has initialized the GTT storage in order to implement the DDL of the
GTT.
As for GTT, there is only one definition(include index on GTT), but each backend may have one data.
For the implementation of drop GTT, I assume that all data and definitions need to be deleted.

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

On 08.11.2019 10:50, 曾文旌(义从) wrote:
> In my opinion, it is not a good idea to trigger a btbuild with a select or DML, the cost of which depends on the
amountof data in the GTT.
 
IMHO it is better than returning error.
Also index will be used only if cost of plan with index will be 
considered better than cost of plan without index. If you do not have 
index, then you have to scan the whole table.
Time of such scan is comparable with time of building index.

Yes, I agree that indexes for GTT are used to be created together with 
table itself before it is used by any application.
But if later DBA recognized that efficient execution of queries requires 
some more indexes,
it will be strange and dangerous to prevent him from adding such index 
until all clients which have accessed this table will drop their 
connections.
Also maintaining in shared memory information about attached backends 
seems to be overkill.

>>
>> This code initializes B-Tree and load data in it when GTT index is access and is not initialized yet.
>> It looks a little bit hacker but it works.
>>
>> I also wonder why you are keeping information about GTT in shared memory. Looks like the only information we really
needto share is table's metadata.
 
>> But it is already shared though catalog. All other GTT related information is private to backend so I do not see
reasonsto place it in shared memory.
 
> The shared hash structure tracks which backend has initialized the GTT storage in order to implement the DDL of the
GTT.
Sorry, I do not understand this argument.
DDL is performed on shared metadata present in global catalog.
Standard postgres invalidation mechanism is used to notify all backends 
about schema changes.
Why do we need to maintain some extra information in shared memory.
Can you give me example of DLL which does't work without such shared hash?

> As for GTT, there is only one definition(include index on GTT), but each backend may have one data.
> For the implementation of drop GTT, I assume that all data and definitions need to be deleted.

Data of dropped GTT is removed on normal backend termination or cleaned 
up at server restart in case of abnormal shutdown (as it is done for 
local temp tables).
I have not used any shared control structures for GTT in my 
implementation and that is why I wonder why do you need it and what are 
the expected problems with my
implementation?

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

My comments for global_private_temp-4.patch

Thank you very much for inspecting my patch.

good side:

1 Lots of  index type on GTT. I think we need support for all kinds of indexes.

2 serial column on GTT.

3 INHERITS GTT.

4 PARTITION GTT.

I didn't choose to support them in the first release, but you did.

Other side:

1 case: create global temp table gtt2(a int primary key, b text) on commit delete rows;

I think you've lost the meaning of the on commit delete rows clause.

After the GTT is created, the other sessions feel that this is an on commit PRESERVE rows GTT.

Yes, there was bug in my implementation of ON COMMIT DELETE ROWS for GTT.
It is fixed in global_private_temp-6.patch

2 truncate gtt, mybe this is a bug in DropRelFileNodeBuffers.

GTT's local buffer is not released.

Case:

postgres=# insert into gtt2 values(1,'xx');

INSERT 0 1

postgres=# truncate gtt2;

TRUNCATE TABLE

postgres=# insert into gtt2 values(1,'xx');

ERROR:  unexpected data beyond EOF in block 0 of relation base/13579/t3_16384

HINT:  This has been seen to occur with buggy kernels; consider updating your system.

Yes another bug, also fixed in new version of the patch.

3  lock type of truncate GTT.

I don't think it's a good idea to hold a big lock with truncate GTT, because it only needs to process private data.

Sorry, I do not understand which lock you are talking about.
I have not introduced any special locks for GTT.

4 GTT's ddl Those ddl that need to rewrite data files may need attention.

We have discussed in the previous email. This is why I used shared hash to track the GTT file.

You are right.
But instead of prohibiting ALTER TABLE at all for GTT, we can check
that there are no other backends using it.
I do not think that we should maintain some hash in shared memory to check it.
As far as ALTER TABLE is rare and slow operation in any case, we can just check presence of GTT files
created by other backends.
I have implemented this check in global_private_temp-6.patch

5 There will be problems with DDL that will change relfilenode. Such as cluster GTT ,vacuum full GTT.

A session completes vacuum full gtt(a), and other sessions will immediately start reading and writing new storage files and existing data is also lost.

I disable them in my current version.

Thank you for noticing it.
Autovacuum full should really be prohibited for GTT.

6 drop GTT

I think drop GTT should clean up all storage files and definitions. How do you think?

Storage files will be cleaned in any case on backend termination.
Certainly if backend creates  and deletes huge number of GTT in the loop, it can cause space exhaustion.
But it seems to be very strange pattern of GTT usage.

7 MVCC visibility clog clean

GTT data visibility rules, like regular tables, so GTT also need clog.

We need to avoid the clog that GTT needs to be cleaned up. 

At the same time, GTT does not do autovacuum, and retaining "too old data" will cause wraparound data loss.

I have given a solution in my design.

But why do we need some special handling of visibility rules for GTT comparing with normal (local) temp tables?
Them are also not proceeded by autovacuum?

In principle, I have also implemented special visibility rules for GTT, but only for the case when them
are accessed at replica. And it is not included in this patch, because everybody think that access to GTT
replica should be considered in separate patch.

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

Hi,

I think we need to do something with having two patches aiming to add
global temporary tables:

[1] https://commitfest.postgresql.org/26/2349/

[2] https://commitfest.postgresql.org/26/2233/

As a reviewer I have no idea which of the threads to look at - certainly
not without reading both threads, which I doubt anyone will really do.
The reviews and discussions are somewhat intermixed between those two
threads, which makes it even more confusing.

I think we should agree on a minimal patch combining the necessary/good
bits from the various patches, and terminate one of the threads (i.e.
mark it as rejected or RWF). And we need to do that now, otherwise
there's about 0% chance of getting this into v13.

In general, I agree with the sentiment Rober expressed in [1] - the
patch needs to be as small as possible, not adding "nice to have"
features (like support for parallel queries - I very much doubt just
using shared instead of local buffers is enough to make it work.)

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In the previous communication

1 we agreed on the general direction
1.1 gtt use local (private) buffer
1.2 no replica access in first version

2 We feel that gtt needs to maintain statistics, but there is no agreement on what it will be done.

3 Still no one commented on GTT's transaction information processing, they include
3.1 Should gtt's frozenxid need to be care?
3.2 gtt’s clog clean
3.3 How to deal with "too old" gtt data

I suggest we discuss further, reach an agreement, and merge the two patches to one.


Wenjing


> 2020年1月6日 上午4:06，Tomas Vondra <tomas.vondra@2ndquadrant.com> 写道：
>
> Hi,
>
> I think we need to do something with having two patches aiming to add
> global temporary tables:
>
> [1] https://commitfest.postgresql.org/26/2349/
>
> [2] https://commitfest.postgresql.org/26/2233/
>
> As a reviewer I have no idea which of the threads to look at - certainly
> not without reading both threads, which I doubt anyone will really do.
> The reviews and discussions are somewhat intermixed between those two
> threads, which makes it even more confusing.
>
> I think we should agree on a minimal patch combining the necessary/good
> bits from the various patches, and terminate one of the threads (i.e.
> mark it as rejected or RWF). And we need to do that now, otherwise
> there's about 0% chance of getting this into v13.
>
> In general, I agree with the sentiment Rober expressed in [1] - the
> patch needs to be as small as possible, not adding "nice to have"
> features (like support for parallel queries - I very much doubt just
> using shared instead of local buffers is enough to make it work.)
>
> regards
>
> --
> Tomas Vondra                  http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Jan 06, 2020 at 01:04:15PM +0800, 曾文旌(义从) wrote:
>In the previous communication
>
>1 we agreed on the general direction
>1.1 gtt use local (private) buffer
>1.2 no replica access in first version
>

OK, good.

>2 We feel that gtt needs to maintain statistics, but there is no
>agreement on what it will be done.
>

I certainly agree GTT needs to maintain statistics, otherwise it'll lead
to poor query plans. AFAIK the current patch stores the info in a hash
table in a backend private memory, and I don't see how else to do that
(e.g. storing it in a catalog would cause catalog bloat).

FWIW this is a reasons why I think just using shared buffers (instead of
local ones) is not sufficient to support parallel queriesl as proposed
by Alexander. The workers would not know the stats, breaking planning of
queries in PARALLEL SAFE plpgsql functions etc.

>3 Still no one commented on GTT's transaction information processing, they include
>3.1 Should gtt's frozenxid need to be care?
>3.2 gtt’s clog clean
>3.3 How to deal with "too old" gtt data
>

No idea what to do about this.

>I suggest we discuss further, reach an agreement, and merge the two patches to one.
>

OK, cool. Thanks for the clarification.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, 6 Jan 2020 at 11:01, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>
> On Mon, Jan 06, 2020 at 01:04:15PM +0800, 曾文旌(义从) wrote:
>
> >2 We feel that gtt needs to maintain statistics, but there is no
> >agreement on what it will be done.
> >
>
> I certainly agree GTT needs to maintain statistics, otherwise it'll lead
> to poor query plans.

+1

> AFAIK the current patch stores the info in a hash
> table in a backend private memory, and I don't see how else to do that
> (e.g. storing it in a catalog would cause catalog bloat).
>

It sounds like it needs a pair of system GTTs to hold the table and
column statistics for other GTTs. One would probably have the same
columns as pg_statistic, and the other just the relevant columns from
pg_class. I can see it being useful for the user to be able to see
these stats, so perhaps they could be UNIONed into the existing stats
view.

Regards,
Dean

On Mon, Jan 06, 2020 at 12:17:43PM +0000, Dean Rasheed wrote:
>On Mon, 6 Jan 2020 at 11:01, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>
>> On Mon, Jan 06, 2020 at 01:04:15PM +0800, 曾文旌(义从) wrote:
>>
>> >2 We feel that gtt needs to maintain statistics, but there is no
>> >agreement on what it will be done.
>> >
>>
>> I certainly agree GTT needs to maintain statistics, otherwise it'll lead
>> to poor query plans.
>
>+1
>
>> AFAIK the current patch stores the info in a hash
>> table in a backend private memory, and I don't see how else to do that
>> (e.g. storing it in a catalog would cause catalog bloat).
>>
>
>It sounds like it needs a pair of system GTTs to hold the table and
>column statistics for other GTTs. One would probably have the same
>columns as pg_statistic, and the other just the relevant columns from
>pg_class. I can see it being useful for the user to be able to see
>these stats, so perhaps they could be UNIONed into the existing stats
>view.
>

Hmmm, yeah. A "temporary catalog" (not sure if it can work exactly the
same as GTT) storing pg_statistics data for GTTs might work, I think. It
would not have the catalog bloat issue, which is good.

I still think we'd need to integrate this with the regular pg_statistic
catalogs somehow, so that people don't have to care about two things. I
mean, extensions like hypopg do use pg_statistic data to propose indexes
etc. and it would be nice if we don't make them more complicated.

Not sure why we'd need a temporary version of pg_class, though?


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 06.01.2020 8:04, 曾文旌(义从) wrote:
> In the previous communication
>
> 1 we agreed on the general direction
> 1.1 gtt use local (private) buffer
> 1.2 no replica access in first version
>
> 2 We feel that gtt needs to maintain statistics, but there is no agreement on what it will be done.
>
> 3 Still no one commented on GTT's transaction information processing, they include
> 3.1 Should gtt's frozenxid need to be care?
> 3.2 gtt’s clog clean
> 3.3 How to deal with "too old" gtt data
>
> I suggest we discuss further, reach an agreement, and merge the two patches to one.
>

I also hope that we should come to the common solution for GTT.
If we do not try to address parallel execution issues and access to temp 
tables at replicas (and I agreed
that it should be avoided in first version of the patch), then GTT patch 
becomes quite small.

The most complex and challenged task is to support GTT for all kind of 
indexes. Unfortunately I can not proposed some good universal solution 
for it.
Just patching all existed indexes implementation seems to be the only 
choice.

Statistic is another important case.
But once again I do not completely understand why we want to address all 
this issues with statistic in first version of the patch? It contradicts 
to the idea to make this patch as small as possible.
Also it seems to me that everybody agreed that users very rarely create 
indexes for temp tables and explicitly analyze them.
So I think GTT will be useful even with limited support of statistic. In 
my version statistics for GTT is provided by pushing correspondent 
information to backend's cache for pg_statistic table.
Also I provided pg_temp_statistic view for inspecting it by users. The 
idea to make pg_statistic a view which combines statistic of normal and 
temporary tables is overkill from my point of view.

I do not understand why do we need to maintain hash with some extra 
information for GTT in backends memory (as it was done in Wenjing patch).
Also idea to use create extension for accessing this information seems 
to be dubious.

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

On 06.01.2020 14:01, Tomas Vondra wrote:
> On Mon, Jan 06, 2020 at 01:04:15PM +0800, 曾文旌(义从) wrote:
>> In the previous communication
>>
>> 1 we agreed on the general direction
>> 1.1 gtt use local (private) buffer
>> 1.2 no replica access in first version
>>
>
> OK, good.
>
>> 2 We feel that gtt needs to maintain statistics, but there is no
>> agreement on what it will be done.
>>
>
> I certainly agree GTT needs to maintain statistics, otherwise it'll lead
> to poor query plans. AFAIK the current patch stores the info in a hash
> table in a backend private memory, and I don't see how else to do that
> (e.g. storing it in a catalog would cause catalog bloat).
>
> FWIW this is a reasons why I think just using shared buffers (instead of
> local ones) is not sufficient to support parallel queriesl as proposed
> by Alexander. The workers would not know the stats, breaking planning of
> queries in PARALLEL SAFE plpgsql functions etc.


I do not think that "all or nothing" approach is so good for software 
development as for database transactions.
Yes, if we have function in PL/pgSQL which performs queries om temporary 
tables, then
parallel workers may build inefficient plan for this queries due to lack 
of statistics.
 From my point of view this is not a pitfall of GTT but result of lack 
of global plan cache in Postgres. And it should be fixed not at GTT level.

Also I never see real use cases with such functions, even in the systems 
which using hard temporary tables and stored procedures.
But there are many other real problems with temp tables  (except already 
mentioned in this thread).
In PgPro/EE we have fixes for some of them, for example:

1. Do not reserve space in the file for temp relations. Right now append 
of relation cause writing zero page to the disk by mdextend.
It cause useless disk IO for temp tables which in most cases fit in 
memory and should not be written at disk.

2. Implicitly perform analyze of temp table intermediately after storing 
data in it. Usually tables are analyzed by autovacuum in background.
But it doesn't work for temp tables which are not processes by 
autovacuum and are accessed immediately after filling them with data and 
lack of statistic  may cause
building very inefficient plan. We have online_analyze extension which 
force analyze of the table after appending some bulk of data to it.
It can be used for normal table but most of all it is useful for temp 
relations.

Unlike hypothetical example with parallel safe function working with 
temp tables,
this are real problems observed by some of our customers.
Them are applicable both to local and global temp tables and this is why 
I do not want to discuss them in context of GTT.


>
>> 3 Still no one commented on GTT's transaction information processing, 
>> they include
>> 3.1 Should gtt's frozenxid need to be care?
>> 3.2 gtt’s clog clean
>> 3.3 How to deal with "too old" gtt data
>>
>
> No idea what to do about this.
>

I wonder what is the specific of GTT here?
The same problem takes place for normal (local) temp tables, doesn't it?


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

On Thu, Jan 09, 2020 at 06:07:46PM +0300, Konstantin Knizhnik wrote:
>
>
>On 06.01.2020 14:01, Tomas Vondra wrote:
>>On Mon, Jan 06, 2020 at 01:04:15PM +0800, 曾文旌(义从) wrote:
>>>In the previous communication
>>>
>>>1 we agreed on the general direction 1.1 gtt use local (private)
>>>buffer 1.2 no replica access in first version
>>>
>>
>>OK, good.
>>
>>>2 We feel that gtt needs to maintain statistics, but there is no
>>>agreement on what it will be done.
>>>
>>
>>I certainly agree GTT needs to maintain statistics, otherwise it'll
>>lead to poor query plans. AFAIK the current patch stores the info in a
>>hash table in a backend private memory, and I don't see how else to do
>>that (e.g. storing it in a catalog would cause catalog bloat).
>>
>>FWIW this is a reasons why I think just using shared buffers (instead
>>of local ones) is not sufficient to support parallel queriesl as
>>proposed by Alexander. The workers would not know the stats, breaking
>>planning of queries in PARALLEL SAFE plpgsql functions etc.
>
>
>I do not think that "all or nothing" approach is so good for software
>development as for database transactions.

Well, sure. I'm not saying we need to have a perfect solution in v1. I'm
saying if we have two choices:

(1) Use shared buffers even if it means the parallel query plan may be
     arbitrarily bad.

(2) Use private buffers, even if it means no parallel queries with temp
     tables.

Then I'm voting for (2) because it's less likely to break down. I can
imagine allowing parallel queries with GTT when there's no risk of
having to plan in the worker, but that's not there yet.

If we can come up with a reasonable solution for the parallel case, we
can enable it later.

>Yes, if we have function in PL/pgSQL which performs queries om 
>temporary tables, then
>parallel workers may build inefficient plan for this queries due to 
>lack of statistics.

IMHO that's a pretty awful deficiency, because it essentially means
users may need to disable parallelism for such queries. Which means
we'll get complaints from users, and we'll have to come up with some
sort of solution. I'd rather not be in that position.

>From my point of view this is not a pitfall of GTT but result of lack 
>of global plan cache in Postgres. And it should be fixed not at GTT 
>level.
>

That doesn't give us free pass to just ignore the issue. Even if it
really was due to a lack of global plan cache, the fact is we don't have
that feature, so we have a problem. I mean, if you need infrastructure
that is not available, you either have to implement that infrastructure
or make it work properly without it.

>Also I never see real use cases with such functions, even in the 
>systems which using hard temporary tables and stored procedures.
>But there are many other real problems with temp tables  (except 
>already mentioned in this thread).

Oh, I'm sure there are pretty large plpgsql applications, and I'd be
surprised if at least some of those were not affected. And I'm sure
there are apps using UDF to do all sorts of stuff (e.g. I wonder if
PostGIS would have this issue - IIRC it's using SPI etc.).

The question is whether we should consider existing apps affected,
because they are using the regular temporary tables and not GTT. So
unless they switch to GTT there is no regression ...

But even in that case I don't think it's a good idea to accept this as
an acceptable limitation. I admit one of the reasons why I think that
may be that statistics and planning are my areas of interest, so I'm not
quite willing to accept incomplete stuff as OK.

>In PgPro/EE we have fixes for some of them, for example:
>
>1. Do not reserve space in the file for temp relations. Right now 
>append of relation cause writing zero page to the disk by mdextend.
>It cause useless disk IO for temp tables which in most cases fit in 
>memory and should not be written at disk.
>
>2. Implicitly perform analyze of temp table intermediately after 
>storing data in it. Usually tables are analyzed by autovacuum in 
>background.
>But it doesn't work for temp tables which are not processes by 
>autovacuum and are accessed immediately after filling them with data 
>and lack of statistic  may cause
>building very inefficient plan. We have online_analyze extension which 
>force analyze of the table after appending some bulk of data to it.
>It can be used for normal table but most of all it is useful for temp 
>relations.
>
>Unlike hypothetical example with parallel safe function working with 
>temp tables,
>this are real problems observed by some of our customers.
>Them are applicable both to local and global temp tables and this is 
>why I do not want to discuss them in context of GTT.
>

I think those are both interesting issues worth fixing, but I don't
think it makes the issue discussed here less important.

>
>>
>>>3 Still no one commented on GTT's transaction information 
>>>processing, they include
>>>3.1 Should gtt's frozenxid need to be care?
>>>3.2 gtt’s clog clean
>>>3.3 How to deal with "too old" gtt data
>>>
>>
>>No idea what to do about this.
>>
>
>I wonder what is the specific of GTT here?
>The same problem takes place for normal (local) temp tables, doesn't it?
>

Not sure. TBH I'm not sure I understand what the issue actually is.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Jan 09, 2020 at 02:17:08PM +0300, Konstantin Knizhnik wrote:
>
>
>On 06.01.2020 8:04, 曾文旌(义从) wrote:
>>In the previous communication
>>
>>1 we agreed on the general direction
>>1.1 gtt use local (private) buffer
>>1.2 no replica access in first version
>>
>>2 We feel that gtt needs to maintain statistics, but there is no agreement on what it will be done.
>>
>>3 Still no one commented on GTT's transaction information processing, they include
>>3.1 Should gtt's frozenxid need to be care?
>>3.2 gtt’s clog clean
>>3.3 How to deal with "too old" gtt data
>>
>>I suggest we discuss further, reach an agreement, and merge the two patches to one.
>>
>
>I also hope that we should come to the common solution for GTT.
>If we do not try to address parallel execution issues and access to 
>temp tables at replicas (and I agreed
>that it should be avoided in first version of the patch), then GTT 
>patch becomes quite small.
>

Well, that was kinda my goal - making the patch as small as possible by
eliminating bits that are contentious or where we don't know the
solution (like planning for parallel queries).

>The most complex and challenged task is to support GTT for all kind of 
>indexes. Unfortunately I can not proposed some good universal solution 
>for it.
>Just patching all existed indexes implementation seems to be the only 
>choice.
>

I haven't looked at the indexing issue closely, but IMO we need to
ensure that every session sees/uses only indexes on GTT that were
defined before the seesion started using the table.

Can't we track which indexes a particular session sees, somehow?

>Statistic is another important case.
>But once again I do not completely understand why we want to address 
>all this issues with statistic in first version of the patch?

I think the question is which "issues with statistic" you mean. I'm sure
we can ignore some of them, e.g. the one with parallel workers not
having any stats (assuming we consider functions using GTT to be
parallel restricted).

>It contradicts to the idea to make this patch as small as possible.

Well, there's "making patch as small as possible" vs. "patch behaving
correctly" trade-off ;-)

>Also it seems to me that everybody agreed that users very rarely 
>create indexes for temp tables and explicitly analyze them.

I certainly *disagree* with this.

We often see temporary tables as a fix or misestimates in complex
queries, and/or as a replacement for CTEs with statistics/indexes. In
fact it's a pretty valuable tool when helping customers with complex
queries affected by poor estimates.

>So I think GTT will be useful even with limited support of statistic. 
>In my version statistics for GTT is provided by pushing correspondent 
>information to backend's cache for pg_statistic table.

I think someone pointed out pushing stuff directly into the cache is
rather problematic, but I don't recall the details.

>Also I provided pg_temp_statistic view for inspecting it by users. The 
>idea to make pg_statistic a view which combines statistic of normal 
>and temporary tables is overkill from my point of view.
>
>I do not understand why do we need to maintain hash with some extra 
>information for GTT in backends memory (as it was done in Wenjing 
>patch).
>Also idea to use create extension for accessing this information seems 
>to be dubious.
>

I think the extension was more a PoC rather than a final solution.


regards
-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 09.01.2020 19:48, Tomas Vondra wrote:
>
>> The most complex and challenged task is to support GTT for all kind 
>> of indexes. Unfortunately I can not proposed some good universal 
>> solution for it.
>> Just patching all existed indexes implementation seems to be the only 
>> choice.
>>
>
> I haven't looked at the indexing issue closely, but IMO we need to
> ensure that every session sees/uses only indexes on GTT that were
> defined before the seesion started using the table.

Why? It contradicts with behavior of normal tables.
Assume that you have active clients and at some point of time DBA 
recognizes that them are spending to much time in scanning some GTT.
It cab create index for this GTT but if existed client will not be able 
to use this index, then we need somehow make this clients to restart 
their sessions?
In my patch I have implemented building indexes for GTT on demand: if 
accessed index on GTT is not yet initialized, then it is filled with 
local data.
>
> Can't we track which indexes a particular session sees, somehow?
>
>> Statistic is another important case.
>> But once again I do not completely understand why we want to address 
>> all this issues with statistic in first version of the patch?
>
> I think the question is which "issues with statistic" you mean. I'm sure
> we can ignore some of them, e.g. the one with parallel workers not
> having any stats (assuming we consider functions using GTT to be
> parallel restricted).

If we do not use shared buffers for GTT then parallel processing of GTT 
is not possible at all, so there is no problem with statistic for 
parallel workers.

>
> I think someone pointed out pushing stuff directly into the cache is
> rather problematic, but I don't recall the details.
>
I have not encountered any problems, so if you can point me on what is 
wrong with this approach, I will think about alternative solution.


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

Then in other session run 2^31 transactions (at my desktop it takes about 2 hours).
As far as temp tables are not proceeded by vacuum, database is stalled:

 ERROR:  database is not accepting commands to avoid wraparound data loss in database "postgres"

It seems to be quite dubious behavior and it is strange to me that nobody complains about it.
We discuss  many issues related with temp tables (statistic, parallel queries,...) which seems to be less critical.

But this problem is not specific to GTT - it can be reproduced with normal (local) temp tables.
This is why I wonder why do we need to solve it in GTT patch.

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

Hi all

On Fri, Jan 10, 2020 at 03:24:34PM +0300, Konstantin Knizhnik wrote:
>
>
>On 09.01.2020 19:30, Tomas Vondra wrote:
>
>
>>
>>>
>>>>
>>>>>3 Still no one commented on GTT's transaction information 
>>>>>processing, they include
>>>>>3.1 Should gtt's frozenxid need to be care?
>>>>>3.2 gtt’s clog clean
>>>>>3.3 How to deal with "too old" gtt data
>>>>>
>>>>
>>>>No idea what to do about this.
>>>>
>>>
>>>I wonder what is the specific of GTT here?
>>>The same problem takes place for normal (local) temp tables, doesn't it?
>>>
>>
>>Not sure. TBH I'm not sure I understand what the issue actually is.
>
>Just open session, create temporary table and insert some data in it.
>Then in other session run 2^31 transactions (at my desktop it takes 
>about 2 hours).
>As far as temp tables are not proceeded by vacuum, database is stalled:
>
> ERROR:  database is not accepting commands to avoid wraparound data 
>loss in database "postgres"
>
>It seems to be quite dubious behavior and it is strange to me that 
>nobody complains about it.
>We discuss  many issues related with temp tables (statistic, parallel 
>queries,...) which seems to be less critical.
>
>But this problem is not specific to GTT - it can be reproduced with 
>normal (local) temp tables.
>This is why I wonder why do we need to solve it in GTT patch.
>

Yeah, I think that's out of scope for GTT patch. Once we solve it for
plain temporary tables, we'll solve it for GTT too.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

On Fri, Jan 10, 2020 at 11:47:42AM +0300, Konstantin Knizhnik wrote:
>
>
>On 09.01.2020 19:48, Tomas Vondra wrote:
>>
>>>The most complex and challenged task is to support GTT for all 
>>>kind of indexes. Unfortunately I can not proposed some good 
>>>universal solution for it.
>>>Just patching all existed indexes implementation seems to be the 
>>>only choice.
>>>
>>
>>I haven't looked at the indexing issue closely, but IMO we need to
>>ensure that every session sees/uses only indexes on GTT that were
>>defined before the seesion started using the table.
>
>Why? It contradicts with behavior of normal tables.
>Assume that you have active clients and at some point of time DBA 
>recognizes that them are spending to much time in scanning some GTT.
>It cab create index for this GTT but if existed client will not be 
>able to use this index, then we need somehow make this clients to 
>restart their sessions?
>In my patch I have implemented building indexes for GTT on demand: if 
>accessed index on GTT is not yet initialized, then it is filled with 
>local data.

Yes, I know the behavior would be different from behavior for regular
tables. And yes, it would not allow fixing slow queries in sessions
without interrupting those sessions.

I proposed just ignoring those new indexes because it seems much simpler
than alternative solutions that I can think of, and it's not like those
other solutions don't have other issues.

For example, I've looked at the "on demand" building as implemented in
global_private_temp-8.patch, I kinda doubt adding a bunch of index build
calls into various places in index code seems somewht suspicious.

* brinbuild is added to brinRevmapInitialize, which is meant to
   initialize state for scanning. It seems wrong to build the index we're
   scanning from this function (layering and all that).

* btbuild is called from _bt_getbuf. That seems a bit ... suspicious?

... and so on for other index types. Also, what about custom indexes
implemented in extensions? It seems a bit strange each of them has to
support this separately.

IMHO if this really is the right solution, we need to make it work for
existing indexes without having to tweak them individually. Why don't we
track a flag whether an index on GTT was initialized in a given session,
and if it was not then call the build function before calling any other
function from the index AM? 

But let's talk about other issues caused by "on demand" build. Imagine
you have 50 sessions, each using the same GTT with a GB of per-session
data. Now you create a new index on the GTT, which forces the sessions
to build it's "local" index. Those builds will use maintenance_work_mem
each, so 50 * m_w_m. I doubt that's expected/sensible.

So I suggest we start by just ignoring the *new* indexes, and improve
this in the future (by building the indexes on demand or whatever).

>>
>>Can't we track which indexes a particular session sees, somehow?
>>
>>>Statistic is another important case.
>>>But once again I do not completely understand why we want to 
>>>address all this issues with statistic in first version of the 
>>>patch?
>>
>>I think the question is which "issues with statistic" you mean. I'm sure
>>we can ignore some of them, e.g. the one with parallel workers not
>>having any stats (assuming we consider functions using GTT to be
>>parallel restricted).
>
>If we do not use shared buffers for GTT then parallel processing of 
>GTT is not possible at all, so there is no problem with statistic for 
>parallel workers.
>

Right.

>>
>>I think someone pointed out pushing stuff directly into the cache is
>>rather problematic, but I don't recall the details.
>>
>I have not encountered any problems, so if you can point me on what is 
>wrong with this approach, I will think about alternative solution.
>

I meant this comment by Robert:

https://www.postgresql.org/message-id/CA%2BTgmoZFWaND4PpT_CJbeu6VZGZKi2rrTuSTL-Ykd97fexTN-w%40mail.gmail.com


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

On 12.01.2020 4:51, Tomas Vondra wrote:
> On Fri, Jan 10, 2020 at 11:47:42AM +0300, Konstantin Knizhnik wrote:
>>
>>
>> On 09.01.2020 19:48, Tomas Vondra wrote:
>>>
>>>> The most complex and challenged task is to support GTT for all kind 
>>>> of indexes. Unfortunately I can not proposed some good universal 
>>>> solution for it.
>>>> Just patching all existed indexes implementation seems to be the 
>>>> only choice.
>>>>
>>>
>>> I haven't looked at the indexing issue closely, but IMO we need to
>>> ensure that every session sees/uses only indexes on GTT that were
>>> defined before the seesion started using the table.
>>
>> Why? It contradicts with behavior of normal tables.
>> Assume that you have active clients and at some point of time DBA 
>> recognizes that them are spending to much time in scanning some GTT.
>> It cab create index for this GTT but if existed client will not be 
>> able to use this index, then we need somehow make this clients to 
>> restart their sessions?
>> In my patch I have implemented building indexes for GTT on demand: if 
>> accessed index on GTT is not yet initialized, then it is filled with 
>> local data.
>
> Yes, I know the behavior would be different from behavior for regular
> tables. And yes, it would not allow fixing slow queries in sessions
> without interrupting those sessions.
>
> I proposed just ignoring those new indexes because it seems much simpler
> than alternative solutions that I can think of, and it's not like those
> other solutions don't have other issues.

Quit opposite: prohibiting sessions to see indexes created before 
session start to use GTT requires more efforts. We need to somehow 
maintain and check GTT first access time.

>
> For example, I've looked at the "on demand" building as implemented in
> global_private_temp-8.patch, I kinda doubt adding a bunch of index build
> calls into various places in index code seems somewht suspicious.

We in any case has to initialize GTT indexes on demand even if we 
prohibit usages of indexes created after first access by session to GTT.
So the difference is only in one thing: should we just initialize empty 
index or populate it with local data (if rules for index usability are 
the same for GTT as for normal tables).
 From implementation point of view there is no big difference. Actually 
building index in standard way is even simpler than constructing empty 
index. Originally I have implemented
first approach (I just forgot to consider case when GTT was already user 
by a session). Then I rewrited it using second approach and patch even 
became simpler.

>
> * brinbuild is added to brinRevmapInitialize, which is meant to
>   initialize state for scanning. It seems wrong to build the index we're
>   scanning from this function (layering and all that).
>
> * btbuild is called from _bt_getbuf. That seems a bit ... suspicious?


As I already mentioned - support of indexes for GTT is one of the most 
challenged things in my patch.
I didn't find good and universal solution. So I agreed that call of 
btbuild from _bt_getbuf may be considered as suspicious.
I will be pleased if you or sombody else can propose better elternative 
and not only for B-Tree, but for all other indexes.

But as I already wrote above, prohibiting session to used indexes 
created after first access to GTT doesn't solve the problem.
For normal tables (and for local temp tables) indexes are initialized at 
the time of their creation.
With GTT it doesn't work, because each session has its own local data of 
GTT.
We should either initialize/build index on demand (when it is first 
accessed), either at the moment of session start initialize indexes for 
all existed GTTs.
Last options seem to be much worser from my point of view: there may me 
huge number of GTT and session may not need to access GTT at all.
>
> ... and so on for other index types. Also, what about custom indexes
> implemented in extensions? It seems a bit strange each of them has to
> support this separately.

I have already complained about it: my patch supports GTT for all 
built-in indexes, but custom indexes has to handle it themselves.
Looks like to provide some generic solution we need to extend index API, 
providing two diffrent operations: creation and initialization.
But extending index API is very critical change... And also it doesn't 
solve the problem with all existed extensions: them in any case have
to be rewritten to implement new API version in order to support GTT.
>
> IMHO if this really is the right solution, we need to make it work for
> existing indexes without having to tweak them individually. Why don't we
> track a flag whether an index on GTT was initialized in a given session,
> and if it was not then call the build function before calling any other
> function from the index AM?
> But let's talk about other issues caused by "on demand" build. Imagine
> you have 50 sessions, each using the same GTT with a GB of per-session
> data. Now you create a new index on the GTT, which forces the sessions
> to build it's "local" index. Those builds will use maintenance_work_mem
> each, so 50 * m_w_m. I doubt that's expected/sensible.

I do not see principle difference here with scenario when 50 sessions 
create (local) temp table,
populate it with GB of data and create index for it.

>
> So I suggest we start by just ignoring the *new* indexes, and improve
> this in the future (by building the indexes on demand or whatever).

Sorry, but still do not agree with this suggestions:
- it doesn't simplify things
- it makes behavior of GTT incompatible with normal tables.
- it doesn't prevent some bad or unexpected behavior which can't be 
currently reproduced with normal (local) temp tables.

>
>>>
>>> I think someone pointed out pushing stuff directly into the cache is
>>> rather problematic, but I don't recall the details.
>>>
>> I have not encountered any problems, so if you can point me on what 
>> is wrong with this approach, I will think about alternative solution.
>>
>
> I meant this comment by Robert:
>
> https://www.postgresql.org/message-id/CA%2BTgmoZFWaND4PpT_CJbeu6VZGZKi2rrTuSTL-Ykd97fexTN-w%40mail.gmail.com 
>
>
"if any code tried to access the statistics directly from the table, 
rather than via the caches".

Currently optimizer is accessing statistic though caches. So this 
approach works. If somebody will rewrite optimizer or provide own custom 
optimizer in extension which access statistic directly
then it we really be a problem. But I wonder why bypassing catalog cache 
may be needed.

Moreover, if we implement alternative solution - for example make 
pg_statistic a view which combines results for normal tables and GTT, 
then existed optimizer has to be rewritten
because it can not access statistic in the way it is doing now. And 
there will be all problem with all existed extensions which are 
accessing statistic in most natural way - through system cache.



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

On Mon, Jan 13, 2020 at 11:08:40AM +0300, Konstantin Knizhnik wrote:
>
>
>On 12.01.2020 4:51, Tomas Vondra wrote:
>>On Fri, Jan 10, 2020 at 11:47:42AM +0300, Konstantin Knizhnik wrote:
>>>
>>>
>>>On 09.01.2020 19:48, Tomas Vondra wrote:
>>>>
>>>>>The most complex and challenged task is to support GTT for all 
>>>>>kind of indexes. Unfortunately I can not proposed some good 
>>>>>universal solution for it.
>>>>>Just patching all existed indexes implementation seems to be 
>>>>>the only choice.
>>>>>
>>>>
>>>>I haven't looked at the indexing issue closely, but IMO we need to
>>>>ensure that every session sees/uses only indexes on GTT that were
>>>>defined before the seesion started using the table.
>>>
>>>Why? It contradicts with behavior of normal tables.
>>>Assume that you have active clients and at some point of time DBA 
>>>recognizes that them are spending to much time in scanning some 
>>>GTT.
>>>It cab create index for this GTT but if existed client will not be 
>>>able to use this index, then we need somehow make this clients to 
>>>restart their sessions?
>>>In my patch I have implemented building indexes for GTT on demand: 
>>>if accessed index on GTT is not yet initialized, then it is filled 
>>>with local data.
>>
>>Yes, I know the behavior would be different from behavior for regular
>>tables. And yes, it would not allow fixing slow queries in sessions
>>without interrupting those sessions.
>>
>>I proposed just ignoring those new indexes because it seems much simpler
>>than alternative solutions that I can think of, and it's not like those
>>other solutions don't have other issues.
>
>Quit opposite: prohibiting sessions to see indexes created before 
>session start to use GTT requires more efforts. We need to somehow 
>maintain and check GTT first access time.
>

Hmmm, OK. I'd expect such check to be much simpler than the on-demand
index building, but I admit I haven't tried implementing either of those
options.

>>
>>For example, I've looked at the "on demand" building as implemented in
>>global_private_temp-8.patch, I kinda doubt adding a bunch of index build
>>calls into various places in index code seems somewht suspicious.
>
>We in any case has to initialize GTT indexes on demand even if we 
>prohibit usages of indexes created after first access by session to 
>GTT.
>So the difference is only in one thing: should we just initialize 
>empty index or populate it with local data (if rules for index 
>usability are the same for GTT as for normal tables).
>From implementation point of view there is no big difference. Actually 
>building index in standard way is even simpler than constructing empty 
>index. Originally I have implemented
>first approach (I just forgot to consider case when GTT was already 
>user by a session). Then I rewrited it using second approach and patch 
>even became simpler.
>
>>
>>* brinbuild is added to brinRevmapInitialize, which is meant to
>>  initialize state for scanning. It seems wrong to build the index we're
>>  scanning from this function (layering and all that).
>>
>>* btbuild is called from _bt_getbuf. That seems a bit ... suspicious?
>
>
>As I already mentioned - support of indexes for GTT is one of the most 
>challenged things in my patch.
>I didn't find good and universal solution. So I agreed that call of 
>btbuild from _bt_getbuf may be considered as suspicious.
>I will be pleased if you or sombody else can propose better 
>elternative and not only for B-Tree, but for all other indexes.
>
>But as I already wrote above, prohibiting session to used indexes 
>created after first access to GTT doesn't solve the problem.
>For normal tables (and for local temp tables) indexes are initialized 
>at the time of their creation.
>With GTT it doesn't work, because each session has its own local data 
>of GTT.
>We should either initialize/build index on demand (when it is first 
>accessed), either at the moment of session start initialize indexes 
>for all existed GTTs.
>Last options seem to be much worser from my point of view: there may 
>me huge number of GTT and session may not need to access GTT at all.
>>
>>... and so on for other index types. Also, what about custom indexes
>>implemented in extensions? It seems a bit strange each of them has to
>>support this separately.
>
>I have already complained about it: my patch supports GTT for all 
>built-in indexes, but custom indexes has to handle it themselves.
>Looks like to provide some generic solution we need to extend index 
>API, providing two diffrent operations: creation and initialization.
>But extending index API is very critical change... And also it doesn't 
>solve the problem with all existed extensions: them in any case have
>to be rewritten to implement new API version in order to support GTT.
>

Why not to allow creating only indexes implementing this new API method
(on GTT)?

>>
>>IMHO if this really is the right solution, we need to make it work for
>>existing indexes without having to tweak them individually. Why don't we
>>track a flag whether an index on GTT was initialized in a given session,
>>and if it was not then call the build function before calling any other
>>function from the index AM?
>>But let's talk about other issues caused by "on demand" build. Imagine
>>you have 50 sessions, each using the same GTT with a GB of per-session
>>data. Now you create a new index on the GTT, which forces the sessions
>>to build it's "local" index. Those builds will use maintenance_work_mem
>>each, so 50 * m_w_m. I doubt that's expected/sensible.
>
>I do not see principle difference here with scenario when 50 sessions 
>create (local) temp table,
>populate it with GB of data and create index for it.
>

I'd say the high memory consumption is pretty significant.

>>
>>So I suggest we start by just ignoring the *new* indexes, and improve
>>this in the future (by building the indexes on demand or whatever).
>
>Sorry, but still do not agree with this suggestions:
>- it doesn't simplify things
>- it makes behavior of GTT incompatible with normal tables.
>- it doesn't prevent some bad or unexpected behavior which can't be 
>currently reproduced with normal (local) temp tables.
>
>>
>>>>
>>>>I think someone pointed out pushing stuff directly into the cache is
>>>>rather problematic, but I don't recall the details.
>>>>
>>>I have not encountered any problems, so if you can point me on 
>>>what is wrong with this approach, I will think about alternative 
>>>solution.
>>>
>>
>>I meant this comment by Robert:
>>
>>https://www.postgresql.org/message-id/CA%2BTgmoZFWaND4PpT_CJbeu6VZGZKi2rrTuSTL-Ykd97fexTN-w%40mail.gmail.com
>>
>>
>"if any code tried to access the statistics directly from the table, 
>rather than via the caches".
>
>Currently optimizer is accessing statistic though caches. So this 
>approach works. If somebody will rewrite optimizer or provide own 
>custom optimizer in extension which access statistic directly
>then it we really be a problem. But I wonder why bypassing catalog 
>cache may be needed.
>

I don't know, but it seems extensions like hypopg do it.

>Moreover, if we implement alternative solution - for example make 
>pg_statistic a view which combines results for normal tables and GTT, 
>then existed optimizer has to be rewritten
>because it can not access statistic in the way it is doing now. And 
>there will be all problem with all existed extensions which are 
>accessing statistic in most natural way - through system cache.
>

Perhaps. I don't know enough about this part of the code to have a
strong opinion.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

On Mon, Jan 13, 2020 at 05:32:53PM +0100, Tomas Vondra wrote:
> On Mon, Jan 13, 2020 at 11:08:40AM +0300, Konstantin Knizhnik wrote:
> >
> >"if any code tried to access the statistics directly from the table, 
> >rather than via the caches".
> >
> >Currently optimizer is accessing statistic though caches. So this 
> >approach works. If somebody will rewrite optimizer or provide own 
> >custom optimizer in extension which access statistic directly
> >then it we really be a problem. But I wonder why bypassing catalog 
> >cache may be needed.
> >
> 
> I don't know, but it seems extensions like hypopg do it.

AFAIR, hypopg only opens pg_statistic to use its tupledesc when creating
statistics on hypothetical partitions, but it should otherwise never reads or
need plain pg_statistic rows.

On Mon, Jan 13, 2020 at 09:12:38PM +0100, Julien Rouhaud wrote:
>On Mon, Jan 13, 2020 at 05:32:53PM +0100, Tomas Vondra wrote:
>> On Mon, Jan 13, 2020 at 11:08:40AM +0300, Konstantin Knizhnik wrote:
>> >
>> >"if any code tried to access the statistics directly from the table,
>> >rather than via the caches".
>> >
>> >Currently optimizer is accessing statistic though caches. So this
>> >approach works. If somebody will rewrite optimizer or provide own
>> >custom optimizer in extension which access statistic directly
>> >then it we really be a problem. But I wonder why bypassing catalog
>> >cache may be needed.
>> >
>>
>> I don't know, but it seems extensions like hypopg do it.
>
>AFAIR, hypopg only opens pg_statistic to use its tupledesc when creating
>statistics on hypothetical partitions, but it should otherwise never reads or
>need plain pg_statistic rows.

Ah, OK! Thanks for the clarification. I knew it does something with the
catalog, didn't realize it only gets the descriptor.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

Thank you for review my patch.

> 2020年1月12日 上午9:14，Tomas Vondra <tomas.vondra@2ndquadrant.com> 写道：
>
> On Fri, Jan 10, 2020 at 03:24:34PM +0300, Konstantin Knizhnik wrote:
>>
>>
>> On 09.01.2020 19:30, Tomas Vondra wrote:
>>
>>
>>>
>>>>
>>>>>
>>>>>> 3 Still no one commented on GTT's transaction information processing, they include
>>>>>> 3.1 Should gtt's frozenxid need to be care?
>>>>>> 3.2 gtt’s clog clean
>>>>>> 3.3 How to deal with "too old" gtt data
>>>>>>
>>>>>
>>>>> No idea what to do about this.
>>>>>
>>>>
>>>> I wonder what is the specific of GTT here?
>>>> The same problem takes place for normal (local) temp tables, doesn't it?
>>>>
>>>
>>> Not sure. TBH I'm not sure I understand what the issue actually is.
>>
>> Just open session, create temporary table and insert some data in it.
>> Then in other session run 2^31 transactions (at my desktop it takes about 2 hours).
>> As far as temp tables are not proceeded by vacuum, database is stalled:
>>
>>  ERROR:  database is not accepting commands to avoid wraparound data loss in database "postgres"
>>
>> It seems to be quite dubious behavior and it is strange to me that nobody complains about it.
>> We discuss  many issues related with temp tables (statistic, parallel queries,...) which seems to be less critical.
>>
>> But this problem is not specific to GTT - it can be reproduced with normal (local) temp tables.
>> This is why I wonder why do we need to solve it in GTT patch.
>>
>
> Yeah, I think that's out of scope for GTT patch. Once we solve it for
> plain temporary tables, we'll solve it for GTT too.
1. The core problem is that the data contains transaction information (xid), which needs to be vacuum(freeze) regularly
toavoid running out of xid. 
The autovacuum supports vacuum regular table but local temp does not. autovacuum also does not support GTT.

2. However, the difference between the local temp table and the global temp table(GTT) is that
a) For local temp table: one table hava one piece of data. the frozenxid of one local temp table is store in the
catalog(pg_class). 
b) For global temp table: each session has a separate copy of data, one GTT may contain maxbackend frozenxid.
and I don't think it's a good idea to keep frozenxid of GTT in the catalog(pg_class).
It becomes a question: how to handle GTT transaction information?

I agree that problem 1 should be completely solved by a some feature, such as local transactions. It is definitely not
includedin the GTT patch. 

But, I think we need to ensure the durability of GTT data. For example, data in GTT cannot be lost due to the clog
beingcleaned up. It belongs to problem 2. 



Wenjing


>
> regards
>
> --
> Tomas Vondra                  http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

> 2020年1月13日 下午4:08，Konstantin Knizhnik <k.knizhnik@postgrespro.ru> 写道：
>
>
>
> On 12.01.2020 4:51, Tomas Vondra wrote:
>> On Fri, Jan 10, 2020 at 11:47:42AM +0300, Konstantin Knizhnik wrote:
>>>
>>>
>>> On 09.01.2020 19:48, Tomas Vondra wrote:
>>>>
>>>>> The most complex and challenged task is to support GTT for all kind of indexes. Unfortunately I can not proposed
somegood universal solution for it. 
>>>>> Just patching all existed indexes implementation seems to be the only choice.
>>>>>
>>>>
>>>> I haven't looked at the indexing issue closely, but IMO we need to
>>>> ensure that every session sees/uses only indexes on GTT that were
>>>> defined before the seesion started using the table.
>>>
>>> Why? It contradicts with behavior of normal tables.
>>> Assume that you have active clients and at some point of time DBA recognizes that them are spending to much time in
scanningsome GTT. 
>>> It cab create index for this GTT but if existed client will not be able to use this index, then we need somehow
makethis clients to restart their sessions? 
>>> In my patch I have implemented building indexes for GTT on demand: if accessed index on GTT is not yet initialized,
thenit is filled with local data. 
>>
>> Yes, I know the behavior would be different from behavior for regular
>> tables. And yes, it would not allow fixing slow queries in sessions
>> without interrupting those sessions.
>>
>> I proposed just ignoring those new indexes because it seems much simpler
>> than alternative solutions that I can think of, and it's not like those
>> other solutions don't have other issues.
>
> Quit opposite: prohibiting sessions to see indexes created before session start to use GTT requires more efforts. We
needto somehow maintain and check GTT first access time. 
>
>>
>> For example, I've looked at the "on demand" building as implemented in
>> global_private_temp-8.patch, I kinda doubt adding a bunch of index build
>> calls into various places in index code seems somewht suspicious.
>
> We in any case has to initialize GTT indexes on demand even if we prohibit usages of indexes created after first
accessby session to GTT. 
> So the difference is only in one thing: should we just initialize empty index or populate it with local data (if
rulesfor index usability are the same for GTT as for normal tables). 
> From implementation point of view there is no big difference. Actually building index in standard way is even simpler
thanconstructing empty index. Originally I have implemented 
> first approach (I just forgot to consider case when GTT was already user by a session). Then I rewrited it using
secondapproach and patch even became simpler. 
>
>>
>> * brinbuild is added to brinRevmapInitialize, which is meant to
>>   initialize state for scanning. It seems wrong to build the index we're
>>   scanning from this function (layering and all that).
>>
>> * btbuild is called from _bt_getbuf. That seems a bit ... suspicious?
>
>
> As I already mentioned - support of indexes for GTT is one of the most challenged things in my patch.
> I didn't find good and universal solution. So I agreed that call of btbuild from _bt_getbuf may be considered as
suspicious.
> I will be pleased if you or sombody else can propose better elternative and not only for B-Tree, but for all other
indexes.
>
> But as I already wrote above, prohibiting session to used indexes created after first access to GTT doesn't solve the
problem.
> For normal tables (and for local temp tables) indexes are initialized at the time of their creation.
> With GTT it doesn't work, because each session has its own local data of GTT.
> We should either initialize/build index on demand (when it is first accessed), either at the moment of session start
initializeindexes for all existed GTTs. 
> Last options seem to be much worser from my point of view: there may me huge number of GTT and session may not need
toaccess GTT at all. 
>>
>> ... and so on for other index types. Also, what about custom indexes
>> implemented in extensions? It seems a bit strange each of them has to
>> support this separately.
>
> I have already complained about it: my patch supports GTT for all built-in indexes, but custom indexes has to handle
itthemselves. 
> Looks like to provide some generic solution we need to extend index API, providing two diffrent operations: creation
andinitialization. 
> But extending index API is very critical change... And also it doesn't solve the problem with all existed extensions:
themin any case have 
> to be rewritten to implement new API version in order to support GTT.
>>
>> IMHO if this really is the right solution, we need to make it work for
>> existing indexes without having to tweak them individually. Why don't we
>> track a flag whether an index on GTT was initialized in a given session,
>> and if it was not then call the build function before calling any other
>> function from the index AM?
>> But let's talk about other issues caused by "on demand" build. Imagine
>> you have 50 sessions, each using the same GTT with a GB of per-session
>> data. Now you create a new index on the GTT, which forces the sessions
>> to build it's "local" index. Those builds will use maintenance_work_mem
>> each, so 50 * m_w_m. I doubt that's expected/sensible.
>
> I do not see principle difference here with scenario when 50 sessions create (local) temp table,
> populate it with GB of data and create index for it.
I think the problem is that when one session completes the creation of the index on GTT,
it will trigger the other sessions build own local index of GTT in a centralized time.
This will consume a lot of hardware resources (cpu io memory) in a short time,
and even the database service becomes slow, because 50 sessions are building index.
I think this is not what we expected.

>
>>
>> So I suggest we start by just ignoring the *new* indexes, and improve
>> this in the future (by building the indexes on demand or whatever).
>
> Sorry, but still do not agree with this suggestions:
> - it doesn't simplify things
> - it makes behavior of GTT incompatible with normal tables.
> - it doesn't prevent some bad or unexpected behavior which can't be currently reproduced with normal (local) temp
tables.
From a user perspective, this proposal is reasonable.
From an implementation perspective, the same GTT index needs to maintain different states (valid or invalid) in
differentsessions,  
which seems difficult to do in the current framework.

So in my first version, I chose to complete all index creation before using GTT.
I think this will satisfy most use cases.

>
>>
>>>>
>>>> I think someone pointed out pushing stuff directly into the cache is
>>>> rather problematic, but I don't recall the details.
>>>>
>>> I have not encountered any problems, so if you can point me on what is wrong with this approach, I will think about
alternativesolution. 
>>>
>>
>> I meant this comment by Robert:
>>
>> https://www.postgresql.org/message-id/CA%2BTgmoZFWaND4PpT_CJbeu6VZGZKi2rrTuSTL-Ykd97fexTN-w%40mail.gmail.com
>>
> "if any code tried to access the statistics directly from the table, rather than via the caches".
>
> Currently optimizer is accessing statistic though caches. So this approach works. If somebody will rewrite optimizer
orprovide own custom optimizer in extension which access statistic directly 
> then it we really be a problem. But I wonder why bypassing catalog cache may be needed.
>
> Moreover, if we implement alternative solution - for example make pg_statistic a view which combines results for
normaltables and GTT, then existed optimizer has to be rewritten 
> because it can not access statistic in the way it is doing now. And there will be all problem with all existed
extensionswhich are accessing statistic in most natural way - through system cache. 
>
>
>
> --
> Konstantin Knizhnik
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company

On 15.01.2020 16:10, 曾文旌(义从) wrote:
>
>> I do not see principle difference here with scenario when 50 sessions create (local) temp table,
>> populate it with GB of data and create index for it.
> I think the problem is that when one session completes the creation of the index on GTT,
> it will trigger the other sessions build own local index of GTT in a centralized time.
> This will consume a lot of hardware resources (cpu io memory) in a short time,
> and even the database service becomes slow, because 50 sessions are building index.
> I think this is not what we expected.


First of all creating index for GTT ni one session doesn't immediately 
initiate building indexes in all other sessions.
Indexes are built on demand. If session is not using this GTT any more, 
then index for it will not build at all.
And if GTT is really are actively used by all sessions, then building 
index and using it for constructing optimal execution plan is better,
then continue to  use sequential scan and read all GTT data from the disk.

And as I already mentioned I do not see some principle difference in 
aspect of resource consumptions comparing with current usage of local 
temp tables.
If we have have many sessions, each creating temp table, populating it 
with data and building index for it, then we will
observe the same CPU utilization and memory resource consumption as in 
case of using GTT and creating index for it.

Sorry, but I still not convinced by your and Tomas arguments.
Yes, building GTT index may cause high memory consumption 
(maintenance_work_mem * n_backends).
But such consumption can be  observed also without GTT and it has to be 
taken in account when choosing value for maintenance_work_mem.
But from my point of view it is much more important to make behavior of 
GTT as much compatible with normal tables as possible.
Also from database administration point of view, necessity to restart 
sessions to make then use new indexes seems to be very strange and 
inconvenient.
Alternatively DBA can address the problem with high memory consumption 
by adjusting maintenance_work_mem, so this solution is more flexible.



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

On 2020-01-19 18:04, 曾文旌(义从) wrote:
>> 2020年1月14日 下午9:20，Pavel Stehule <pavel.stehule@gmail.com> 写道：
>> út 14. 1. 2020 v 14:09 odesílatel 曾文旌(义从) <wenjing.zwj@alibaba-inc.com 
>> <mailto:wenjing.zwj@alibaba-inc.com>> napsal:

>> [global_temporary_table_v4-pg13.patch ]

Hi,

This patch doesn't quiet apply for me:

patching file src/backend/access/common/reloptions.c
patching file src/backend/access/gist/gistutil.c
patching file src/backend/access/hash/hash.c
Hunk #1 succeeded at 149 (offset 3 lines).
patching file src/backend/access/heap/heapam_handler.c
patching file src/backend/access/heap/vacuumlazy.c
patching file src/backend/access/nbtree/nbtpage.c
patching file src/backend/access/table/tableam.c
patching file src/backend/access/transam/xlog.c
patching file src/backend/catalog/Makefile
Hunk #1 FAILED at 44.
1 out of 1 hunk FAILED -- saving rejects to file 
src/backend/catalog/Makefile.rej
[...]
    (The rest applies without errors)

src/backend/catalog/Makefile.rej contains:

------------------------
--- src/backend/catalog/Makefile
+++ src/backend/catalog/Makefile
@@ -44,6 +44,8 @@ OBJS = \
      storage.o \
      toasting.o

+OBJS += storage_gtt.o
+
  BKIFILES = postgres.bki postgres.description postgres.shdescription

  include $(top_srcdir)/src/backend/common.mk
------------------------

Can you have a look?


thanks,

Erik Rijkers

> 2020年1月20日 上午1:32，Erik Rijkers <er@xs4all.nl> 写道：
>
> On 2020-01-19 18:04, 曾文旌(义从) wrote:
>>> 2020年1月14日 下午9:20，Pavel Stehule <pavel.stehule@gmail.com> 写道：
>>> út 14. 1. 2020 v 14:09 odesílatel 曾文旌(义从) <wenjing.zwj@alibaba-inc.com <mailto:wenjing.zwj@alibaba-inc.com>>
napsal:
>
>>> [global_temporary_table_v4-pg13.patch ]
>
> Hi,
>
> This patch doesn't quiet apply for me:
>
> patching file src/backend/access/common/reloptions.c
> patching file src/backend/access/gist/gistutil.c
> patching file src/backend/access/hash/hash.c
> Hunk #1 succeeded at 149 (offset 3 lines).
> patching file src/backend/access/heap/heapam_handler.c
> patching file src/backend/access/heap/vacuumlazy.c
> patching file src/backend/access/nbtree/nbtpage.c
> patching file src/backend/access/table/tableam.c
> patching file src/backend/access/transam/xlog.c
> patching file src/backend/catalog/Makefile
> Hunk #1 FAILED at 44.
> 1 out of 1 hunk FAILED -- saving rejects to file src/backend/catalog/Makefile.rej
> [...]
>   (The rest applies without errors)
>
> src/backend/catalog/Makefile.rej contains:
>
> ------------------------
> --- src/backend/catalog/Makefile
> +++ src/backend/catalog/Makefile
> @@ -44,6 +44,8 @@ OBJS = \
>     storage.o \
>     toasting.o
>
> +OBJS += storage_gtt.o
> +
> BKIFILES = postgres.bki postgres.description postgres.shdescription
>
> include $(top_srcdir)/src/backend/common.mk
> ------------------------
>
> Can you have a look?
I updated the code and remade the patch.
Please give me feedback if you have any more questions.




> 
> 
> thanks,
> 
> Erik Rijkers
> 
> 
> 
> 
> 

On Sat, Jan 11, 2020 at 8:51 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> I proposed just ignoring those new indexes because it seems much simpler
> than alternative solutions that I can think of, and it's not like those
> other solutions don't have other issues.

+1.

> For example, I've looked at the "on demand" building as implemented in
> global_private_temp-8.patch, I kinda doubt adding a bunch of index build
> calls into various places in index code seems somewht suspicious.

+1. I can't imagine that's a safe or sane thing to do.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

I'm trying to improve this part of the implementation in global_temporary_table_v7-pg13.patch

Please check my patch and give me feedback.

Thanks

Wenjing

Below is my short review of the patch:

+    /*
+     * For global temp table only
+     * use AccessExclusiveLock for ensure safety
+     */
+    {
+        {
+            "on_commit_delete_rows",
+            "global temp table on commit options",
+            RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
+            ShareUpdateExclusiveLock
+        },
+        true
+    },   


The comment seems to be confusing: it says about AccessExclusiveLock but actually uses ShareUpdateExclusiveLock.

-    Assert(TransactionIdIsNormal(onerel->rd_rel->relfrozenxid));
-    Assert(MultiXactIdIsValid(onerel->rd_rel->relminmxid));
+    Assert((RELATION_IS_GLOBAL_TEMP(onerel) && onerel->rd_rel->relfrozenxid == InvalidTransactionId) ||
+        (!RELATION_IS_GLOBAL_TEMP(onerel) && TransactionIdIsNormal(onerel->rd_rel->relfrozenxid)));
+    Assert((RELATION_IS_GLOBAL_TEMP(onerel) && onerel->rd_rel->relminmxid == InvalidMultiXactId) ||
+        (!RELATION_IS_GLOBAL_TEMP(onerel) && MultiXactIdIsValid(onerel->rd_rel->relminmxid)));
 
It is actually equivalent to:

Assert(RELATION_IS_GLOBAL_TEMP(onerel) ^ TransactionIdIsNormal(onerel->rd_rel->relfrozenxid);
Assert(RELATION_IS_GLOBAL_TEMP(onerel) ^ MultiXactIdIsValid(onerel->rd_rel->relminmxid));

+    /* clean temp relation files */
+    if (max_active_gtt > 0)
+        RemovePgTempFiles();
+
     /*
 
I wonder why do we need some special check for GTT here.
From my point of view cleanup at startup of local storage of temp tables should be performed in the same way for local and global temp tables.


-    new_rel_reltup->relfrozenxid = relfrozenxid;
-    new_rel_reltup->relminmxid = relminmxid;
+    /* global temp table not remember transaction info in catalog */
+    if (relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
+    {
+        new_rel_reltup->relfrozenxid = InvalidTransactionId;
+        new_rel_reltup->relminmxid = InvalidMultiXactId;
+    }
+    else
+    {
+        new_rel_reltup->relfrozenxid = relfrozenxid;
+        new_rel_reltup->relminmxid = relminmxid;
+    }
+


Why do we need to do it for GTT?
Did you check that there will be no problems with GTT in case of XID wraparound?
Right now if you create temp table and keep session open, then it will block XID wraparound.

+    /* We allow to drop global temp table only this session use it */
+    if (RELATION_IS_GLOBAL_TEMP(rel))
+    {
+        if (is_other_backend_use_gtt(rel->rd_node))
+            elog(ERROR, "can not drop relation when other backend attached this global temp table");
+    }
+

Here we once again introduce incompatibility with normal (permanent) tables.
Assume that DBA or programmer need to change format of GTT. But there are some active sessions which have used this GTT sometime in the past.
We will not be able to drop this GTT until all this sessions are terminated.
I do not think that it is acceptable behaviour.

+        LOCKMODE    lockmode = AccessExclusiveLock;
+
+        /* truncate global temp table only need RowExclusiveLock */
+        if (get_rel_persistence(rid) == RELPERSISTENCE_GLOBAL_TEMP)
+            lockmode = RowExclusiveLock;


What are the reasons of using RowExclusiveLock for GTT instead of AccessExclusiveLock?
Yes, GTT data is access only by one backend so no locking here seems to be needed at all.
But I wonder what are the motivations/benefits of using weaker lock level here?
There should be no conflicts in any case...

+        /* We allow to create index on global temp table only this session use it */
+        if (is_other_backend_use_gtt(heapRelation->rd_node))
+            elog(ERROR, "can not create index when have other backend attached this global temp table");
+

The same argument as in case of dropping GTT: I do not think that prohibiting DLL operations on GTT used by more than one backend is bad idea.

+    /* global temp table not support foreign key constraint yet */
+    if (RELATION_IS_GLOBAL_TEMP(pkrel))
+        ereport(ERROR,
+                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                 errmsg("referenced relation \"%s\" is not a global temp table",
+                        RelationGetRelationName(pkrel))));
+

Why do we need to prohibit foreign key constraint on GTT?

+    /*
+     * Global temp table get frozenxid from MyProc
+     * to avoid the vacuum truncate clog that gtt need.
+     */
+    if (max_active_gtt > 0)
+    {
+        TransactionId oldest_gtt_frozenxid =
+            list_all_session_gtt_frozenxids(0, NULL, NULL, NULL);
+
+        if (TransactionIdIsNormal(oldest_gtt_frozenxid) &&
+            TransactionIdPrecedes(oldest_gtt_frozenxid, newFrozenXid))
+        {
+            ereport(WARNING,
+                (errmsg("global temp table oldest FrozenXid is far in the past"),
+                 errhint("please truncate them or kill those sessions that use them.")));
+            newFrozenXid = oldest_gtt_frozenxid;
+        }
+    }
+

As far as I understand, content of GTT will never be processes by autovacuum.
So who will update frozenxid of GTT?
I see that up_gtt_relstats is invoked when:
- index is created on GTT
- GTT is truncated
- GTT is vacuumed
So unless GTT is explicitly vacuumed by user, its GTT is and them will not be taken in account
when computing new frozen xid value. Autovacumm will produce this warnings (which will ton be visible by end user and only appended to the log).
And at some moment of time wrap around happen and if there still some old active GTT, we will get incorrect results.



--

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

On 23.01.2020 23:47, Robert Haas wrote:
> On Sat, Jan 11, 2020 at 8:51 PM Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> I proposed just ignoring those new indexes because it seems much simpler
>> than alternative solutions that I can think of, and it's not like those
>> other solutions don't have other issues.
> +1.
>
>> For example, I've looked at the "on demand" building as implemented in
>> global_private_temp-8.patch, I kinda doubt adding a bunch of index build
>> calls into various places in index code seems somewht suspicious.
> +1. I can't imagine that's a safe or sane thing to do.
>

As far as you know there are two versions of GTT implementations now.
And we are going to merge them into single patch.
But there are some principle question concerning provided functionality 
which has to be be discussed:
should we prohibit DDL on GTT if there are more than one sessions using 
it. It includes creation/dropping indexes, dropping table, altering table...

If the answer is "yes", then the question whether to populate new 
indexes with data is no relevant at all, because such situation will not 
be possible.
But in this case we will get incompatible behavior with normal 
(permanent) tables and it seems to be very inconvenient from DBA point 
of view:
it will be necessary to enforce all clients to close their sessions to 
perform some DDL manipulations with GTT.
Some DDLs will be very difficult to implement if GTT is used by more 
than one backend, for example altering table schema.

My current solution is to allow creation/droping index on GTT and 
dropping table itself, while prohibit alter schema at all for GTT.
Wenjing's approach is to prohibit any DDL if GTT is used by more than 
one backend.

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

pá 24. 1. 2020 v 9:39 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:

On 23.01.2020 23:47, Robert Haas wrote:
> On Sat, Jan 11, 2020 at 8:51 PM Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> I proposed just ignoring those new indexes because it seems much simpler
>> than alternative solutions that I can think of, and it's not like those
>> other solutions don't have other issues.
> +1.
>
>> For example, I've looked at the "on demand" building as implemented in
>> global_private_temp-8.patch, I kinda doubt adding a bunch of index build
>> calls into various places in index code seems somewht suspicious.
> +1. I can't imagine that's a safe or sane thing to do.
>

As far as you know there are two versions of GTT implementations now.
And we are going to merge them into single patch.
But there are some principle question concerning provided functionality
which has to be be discussed:
should we prohibit DDL on GTT if there are more than one sessions using
it. It includes creation/dropping indexes, dropping table, altering table...

If the answer is "yes", then the question whether to populate new
indexes with data is no relevant at all, because such situation will not
be possible.
But in this case we will get incompatible behavior with normal
(permanent) tables and it seems to be very inconvenient from DBA point
of view:
it will be necessary to enforce all clients to close their sessions to
perform some DDL manipulations with GTT.
Some DDLs will be very difficult to implement if GTT is used by more
than one backend, for example altering table schema.

My current solution is to allow creation/droping index on GTT and
dropping table itself, while prohibit alter schema at all for GTT.
Wenjing's approach is to prohibit any DDL if GTT is used by more than
one backend.

When I create index on GTT in one session, then I don't expect creating same index in all other sessions that uses same GTT.

But I can imagine to creating index on GTT enforces index in current session, and for other sessions this index will be invalid to end of session.

So there are three possible alternatives:

1. Prohibit index creation of GTT when it used by more than once session.
2. Create index and populate them with data in all sessions using this GTT.
3. Create index only in current session and do not allow to use it in all other sessions already using this GTT (but definitely allow to use it in new sessions).

1 is Wenjing's approach, 2 - is my approach, 3 - is your suggestion :)

I can construct the following table with pro/cons of each approach:

Approach	Compatibility with normal table	User (DBA) friendly	Complexity of implementation	Consistency
1	-	1: requires restart of all sessions to perform operation	2: requires global cache of GTT	3: no man, no problem
2	+	3: if index is created then it is actually needed, isn't it?	1: use existed functionality to create index	2: if alter schema is prohibited
3	-	2: requires restart of all sessions to use created index	3: requires some mechanism for prohibiting index created after first session access to GTT	1: can perform DDL but do no see effect of it

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

You will see a effect of DDL in current session (where you did the change), all other sessions should to live without any any change do reconnect or to RESET connect

Why? I found this requirement quit unnatural and contradicting to the behavior of normal tables.
Actually one of motivation for adding global tempo tables to Postgres is to provide compatibility with Oracle.
Although I know that Oracle design decisions were never considered as  axioms by Postgres community,
but ni case of GTT design I think that we should take in account Oracle approach.
And GTT in Oracle behaves exactly as in my implementation:

https://www.oracletutorial.com/oracle-basics/oracle-global-temporary-table/

It is not clear from this documentation whether index created for GTT in one session can be used in another session which already has some data in this GTT.
But I did experiment with install Oracle server and  can confirm that actually works in this way.

So I do not understand why do we need to complicate our GTT implementation in order to prohibit useful functionality and introduce inconsistency between behavior of normal and global temp tables.

I don't like 2 - when I do index on global temp table, I don't would to wait on indexing on all other sessions. These operations should be maximally independent.

Nobody suggest to wait building index in all sessions.
Indexes will be constructed on demand when session access this table.
If session will no access this table at all, then index will never be constructed.

Once again: logic of dealing with indexes in GTT is very simple.
For normal tables, indexes are initialized at the tame when them are created.
For GTT it is not true. We have to initialize index on demand when it is accessed first time in session.

So it has to be handled in any way.
The question is only whether we should allow creation of index for table already populated with some data?
Actually doesn't require some additional efforts. We can use existed build_index function which initialize index and populates it with data.
So the solution proposed for me is most natural, convenient and simplest solution at the same time. And compatible with Oracle.

Regards

Pavel

 

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

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