Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1egBur-0006iT-KJ for pgsql-hackers@arkaria.postgresql.org; Mon, 29 Jan 2018 16:04:18 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1egBuq-0003W8-Oz for pgsql-hackers@arkaria.postgresql.org; Mon, 29 Jan 2018 16:04:16 +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_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1egBuq-0003Vy-Gk for pgsql-hackers@lists.postgresql.org; Mon, 29 Jan 2018 16:04:16 +0000 Received: from mail.postgrespro.ru ([93.174.131.138]) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1egBuf-0005tC-Q2 for pgsql-hackers@postgresql.org; Mon, 29 Jan 2018 16:04:16 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id 22F5D21C1C39; Mon, 29 Jan 2018 19:04:04 +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 wp.localdomain (gw.postgrespro.ru [93.174.131.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id C945F21C042B; Mon, 29 Jan 2018 19:04:03 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1517241843; bh=MCp/oKKhxsE4biIQrFnxQ/hvXcF/edFPdWEMf3vTWlg=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=Rj3g1sAkPuB58SK2DmwLcAzrc4TuMQmzLobU/Q5/MdDzygRC3d/iEu9GcjlYYeNQ/ zW/kcpMOhPGYQhKgICgeEIWMyRG8cs7KN4RnaJ8lGnD0+kqLuRoAm4/Z9N+HGIyS3q zu2DxKqLxbTC5efvz95ZM+e2PVr3L2eTMfvrHRa0= Date: Mon, 29 Jan 2018 19:04:03 +0300 From: Ildus Kurbangaliev To: Ildar Musin Cc: Robert Haas , Alexander Korotkov , Tomas Vondra , Alvaro Herrera , =?UTF-8?B?0JXQstCz0LXQvdC40Lkg?= =?UTF-8?B?0KjQuNGI0LrQuNC9?= , Andres Freund , Oleg Bartunov , Craig Ringer , Peter Eisentraut , PostgreSQL Hackers , Chapman Flack Subject: Re: [HACKERS] Custom compression methods Message-ID: <20180129190403.3eb59d4a@wp.localdomain> In-Reply-To: References: <20171201194859.le5hvnnrjzhxhm2t@alvherre.pgsql> <20171211155555.05ddd2fc@postgrespro.ru> <20171213151818.75a20259@postgrespro.ru> <20171218115431.2b3e29ba@wp.localdomain> <20180115024930.48583c69@hh> <0633c6e5-5328-5c5b-708b-c4409e3fd131@postgrespro.ru> <20180123160454.131ade09@wp.localdomain> <6fd5dea3-8889-c7bb-9df2-79493b2b6ab0@postgrespro.ru> <20180125172457.5967c022@wp.localdomain> <20180129144452.08f04dc3@wp.localdomain> Organization: Postgres Professional X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk On Mon, 29 Jan 2018 17:29:29 +0300 Ildar Musin wrote: > > Patch applies cleanly, builds without any warnings, documentation > builds ok, all tests pass. > > A remark for the committers. The patch is quite big, so I really wish > more reviewers looked into it for more comprehensive review. Also a > native english speaker should check the documentation and comments. > Another thing is that tests don't cover cmdrop method because the > built-in pglz compression doesn't use it (I know there is an jsonbd > extension [1] based on this patch and which should benefit from > cmdrop method, but it doesn't test it either yet). > > I think I did what I could and so passing this patch to committers > for the review. Changed status to "Ready for committer". > > > [1] https://github.com/postgrespro/jsonbd > Thank you! About cmdrop, I checked that's is called manually, but going to check it thoroughly in my extension. -- --- Ildus Kurbangaliev Postgres Professional: http://www.postgrespro.com Russian Postgres Company