Re: Sort support for macaddr8

From: Chapman Flack <chap(at)anastigmatix(dot)net>
To: Melanie Plageman <melanieplageman(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Sort support for macaddr8
Date: 2019-06-03 21:59:13
Message-ID: 425071ba-022c-850f-9d1e-2084c2cf8b59@anastigmatix.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chapman Flack 2019-06-03 22:04:29 Re: Sort support for macaddr8
Previous Message Jared Rulison 2019-06-03 21:54:41 Use of multi-column gin index