Thank you very much for reviewing this patch.
This is very important to improve the GTT.

> 2020年8月3日 下午3:09,movead...@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 <http://www.highgo.ca/> 
> EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to