Date: Fri, 19 Mar 2021 01:28:00 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Daniil Zakhlystov <usernamedt@yandex-team.ru>
Cc: pgsql-hackers@lists.postgresql.org,
	Konstantin Knizhnik <knizhnik@garret.ru>
Subject: Re: libpq compression
Message-ID: <20210319062800.GI11765@telsasoft.com>
References: <aad16e41-b3f9-e89d-fa57-fb4c694bec25@postgrespro.ru>
 <a364948a-d93b-fe17-f62d-334f34360daa@postgrespro.ru>
 <161609580905.28624.5304095609680400810.pgcf@coridan.postgresql.org>
 <20210319010232.GH11765@telsasoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210319010232.GH11765@telsasoft.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210319062800.GI11765%40telsasoft.com>
Precedence: bulk

On Thu, Mar 18, 2021 at 08:02:32PM -0500, Justin Pryzby wrote:
> On Thu, Mar 18, 2021 at 07:30:09PM +0000, Daniil Zakhlystov wrote:
> > The new status of this patch is: Ready for Committer
> 
> The CF bot is failing , because the last patch sent to the list is from January:
> | Latest attachment (libpq-compression-31.patch) at 2021-01-12 14:05:22 from Konstantin Knizhnik ...
> 
> The most recent messages have all had links to github repos without patches
> attached.
> 
> Also, the branches I've looked at on the github repos all have messy history,
> and need to be squished into a coherent commit or set of commits.
> 
> Would you send a patch to the list with the commits as you propose to merge
> them ?

This needs some significant effort:

 - squish commits;
   * maybe make an 0001 commit supporting zlib, and an 0002 commit adding zstd;
   * add an 0003 patch to enable zlib compression by default (for CI - not for merge);
 - rebase to current master;
 - compile without warnings;
 - assign OIDs at the end of the ranges given by find-unused-oids to avoid
   conflicts with patches being merged to master.

Currently, make check-world gets stuck in several places when I use zlib.

There's commit messages claiming that the compression can be "asymmetric"
between client-server vs server-client, but the commit seems to be unrelated,
and the protocol documentation doesn't describe how this works.

Previously, I suggested that the server should have a "policy" GUC defining
which compression methods are allowed.  Possibly including compression "level".
For example, the default might be to allow zstd, but only up to level 9.

This:
+       /* Initialise values and NULL flags arrays */
+       MemSet(values, 0, sizeof(values));
+       MemSet(nulls, 0, sizeof(nulls));

can be just:
+       bool            nulls[PG_STAT_NETWORK_TRAFFIC_COLS] = {false};
since values is fully populated below.

typo: aslogirhm

I wrote about Solution.pm earlier in this thread, and I see that it's in
Konstantin's repo since Jan 12, but it's not in yours (?) so I think windows
build will fail.

Likewise, commit 1a946e14e in his branch adds compression into to psql
\connninfo based on my suggestion, but it's missing in your branch.

I've added Daniil as a 2nd author and set back to "waiting on author".