Thread: postgres_fdw binary protocol support
Hi everyone, I have made a patch that introduces support for libpq binary protocol in postgres_fdw. The idea is simple, when a user knows that the foreign server is binary compatible with the local and his workload could somehow benefit from using binary protocol, it can be switched on for a particular server or even a particular table. The patch adds a new foreign server and table option 'binary_format' (by default off) and implements serialization/deserialization of query results and parameters for binary protocol. I have tested the patch by switching foreign servers in postgres_fdw.sql tests to binary_mode, the only diff was in the text of the error for parsing an invalid integer value, so it worked as expected for the test. There are a few minor issues I don't like in the code and I am yet to write the tests and docs for it. It would be great to get some feedback and understand, whether this is a welcome feature, before proceeding with all of the abovementioned. Thanks, Ilya Gladyshev
Attachment
Hi Illya,
On Mon, Nov 21, 2022 at 8:50 PM Ilya Gladyshev
<ilya.v.gladyshev@gmail.com> wrote:
>
> Hi everyone,
>
> I have made a patch that introduces support for libpq binary protocol
> in postgres_fdw. The idea is simple, when a user knows that the foreign
> server is binary compatible with the local and his workload could
> somehow benefit from using binary protocol, it can be switched on for a
> particular server or even a particular table.
>
Why do we need this feature? If it's for performance then do we have
performance numbers?
AFAIU, binary compatibility of two postgresql servers depends upon the
binary compatibility of the platforms on which they run. So probably
postgres_fdw can not infer the binary compatibility by itself. Is that
true? We have many postgres_fdw options that user needs to set
manually to benefit from them. It will be good to infer those
automatically as much as possible. Hence this question.
> The patch adds a new foreign server and table option 'binary_format'
> (by default off) and implements serialization/deserialization of query
> results and parameters for binary protocol. I have tested the patch by
> switching foreign servers in postgres_fdw.sql tests to binary_mode, the
> only diff was in the text of the error for parsing an invalid integer
> value, so it worked as expected for the test. There are a few minor
> issues I don't like in the code and I am yet to write the tests and
> docs for it. It would be great to get some feedback and understand,
> whether this is a welcome feature, before proceeding with all of the
> abovementioned.
>
About the patch itself, I see a lot of if (binary) {} else {} block
which are repeated. It will be good if we can add functions/macros to
avoid duplication.
--
Best Wishes,
Ashutosh Bapat
On Tue, 22 Nov 2022 at 08:17, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > > AFAIU, binary compatibility of two postgresql servers depends upon the > binary compatibility of the platforms on which they run. No, libpq binary mode is not architecture-specific. I think you're thinking of on-disk binary compatibility. But libpq binary mode is just a binary network representation of the data instead of an ascii representation. It should be faster and more efficient but it still goes through binary input/output functions (which aren't named input/output) I actually wonder if having this would be a good way to get some code coverage of the binary input/output functions which I suspect is sadly lacking now. It wouldn't necessarily test that they're doing what they're supposed to... but at least they would be getting run which I don't think they are currently? -- greg
> 22 нояб. 2022 г., в 17:10, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> написал(а):
>
> Hi Illya,
>
> On Mon, Nov 21, 2022 at 8:50 PM Ilya Gladyshev
> <ilya.v.gladyshev@gmail.com> wrote:
>>
>> Hi everyone,
>>
>> I have made a patch that introduces support for libpq binary protocol
>> in postgres_fdw. The idea is simple, when a user knows that the foreign
>> server is binary compatible with the local and his workload could
>> somehow benefit from using binary protocol, it can be switched on for a
>> particular server or even a particular table.
>>
>
> Why do we need this feature? If it's for performance then do we have
> performance numbers?
Yes, it is for performance, but I am yet to do the benchmarks. My initial idea was that binary protocol must be more
efficientthan text, because as I understand that’s the whole point of it. However, the minor tests that I have done do
notprove this and I couldn’t find any benchmarks for it online, so I will do further tests to find a use case for it.
> About the patch itself, I see a lot of if (binary) {} else {} block
> which are repeated. It will be good if we can add functions/macros to
> avoid duplication.
Yea, that’s true, I have some ideas about improving it