Thomas Munro <thomas.mu...@enterprisedb.com> writes: > Thanks. Here is a version squashed into one commit, with a decent > commit message and a small improvement: the code to create the hash > table is moved into a static function, to avoid repetition. I will > push this to master early next week, unless there is more feedback or > one of you would prefer to do that.
Nitpicky gripes: * In the commit message's references to prior commits, I think it should be standard to refer to master-branch commit hashes unless there's some really good reason to do otherwise (and then you should say that this commit is on branch X). Your reference to the revert commit is pointing to the REL_10_STABLE back-patch commit. * In the (de)serialization code, it seems kinda ugly to me to use "Oid" as the type of the initial count-holding value, rather than "int". I suppose you did that to avoid alignment issues in case Oid should someday be a different size than "int", but it would be a good thing to have a comment explaining that and pointing out specifically that the first item is a count not an OID. (Right now, a reader has to reverse-engineer that fact, which I do not think is polite to the reader.) * Also, I don't much like the lack of any cross-check that SerializeEnumBlacklist has been passed the correct amount of space. I think there should be at least an assert at the end that it hasn't overrun the space the caller gave it. As-is, there's exactly no protection against the possibility that the hash traversal produced a different number of entries than what EstimateEnumBlacklistSpace saw. regards, tom lane