Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gtw47-0000S1-Uw for pgsql-hackers@arkaria.postgresql.org; Wed, 13 Feb 2019 15:03:12 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gtw46-0003Of-Gw for pgsql-hackers@arkaria.postgresql.org; Wed, 13 Feb 2019 15:03:10 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gtw46-0003OY-4C for pgsql-hackers@lists.postgresql.org; Wed, 13 Feb 2019 15:03:10 +0000 Received: from mail.postgrespro.ru ([93.174.131.138]) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gtw42-0004zX-CZ for pgsql-hackers@postgresql.org; Wed, 13 Feb 2019 15:03:09 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id 465EA21CAED0; Wed, 13 Feb 2019 18:03:05 +0300 (MSK) X-Virus-Scanned: Debian amavisd-new at postgrespro.ru X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=x tagged_above=-99 required=4 WHITELISTED tests=[] autolearn=unavailable Received: from [192.168.27.200] (gw.postgrespro.ru [93.174.131.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id 0A87721CD3CB; Wed, 13 Feb 2019 18:03:04 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1550070185; bh=gBB19/cI5Hc4isTELzVzBTBl/DERYJG6uNEcntaxfLA=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=knMHxOwrqsxTfOnM/xq+mELKFy5xguFvussErEdetkJCozd+QLt5HdUN7X9H87iFQ aQzU5ZGN1HNSz6FmWNf0Qyx5ZizrpMDL6Fs0FAFGL0tUKhFyZhUSeOZzgdA8lecfhi kQ6RsFFHBBQCHuKDATV3aNlcGK8C5uVFMfuTlJuM= Subject: Re: libpq compression To: Dmitry Dolgov <9erthalion6@gmail.com> Cc: Andreas Karlsson , Peter Eisentraut , Andres Freund , PostgreSQL Hackers References: <20190208071431.skfmpbeclrkpzuac@alap3.anarazel.de> <8d210267-a2fa-662f-b167-0e5991d45dd7@proxel.se> <436d1e08-dd9c-d36a-1c0d-517a70a83cd4@proxel.se> <92435f1b-77aa-4386-4cf1-788b3c3d4365@postgrespro.ru> From: Konstantin Knizhnik Message-ID: <8da581ac-62a5-a64a-7dc4-f09ce936612a@postgrespro.ru> Date: Wed, 13 Feb 2019 18:03:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk On 13.02.2019 17:54, Dmitry Dolgov wrote: >> On Wed, Feb 13, 2019 at 3:52 PM Dmitry Dolgov <9erthalion6@gmail.com> wrote: >> >>> On Wed, Feb 13, 2019 at 3:46 PM Konstantin Knizhnik >>> wrote: >>> >>> Moreover, please notice that your implementation is still passing functions >>> tx/rx functions to stream constructor and so zpq_read is still able to read >>> data itself. So I do not understand which problem you have solved by >>> replacing zpq_read with pair of zpq_read_drain+zpq_read. >> Nope, I've removed the call of these functions from zlib_read/write, just >> forgot to remove the initialization part. > Oh, I see the source of confusion. Due to lack of time I've implemented my > changes only for zlib part, sorry that I didn't mention that before. And I have looked at zstd part;) 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. 2. It allows to use (in future) this streaming compression not only for libpq for for other streaming data. And I do not see any disadvantages. 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. -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company