Revise the Asserts added to bimapset manipulation functions - Mailing list pgsql-hackers

From Richard Guo
Subject Revise the Asserts added to bimapset manipulation functions
Date
Msg-id CAMbWs4-djy9qYux2gZrtmxA0StrYXJjvB-oqLxn-d7J88t=PQQ@mail.gmail.com
Whole thread Raw
Responses Re: Revise the Asserts added to bimapset manipulation functions
List pgsql-hackers
The Asserts added to bitmapset.c by commits 71a3e8c43b and 7d58f2342b
contain some duplicates, such as in bms_difference, bms_is_subset,
bms_subset_compare, bms_int_members and bms_join.  For instance,

@@ -953,6 +1033,15 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
        int                     lastnonzero;
        int                     shortlen;
        int                     i;
+#ifdef REALLOCATE_BITMAPSETS
+       Bitmapset  *tmp = a;
+#endif
+
+       Assert(a == NULL || IsA(a, Bitmapset));
+       Assert(b == NULL || IsA(b, Bitmapset));
+
+       Assert(a == NULL || IsA(a, Bitmapset));
+       Assert(b == NULL || IsA(b, Bitmapset));

Sorry that I failed to notice those duplicates when reviewing the
patchset, mainly because they were introduced in different patches.

While removing those duplicates, I think we can add checks in the new
Asserts to ensure that Bitmapsets should not contain trailing zero
words, as the old Asserts did.  That makes the Asserts in the form of:

Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));

I think we can define a new macro for this form and use it to check that
a Bitmapset is valid.

In passing, I prefer to move the Asserts to the beginning of functions,
just for paranoia's sake.

Hence, proposed patch attached.

Thanks
Richard
Attachment

pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: pg_stat_statements: more test coverage
Next
From: shveta malik
Date:
Subject: Re: Track in pg_replication_slots the reason why slots conflict?