On Tue, Jan 18, 2022 at 11:27 PM Michael Paquier <mich...@paquier.xyz> wrote: > WFM. Attached is a patch that extends --compress to handle a method > with an optional compression level. Some extra tests are added to > cover all that.
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. 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. 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. "First check after the compression method" seems like it would be better written "First check for the compression method" or "First check the compression method". -- Robert Haas EDB: http://www.enterprisedb.com
