On Mon, Jan 25, 2021 at 10:42:25PM -0500, Sehrope Sarkuni wrote: > +1 to adding a SHA1 SQL function. Even if it's deprecated, there's plenty > of historical usage that I can see it being useful.
Let's wait for more opinions to see if we agree that this addition is helpful or not. Even if this is not added, I think that there is still value in refactoring the code anyway for the SHA-2 functions. > Either way, the rest of the refactor can be improved a bit to perform a > single palloc() and remove the memcpy(). Attached is a diff for > cryptohashfuncs.c that does that by writing the digest final directly to > the result. It also removes the digest length arg and determines it in the > switch block. There's only one correct digest length for each type so > there's no reason to give callers the option to give the wrong one. Yeah, what you have here is better. + default: + elog(ERROR, "unsupported digest type %d", type); Not using a default clause is the purpose here, as it would generate a compilation warning if a value in the enum is forgotten. Hence, if a new option is added to pg_cryptohash_type in the future, people won't miss that they could add a SQL function for the new option. If we decide that MD5 and SHA1 have no need to use this code path, I'd rather just use elog(ERROR) instead. -- Michael
signature.asc
Description: PGP signature