Hi,

On 18/12/17 14:28, Huong Dangminh wrote:
>>>>
>>>> Your patch is fine for me.
>>>> But logicalrep_typmap_getid will be unused.
>>>
>>> Yeah, actually we don't use LogicalRepTyp.typoid as well. If we allow
>>> to replicate data to an another data type of column, what is the data
>>> type mapping on subscriber for? It seems enough to have remote data
>>> type oid, namespace and name.
>>
>> Yeah, so the meaning of the type map will be disappeared, aren't you?
>> I updated the patch with removing of LogicalRepTyp.typoid and changing
>> concept "typmap" to "remote type".
>> How do you think?
> 
> Sorry, Please confirm V7 of patch (attached in this mail).
> 

I think the changes make sense in terms of how it all works now.

That said I don't think the renaming idea is a good one, the naming was
chosen to be future proof because 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).

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.

I'd keep your general approach but keep using typmap naming.

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

Reply via email to