Обсуждение: Re: [Proposal] Global temporary tables

Поиск
Список
Период
Сортировка

Re: [Proposal] Global temporary tables

От
"movead.li@highgo.ca"
Дата:

>Fixed in global_temporary_table_v29-pg13.patch
>Please check.

I find this is the most latest mail with an attachment, so I test and reply on
this thread, several points as below:

1. I notice it produces new relfilenode when new session login and some
data insert. But the relfilenode column in pg_class still the one when create
the global temp table. I think you can try to show 0 in this area as what nail
relation does. 

2. The nail relations handle their relfilenodes by RelMapFile struct, and this
patch use hash entry and relfilenode_list, maybe RelMapFile approach more
understandable in my opinion. Sorry if I miss the real design for that.

3. I get a wrong result of pg_relation_filepath() function for global temp table,
I think it's necessaryto keep this an correct output.

4. In gtt_search_by_relid() function, it has not handle the missing_ok argument
if gtt_storage_local_hash is null. There should be some comments if it's the right
code.

5. It's a long patch and hard to review, I think it will pretty good if it can be
divided into several subpatches with relatively independent subfunctions.


Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Re: [Proposal] Global temporary tables

От
曾文旌
Дата:
Thank you very much for reviewing this patch.
This is very important to improve the GTT.

2020年8月3日 下午3:09,movead.li@highgo.ca 写道:


>Fixed in global_temporary_table_v29-pg13.patch
>Please check.

I find this is the most latest mail with an attachment, so I test and reply on
this thread, several points as below:

1. I notice it produces new relfilenode when new session login and some
data insert. But the relfilenode column in pg_class still the one when create
the global temp table. I think you can try to show 0 in this area as what nail
relation does. 
I think getting the GTT to have a default relfilenode looks closer to the existing implementation, and setting it to 0 requires extra work and has no clear benefit.
What do you think?
I'd like to know the reasons for your suggestion.


2. The nail relations handle their relfilenodes by RelMapFile struct, and this
patch use hash entry and relfilenode_list, maybe RelMapFile approach more
understandable in my opinion. Sorry if I miss the real design for that.
We can see the STORAGE and statistics info for the GTT, including relfilenode, through view pg_gtt_relstats

postgres=# \d gtt
                Table "public.gtt"
 Column |  Type   | Collation | Nullable | Default 
--------+---------+-----------+----------+---------
 a      | integer |           |          | 
 b      | integer |           |          | 

postgres=# insert into gtt values(1,1);
INSERT 0 1
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16384 |        0 |         0 |             0 |          532 |          1
(1 row)

postgres=# truncate gtt;
TRUNCATE TABLE
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16387 |        0 |         0 |             0 |          533 |          1
(1 row)


3. I get a wrong result of pg_relation_filepath() function for global temp table,
I think it's necessaryto keep this an correct output.

postgres=# select pg_relation_filepath(oid) from pg_class where relname = 'gtt';
 pg_relation_filepath 
----------------------
 base/13835/t3_16384
(1 row)

I didn't find anything wrong. Could you please give me a demo.


4. In gtt_search_by_relid() function, it has not handle the missing_ok argument
if gtt_storage_local_hash is null. There should be some comments if it's the right
code.
This is a problem that has been fixed in global_temporary_table_v34-pg13.patch.


5. It's a long patch and hard to review, I think it will pretty good if it can be
divided into several subpatches with relatively independent subfunctions.
Thank you for your suggestion, and I am considering doing so, including adding comments.


Wenjing



Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Вложения

Re: [Proposal] Global temporary tables

От
"movead.li@highgo.ca"
Дата:

>I find this is the most latest mail with an attachment, so I test and reply on
>this thread, several points as below:

>1. I notice it produces new relfilenode when new session login and some
>data insert. But the relfilenode column in pg_class still the one when create
>the global temp table. I think you can try to show 0 in this area as what nail
>relation does. 
>I think getting the GTT to have a default relfilenode looks closer to the existing implementation, and setting it to 0 requires extra work and has no clear benefit.
>What do you think?
>I'd like to know the reasons for your suggestion.
The 'relfilenode' mean the file no on disk which different from oid of a relation,
 the default one is a wrong for gtt, so I think it's not so good to show it in 
pg_class.

>2. The nail relations handle their relfilenodes by RelMapFile struct, and this
>patch use hash entry and relfilenode_list, maybe RelMapFile approach more
>understandable in my opinion. Sorry if I miss the real design for that.
>We can see the STORAGE and statistics info for the GTT, including relfilenode, through view pg_gtt_relstats

