Re: Adding doubly linked list type which stores the number of items in the list

On Mon, Oct 31, 2022 at 6:58 PM Aleksander Alekseev
<aleksander(at)timescale(dot)com> wrote:
>
> Hi hackers,
>
> > I will take another look at v3 tomorrow and probably mark it RfC.
>
> I very much like the patch. While on it:
>
> ```
> +static inline bool
> +dclist_is_empty(dclist_head *head)
> +{
> + Assert(dlist_is_empty(&head->dlist) == (head->count == 0));
> + return (head->count == 0);
> +}
> ```
>
> Should we consider const'ifying the arguments of the dlist_*/dclist_*
> functions that don't change the arguments?

+1, but as a separate discussion/thread/patch IMO.

> Additionally it doesn't seem that we have any unit tests for dlist /
> dclist. Should we consider adding unit tests for them to
> src/test/regress?

Most of the dlist_* functions are being covered I guess. AFAICS,
dclist_* functions that aren't covered are dclist_insert_after(),
dclist_insert_before(), dclist_pop_head_node(), dclist_move_head(),
dclist_move_tail(), dclist_has_next(), dclist_has_prev(),
dclist_next_node(), dclist_prev_node(), dclist_head_element_off(),
dclist_head_node(), dclist_tail_element_off(), dclist_head_element().

IMO, adding an extension under src/test/modules to cover missing or
all dlist_* and dclist_* functions makes sense. It improves the code
coverage. FWIW, test_lfind is one such recent test extension.

> To clarify, IMO both questions are out of scope of this specific patch
> and should be submitted separately.

You're right, both of them must be discussed separately.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com