Re: [bug] Logical Decoding of relation rewrite with toast does not reset toast_hash

On 8/12/21 1:28 PM, Amit Kapila wrote:
> On Thu, Aug 12, 2021 at 4:30 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Tue, Aug 10, 2021 at 5:30 PM Drouvot, Bertrand <bdrouvot(at)amazon(dot)com> wrote:
>>>
>>> Please find attached the new version that:
>>>
>>> - sets "relwrewrite" for the toast.
>>>
>> --- a/src/backend/commands/tablecmds.c
>> +++ b/src/backend/commands/tablecmds.c
>> @@ -3861,6 +3861,10 @@ RenameRelationInternal(Oid myrelid, const char
>> *newrelname, bool is_internal, bo
>> */
>> namestrcpy(&(relform->relname), newrelname);
>>
>> + /* reset relrewrite for toast */
>> + if (relform->relkind == RELKIND_TOASTVALUE)
>> + relform->relrewrite = InvalidOid;
>> +
>>
>> I find this change quite ad-hoc. I think this API is quite generic to
>> make such a change. I see two ways for this (a) pass a new bool flag
>> (reset_toast_rewrite) in this API and then make this change, (b) in
>> the specific place where we need this, change relrewrite separately
>> via a new API.
>>
>> I would prefer (b) in the ideal case, but I understand it would be an
>> additional cost, so maybe (a) is also okay. What do you people think?
>>
> One minor comment:
> +/*
> + * Test decoding relation rewrite with toast.
> + * The insert into tbl2 within the same transaction
> + * is there to check there is no remaining toast_hash
> + * not being reset.
> + */
>
> You can extend each line of comment up to 80 chars. The current one
> looks a bit odd.

Thanks. I'll update the patch and comment that way once we have decided
if we are going for a) or b).

Thanks

Bertrand