postgres=# \d gtt
                Table "public.gtt"
 Column |  Type   | Collation | Nullable | Default 
--------+---------+-----------+----------+---------
 a      | integer |           |          | 
 b      | integer |           |          | 

postgres=# insert into gtt values(1,1);
INSERT 0 1
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16384 |        0 |         0 |             0 |          532 |          1
(1 row)

postgres=# truncate gtt;
TRUNCATE TABLE
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16387 |        0 |         0 |             0 |          533 |          1
(1 row)


I just suggest a way which maybe most naturely to the exist code struct, and it's
uo to you.


>3. I get a wrong result of pg_relation_filepath() function for global temp table,
>I think it's necessaryto keep this an correct output.

postgres=# select pg_relation_filepath(oid) from pg_class where relname = 'gtt';
 pg_relation_filepath 
----------------------
 base/13835/t3_16384
(1 row)

I didn't find anything wrong. Could you please give me a demo.

In my opinoin it should show 'base/13835/t3_16387', other than 'base/13835/t3_16384',
because the relfilenode change to 16387 when you truncate it in step 2.

>4. In gtt_search_by_relid() function, it has not handle the missing_ok argument
>if gtt_storage_local_hash is null. There should be some comments if it's the right
>code.
>This is a problem that has been fixed in global_temporary_table_v34-pg13.patch.
Sorry about it, I can not find it in mail thread and maybe I miss something. The mail thread
is so long, it's better to create a new mail thread I think.


Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Re: [Proposal] Global temporary tables

От
曾文旌
Дата:


2020年8月7日 下午5:30,movead.li@highgo.ca 写道:


>I find this is the most latest mail with an attachment, so I test and reply on
>this thread, several points as below:

>1. I notice it produces new relfilenode when new session login and some
>data insert. But the relfilenode column in pg_class still the one when create
>the global temp table. I think you can try to show 0 in this area as what nail
>relation does. 
>I think getting the GTT to have a default relfilenode looks closer to the existing implementation, and setting it to 0 requires extra work and has no clear benefit.
>What do you think?
>I'd like to know the reasons for your suggestion.
The 'relfilenode' mean the file no on disk which different from oid of a relation,
 the default one is a wrong for gtt, so I think it's not so good to show it in 
pg_class.

>2. The nail relations handle their relfilenodes by RelMapFile struct, and this
>patch use hash entry and relfilenode_list, maybe RelMapFile approach more
>understandable in my opinion. Sorry if I miss the real design for that.
>We can see the STORAGE and statistics info for the GTT, including relfilenode, through view pg_gtt_relstats

postgres=# \d gtt
                Table "public.gtt"
 Column |  Type   | Collation | Nullable | Default 
--------+---------+-----------+----------+---------
 a      | integer |           |          | 
 b      | integer |           |          | 

postgres=# insert into gtt values(1,1);
INSERT 0 1
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16384 |        0 |         0 |             0 |          532 |          1
(1 row)

postgres=# truncate gtt;
TRUNCATE TABLE
postgres=# select * from pg_gtt_relstats ;
 schemaname | tablename | relfilenode | relpages | reltuples | relallvisible | relfrozenxid | relminmxid 
------------+-----------+-------------+----------+-----------+---------------+--------------+------------
 public     | gtt       |       16387 |        0 |         0 |             0 |          533 |          1
(1 row)


I just suggest a way which maybe most naturely to the exist code struct, and it's
uo to you.


>3. I get a wrong result of pg_relation_filepath() function for global temp table,
>I think it's necessaryto keep this an correct output.

postgres=# select pg_relation_filepath(oid) from pg_class where relname = 'gtt';
 pg_relation_filepath 
----------------------
 base/13835/t3_16384
(1 row)

I didn't find anything wrong. Could you please give me a demo.

In my opinoin it should show 'base/13835/t3_16387', other than 'base/13835/t3_16384',
because the relfilenode change to 16387 when you truncate it in step 2.

>4. In gtt_search_by_relid() function, it has not handle the missing_ok argument
>if gtt_storage_local_hash is null. There should be some comments if it's the right
>code.
>This is a problem that has been fixed in global_temporary_table_v34-pg13.patch.
Sorry about it, I can not find it in mail thread and maybe I miss something. The mail thread
is so long, it's better to create a new mail thread I think.

The latest status is tracked here

The latest patch is V35. I don't know why the patches in some of my emails are indexed, but some of them are not.



Wenjing





Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Вложения