range_deserialize + range_get_flags

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

Browse pgsql-hackers by date

  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