Re: libpq compression

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>, Robbie Harwood <rharwood(at)redhat(dot)com>
Cc: Andreas Karlsson <andreas(at)proxel(dot)se>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq compression
Date: 2019-02-14 16:45:33
Message-ID: CA+q6zcW=JyhpyxVK-c-Yt3OO54CYRcL6jKtOkxvrM+Jz-oCYBg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

For the records, I'm really afraid of interfering with the conversation at this
point, but I believe it's necessary for the sake of a good feature :)

> On Wed, Feb 13, 2019 at 4:03 PM Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru> wrote:
>
> 1. When decompressor has not enough data to produce any extra output, it
> doesn't return error. It just not moving forward output position in the
> buffer.

I'm confused, because here is what I see in zlib:

// zlib.h
inflate() returns Z_OK if some progress has been made, ... , Z_BUF_ERROR
if no progress was possible or if there was not enough room in the output
buffer when Z_FINISH is used. Note that Z_BUF_ERROR is not fatal, and
inflate() can be called again with more input and more output space to
continue decompressing.

So, sounds like if no moving forward happened, there should be Z_BUF_ERROR. But
I haven't experimented with this yet to figure out.

> Ok, but still I think that it is better to pass tx/rx function to stream.
> There are two important advantages:
> 1. It eliminates code duplication.

Ok.

> 2. It allows to use (in future) this streaming compression not only for
> libpq for for other streaming data.

Can you elaborate on this please?

> Concerning "layering violation" may be it is better to introduce some other
> functions something like inflate_read, deflate_write and call them instead of
> secure_read. But from my point of view it will not improve readability and
> modularity of code.

If we will unwrap the current compression logic to not contain tx/rx functions,
isn't it going to be the same as you describing it anyway, just from the higher
point of view? What I'm saying is that there is a compression logic, for it
some data would be read or written from it, just not right here an now by
compression code itself, but rather by already existing machinery (which could
be even beneficial for the patch implementation).

> And I do not see any disadvantages.

The main disadvantage, as I see it, is that there is no agreement about this
approach. Probably in such situations it makes sense to experiment with
different suggestions, to see how would they look like - let's be flexible.

> On Wed, Feb 13, 2019 at 8:34 PM Robbie Harwood <rharwood(at)redhat(dot)com> wrote:
>
> In order to comply with your evident desires, consider this message a
> courtesy notice that I will no longer be reviewing this patch or
> accepting future code from you.

I'm failing to see why miscommunication should necessarily lead to such
statements, but it's your decision after all.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Oleksii Kliukin 2019-02-14 16:56:17 Per-tablespace autovacuum settings
Previous Message Tom Lane 2019-02-14 16:43:05 Inconsistencies between dependency.c and objectaddress.c