On 6/3/19 5:03 PM, Chapman Flack wrote: > On 6/3/19 3:23 PM, Melanie Plageman wrote: >> Peter and I implemented this small (attached) patch to extend >> abbreviated key compare sort to macaddr8 datatype (currently supported >> for macaddr). > > Am I going cross-eyed, or would the memset be serving more of a purpose > if it were in the SIZEOF_DATUM != 8 branch?
It looks like a copy-pasto coming from mac.c, where the size of the thing to be compared isn't itself 8 bytes. With sizeof(macaddr) being 6, that original code may have had these cases in mind: - SIZEOF_DATUM is something smaller than 6 (likely 4). The whole key doesn't fit, but that's ok, because abbreviated "equality" just means to recheck with the authoritative routine. - SIZEOF_DATUM is exactly 6. Probably not a thing. - SIZEOF_DATUM is anything larger than 6 (likely 8). Needs the memset. Also, in this case, abbreviated "equality" could be taken as true equality, never needing the authoritative fallback. For macaddr8, the cases morph into these: - SIZEOF_DATUM is something smaller than 8 (likely 4). Ok; it's just an abbreviation. - SIZEOF_DATUM is exactly 8. Now an actual thing, even likely. - SIZEOF_DATUM is larger than 8. Our flying cars run postgres, and we need the memset to make sure they don't crash. This leaves me with a couple of questions: 1. (This one seems like a bug.) In the little-endian case, if SIZEOF_DATUM is smaller than the type, I'm not convinced by doing the DatumBigEndianToNative() after the memcpy(). Seems like that's too late to make sure the most-significant bytes got copied. 2. (This one seems like an API opportunity.) If it becomes common to add abbreviation support for smallish types such that (as here, when SIZEOF_DATUM >= 8), an abbreviated "equality" result is in fact authoritative, would it be worthwhile to have some way for the sort support routine to announce that fact to the caller? That could spare the caller the effort of re-checking with the authoritative routine. It could also (by making the equality case less costly) end up changing the weight assigned to the cardinality estimate in deciding whether to abbrev.. Regards, -Chap
