Re: [Proposal] Global temporary tables

po 3. 2. 2020 v 14:03 odesílatel 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>
napsal:

>
>
> 2020年2月2日 上午2:00，Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> 写道：
>
>
>
> so 1. 2. 2020 v 14:39 odesílatel 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>
> napsal:
>
>>
>>
>> 2020年1月30日 下午10:21，Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> 写道：
>>
>>
>>
>> čt 30. 1. 2020 v 15:17 odesílatel 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>
>> napsal:
>>
>>>
>>>
>>> > 2020年1月29日 下午9:48，Robert Haas <robertmhaas(at)gmail(dot)com> 写道：
>>> >
>>> > On Tue, Jan 28, 2020 at 12:12 PM 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>
>>> wrote:
>>> >>> Opinion by Pavel
>>> >>> + rel->rd_islocaltemp = true; <<<<<<< if this is valid, then the
>>> name of field "rd_islocaltemp" is not probably best
>>> >>> I renamed rd_islocaltemp
>>> >>
>>> >> I don't see any change?
>>> >>
>>> >> Rename rd_islocaltemp to rd_istemp in
>>> global_temporary_table_v8-pg13.patch
>>> >
>>> > In view of commit 6919b7e3294702adc39effd16634b2715d04f012, I think
>>> > that this has approximately a 0% chance of being acceptable. If you're
>>> > setting a field in a way that is inconsistent with the current use of
>>> > the field, you're probably doing it wrong, because the field has an
>>> > existing purpose to which new code must conform. And if you're not
>>> > doing that, then you don't need to rename it.
>>> Thank you for pointing it out.
>>> I've rolled back the rename.
>>> But I still need rd_localtemp to be true, The reason is that
>>> 1 GTT The GTT needs to support DML in read-only transactions ,like local
>>> temp table.
>>> 2 GTT does not need to hold the lock before modifying the index buffer
>>> ,also like local temp table.
>>>
>>> Please give me feedback.
>>>
>>
>> maybe some like
>>
>> rel->rd_globaltemp = true;
>>
>> and somewhere else
>>
>> if (rel->rd_localtemp || rel->rd_globaltemp)
>> {
>> ...
>> }
>>
>> I tried to optimize code in global_temporary_table_v10-pg13.patch
>>
>>
>> Please give me feedback.
>>
>
> I tested this patch and I have not any objections - from my user
> perspective it is work as I expect
>
> +#define RELATION_IS_TEMP(relation) \
> + ((relation)->rd_islocaltemp || \
> + (relation)->rd_rel->relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
>
> It looks little bit unbalanced
>
> maybe is better to inject rd_isglobaltemp to relation structure
>
> and then
>
> it should to like
>
> +#define RELATION_IS_TEMP(relation) \
> + ((relation)->rd_islocaltemp || \
> + (relation)->rd_isglobaltemp))
>
> But I have not idea if it helps in complex
>
> In my opinion
> For local temp table we need (relation)->rd_rel->relpersistence ==
> RELPERSISTENCE_TEMP
> and because one local temp table belongs to only one session, need to mark
> one sessions rd_islocaltemp = true ,and other to rd_islocaltemp = false.
>

so it means so table is assigned to current session or not. In this moment
I think so name "islocaltemp" is not the best - because there can be "local
temp table" that has this value false.

The name should be better describe so this table as attached to only
current session, and not for other, or is accessed by all session.

In this case I can understand why for GTT is possible to write
..rd_islocaltemp = true. But is signal so rd_islocaltemp is not good name
and rd_istemp is not good name too.

> But For GTT, just need (relation)->rd_rel->relpersistence ==
> RELPERSISTENCE_GLOBAL_GLOBAL_TEMP
> One GTT can be used for every session, so no need rd_isglobaltemp
> anymore. This seems duplicated and redundant.
>

I didn't understand well the sematic of rd_islocaltemp so my ideas in this
topics was not good. Now I think so rd_islocalname is not good name and can
be renamed if some body find better name. "istemptable" is not good too,
because there is important if relation is attached to the session or not.

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