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

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.

--
With Regards,
Amit Kapila.