On Tue, 26 Sept 2023 at 21:45, Ashutosh Bapat
<ashutosh.bapat.oss@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