RE: User defined data types in Logical Replication

Hi Sawada-san,

> >> eventually we'll need to map types to local oid (and possibly more)
> >> where the local info is cached so that we can interpret binary
> >> representation of replicated data (which we'll add at some point
> >> since it's big performance boost).
>
> Sounds good.
>
> >>
> >> So I am afraid that if we do the rename of typmap to remotetype in
> >> this patch it will a) make backports of fixes in the related code
> >> harder, b) force us to rename it back again in the future.
> >
> > Thanks for your comment.
> >
> >> I'd keep your general approach but keep using typmap naming.
> >
> > I update the patch as Petr Jelineks mention, keep using typmap naming.
> >
>
> Thank you for updating the patch. Here is a review comment.

Thanks for reviewing.

> - if (errarg->attnum < 0)
> + rel = errarg->rel;
> + remote_attnum = rel->attrmap[errarg->local_attnum];
> +
> + if (remote_attnum < 0)
> return;
>
> I think errarg->local_attnum can be -1 here and access an invalid address
> if slot_store_error_callback() is called before setting
> errarg.local_attnum. I cannot see such call path in the current code so
> far but would need to be fixed.

I updated the patch to fix it.

---
Thanks and best regards,
Dang Minh Huong
NEC Solution Innovators, Ltd.
http://www.nec-solutioninnovators.co.jp/en/