David Rowley <dgrowleyml@gmail.com> writes: > I'm fine with this one as it's the same as what I already mentioned > earlier. I had imagined doing bms_del_member(bms_copy ... but maybe > the compiler is able to optimise away the additional store. Likely, it > does not matter much as pallocing memory likely adds far more overhead > anyway.
I actually wrote it that way to start with, but undid it after noticing that the existing code in remove_rel_from_restrictinfo does it in separate steps, and thinking that that was good for both separation of concerns and a cleaner git history. I too can't believe that an extra fetch will be noticeable compared to the cost of the adjacent bms_xxx operations.
I also think it seems better to do bms_copy in separate steps, not only because this keeps consistent with the existing code in remove_rel_from_restrictinfo, but also because we need to do bms_del_member twice for each lefthand/righthand relid set.
Speaking of consistency, do you think it would improve the code's readability if we rearrange the code in remove_rel_from_query so that the modifications of the same relid set are grouped together, just like what we do in remove_rel_from_restrictinfo? I mean something like:
