Re: Making empty Bitmapsets always be NULL

On Thu, Mar 2, 2023 at 6:59 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Yeah. I split out those executor fixes as 0002; 0003 is the changes
> to bitmapsets proper, and then 0004 removes now-dead code.

+1 to all these patches. Some minor comments from me.

*0003
It seems that the Bitmapset checked by bms_is_empty_internal cannot be
NULL from how it is computed by a function. So I wonder if we can
remove the check of 'a' being NULL in that function, or reduce it to an
Assert.

- if (a == NULL)
- return true;
+ Assert(a != NULL);

*0004
It seems that in create_lateral_join_info around line 689, the
bms_is_empty check of lateral_relids is not necessary, since we've
checked that lateral_relids cannot be NULL several lines earlier.

@@ -682,12 +682,6 @@ create_lateral_join_info(PlannerInfo *root)
if (lateral_relids == NULL)
continue;

- /*
- * We should not have broken the invariant that lateral_relids is
- * exactly NULL if empty.
- */
- Assert(!bms_is_empty(lateral_relids));
-

Thanks
Richard