Hi Nathan, > I don't think adding crc32c() would sufficiently increase the scope. We'd > use the existing implementations for both crc32() and crc32c(). And > besides, this could be useful for adding tests for that code. > > + <function>crc32</function> ( <type>text</type> ) > > Do we need a version of the function that takes a text input? It's easy > enough to cast to a bytea. > > + <returnvalue>text</returnvalue> > > My first reaction is that we should just have this return bytea like the > SHA ones do, if for no other reason than commit 10cfce3 seems intended to > move us away from returning text for these kinds of functions. Upthread, > you mentioned the possibility of returning a bigint, too. I think I'd > still prefer bytea in case we want to add, say, crc64() or crc16() in the > future. That would allow us to keep all of these functions consistent > instead of returning different types for each. However, I understand that > returning the numeric types might be more convenient. I'm curious what > others think about this. > > + Computes the CRC32 <link linkend="functions-hash-note">hash</link> of > + the binary string, with the result written in hexadecimal. > > I'm not sure we should call the check values "hashes." Wikipedia does > include them in the "List of hash functions" page [0], but it seems to > deliberately avoid calling them hashes in the CRC page [1]. I'd suggest > calling them "CRC32 values" instead.
Thanks for the code review. Here is the updated patch. -- Best regards, Aleksander Alekseev
v2-0001-Add-crc32-bytea-crc32c-bytea.patch
Description: Binary data
