Re: Refactoring of compression options in pg_basebackup
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "David G. Johnston" <david.g.johnston@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Magnus Hagander <magnus@hagander.net>, Georgios Kokolatos <gkokolatos@pm.me>, Postgres hackers <pgsql-hackers@lists.postgresql.org>, Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Date: 2022-01-20T07:03:02Z
Lists: pgsql-hackers
Attachments
- v6-0001-Rework-the-option-set-of-pg_basebackup.patch (text/x-diff) patch v6-0001
On Wed, Jan 19, 2022 at 08:35:23AM -0500, Robert Haas wrote: > I think that this will reject something like --compress=nonetheless by > telling you that 't' is not a valid separator. I think it would be > better to code this so that we first identify the portion preceding > the first colon, or the whole string if there is no colon. Then we > check whether that part is a compression method that we recognize. If > not, we complain. Well, if no colon is specified, we still need to check if optarg is a pure integer if it does not match any of the supported methods, as --compress=0 should be backward compatible with no compression and --compress=1~9 should imply gzip, no? > If so, we then check whatever is after the separator > for validity - and this might differ by type. For example, we could > then immediately reject none:4, and if in the future we want to allow > lz4:fast3, we could. Okay. > I think the code that handles the bare integer case should be at the > top of the function and should return, because that code is short. > Then the rest of the function doesn't need to be indented as deeply. Done this way, I hope. -- Michael
Commits
-
Extend the options of pg_basebackup to control compression
- 5c649fe15336 15.0 landed
-
Add TAP tests for pg_basebackup with compression
- 50e144193c77 15.0 landed
-
Refactor tar method of walmethods.c to rely on the compression method
- 000f3adfdc43 15.0 landed