Re: Fixing a few minor misusages of bms_union()

David Rowley <dgrowleyml@gmail.com> writes:
> Posting here to see if anyone knows a reason for not doing this that
> I've overlooked.

This change in substitute_phv_relids_walker is *not* safe according
to the routine's head comment:

 * NOTE: although this has the form of a walker, we cheat and modify the
 * nodes in-place.  This should be OK since the tree was copied by
 * pullup_replace_vars earlier.  Avoid scribbling on the original values of
 * the bitmapsets, though, because expression_tree_mutator doesn't copy those.

The change in generate_union_paths is obviously safe, though, since
that "relids" is entirely locally built.

I'm not convinced one way or the other about changing
markNullableIfNeeded.  I can't offhand think of a reason why
a Var would be sharing varnullingrels with some other node
at this point in the proceedings.  However, the comment
suggests that varnullingrels is probably NULL anyway, so that
there's nothing to be gained.

            regards, tom lane