Re: Binary support for pgoutput plugin - Mailing list pgsql-hackers

From Dmitry Dolgov
Subject Re: Binary support for pgoutput plugin
Date
Msg-id 20191027150228.brktafmnynzsoly7@localhost
Whole thread Raw
In response to Re: Binary support for pgoutput plugin  (Dave Cramer <davecramer@gmail.com>)
Responses Re: Binary support for pgoutput plugin
List pgsql-hackers
> On Mon, Jun 17, 2019 at 10:29:26AM -0400, Dave Cramer wrote:
> > Which is what I have done. Thanks
> >
> > I've attached both patches for comments.
> > I still have to add documentation.
>
> Additional patch for documentation.

Thank you for the patch! Unfortunately 0002 has some conflicts, could
you please send a rebased version? In the meantime I have few questions:

    -LogicalRepRelId
    +void
     logicalrep_read_insert(StringInfo in, LogicalRepTupleData *newtup)
     {
        char        action;
    -    LogicalRepRelId relid;
    -
    -    /* read the relation id */
    -    relid = pq_getmsgint(in, 4);

        action = pq_getmsgbyte(in);
        if (action != 'N')
    @@ -175,7 +173,6 @@ logicalrep_read_insert(StringInfo in, LogicalRepTupleData *newtup)

        logicalrep_read_tuple(in, newtup);

    -    return relid;
     }

    ....

    -    relid = logicalrep_read_insert(s, &newtup);
    +    /* read the relation id */
    +    relid = pq_getmsgint(s, 4);
        rel = logicalrep_rel_open(relid, RowExclusiveLock);
    +
    +    logicalrep_read_insert(s, &newtup);

Maybe I'm missing something, what is the reason of moving pq_getmsgint
out of logicalrep_read_*? Just from the patch it seems that the code is
equivalent.

> There is one obvious hack where in binary mode I reset the input
> cursor to allow the binary input to be re-read From what I can tell
> the alternative is to convert the data in logicalrep_read_tuple but
> that would require moving a lot of the logic currently in worker.c to
> proto.c. This seems minimally invasive.

Which logic has to be moved for example? Actually it sounds more natural
to me, if this functionality would be in e.g. logicalrep_read_tuple
rather than slot_store_data, since it has something to do with reading
data. And it seems that in pglogical it's also located in
pglogical_read_tuple.



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Proposal: Add more compile-time asserts to exposeinconsistencies.
Next
From: Tom Lane
Date:
Subject: Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'