On Mon, Dec 18, 2017 at 11:25 PM, Huong Dangminh <huo-dangm...@ys.jp.nec.com> wrote: >> 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. - 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. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center