range_deserialize + range_get_flags
От | Alvaro Herrera |
---|---|
Тема | range_deserialize + range_get_flags |
Дата | |
Msg-id | 20200306200343.GA625@alvherre.pgsql обсуждение исходный текст |
Список | 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
Вложения
В списке pgsql-hackers по дате отправления: