Re: [PATCH] Logical decoding of TRUNCATE

On 22/01/18 19:45, Petr Jelinek wrote:
> On 19/01/18 12:37, Marco Nenciarini wrote:
>> Hi All,
>>>>> + /* logicalrep_rel_close call not needed, because ExecuteTruncateGuts
>>>>> + * already closes the relations. Setting localrel to NULL in the map entry
>>>>> + * is still needed.
>>>>> + */
>>>>> + rel->localrel = NULL;
>>>>
>>>> This is somewhat ugly. Perhaps the ExecuteTruncateGuts should track
>>>> which relations it opened and only close those and the rest should be
>>>> closed by caller? That should also remove the other ugly part which is
>>>> that the ExecuteTruncateGuts modifies the input list. What if caller
>>>> wanted to use those relations it sent as parameter later?
>>>
>>> Agreed
>>>
>>
>> Attached a new version of the patch addressing these issues.
>>
>
> Besides the small thing I wrote for the 0001 in the other thread I am
> pretty much happy with this now.
>

Actually on second look, I don't like the new boolean parameter much.
I'd rather we didn't touch the input list and always close only
relations opened inside the ExecuteTruncateGuts().

It may mean more list(s) but the current interface is still not clean.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services