From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Jeff Davis <pgsql(at)j-davis(dot)com>, Paul Jungwirth <pj(at)illuminatedcomputing(dot)com> |
Subject: | range_deserialize + range_get_flags |
Date: | 2020-03-06 20:03:43 |
Message-ID: | 20200306200343.GA625@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I noticed a weird thing about rangetypes API while reviewing multiranges
-- the combination of range_deserialize immediately followed by
range_get_flags. It seems quite odd to have range_deserialize return
only one flag (empty) and force callers to use a separate
range_get_flags call in order to fetch anything else they need. I
propose that it's simpler to have range_deserialize have an out param
for flags (replacing "empty"), and callers can examine "IsEmpty" from
that using a macro accessor. So there are two macros now:
RangeFlagsIsEmpty() takes the 'flags' value and return whether the bit
is set. Its companion RangeIsEmpty() does the range_get_flags() dance.
The attached patch does that, with a net savings of 8 lines of code in
rangetypes.c. I know, it's not amazing. But it's slightly cleaner this
way IMO.
The reason things are this way is that initially (commit 4429f6a9e3e1)
were all opaque; the external observer could only see "empty" when
deserializing the value. Then commit 37ee4b75db8f added
range_get_flags() to obtain the flags from a range, but at that point it
was only used in places that did not deserialized the range anyway, so
it was okay. I think it was commit c66e4f138b04 that ended up having
both deserialize and get_flags in succession. So things are weird now,
but they have not always been like that.
I also chose to represent the flags out param as uint8 in
range_deserialize. With this change, the compiler warns for callers
using the old API (it used to take bool *), giving a chance to update.
--
Álvaro Herrera
Attachment | Content-Type | Size |
---|---|---|
range-deserialize-empty.patch | text/x-diff | 32.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-03-06 20:04:27 | Re: More tests to stress directly checksum_impl.h |
Previous Message | Chapman Flack | 2020-03-06 19:53:23 | Re: Unicode escapes with any backend encoding |