On Tue, 26 Sept 2023 at 21:45, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > However, I agree that changing find_base_rel() the way you have done > in your patch is fine and mildly future-proof. +1 to that idea > irrespective of what bitmapset functions do.
I'm not a fan of adding additional run-time overhead for this theoretical problem. find_base_rel() could be made more robust for free by just casting the relid and simple_rel_array_size to uint32 while checking that relid < root->simple_rel_array_size. The 0th element should be NULL anyway, so "if (rel)" should let relid==0 calls through and allow that to ERROR still. I see that just changes a "jle" to "jnb" vs adding an additional jump for Ranier's version. [1] It seems worth not making find_base_rel() more expensive than it is today as commonly we just reference root->simple_rel_array[n] directly anyway because it's cheaper. It would be nice if we didn't add further overhead to find_base_rel() as this would make the case for using PlannerInfo.simple_rel_array directly even stronger. David [1] https://godbolt.org/z/qrxKTbvva
