Fixing a few minor misusages of bms_union()
От | David Rowley |
---|---|
Тема | Fixing a few minor misusages of bms_union() |
Дата | |
Msg-id | CAApHDvoCcoS-p5tZNJLTxFOKTYNjqVh7Dwf+5ikDUBwnvWftRw@mail.gmail.com обсуждение исходный текст |
Ответы |
Re: Fixing a few minor misusages of bms_union()
Re: Fixing a few minor misusages of bms_union() |
Список | pgsql-hackers |
While working in prepunion.c, I noticed that generate_union_paths() has some code being called in a loop that's doing: relids = bms_union(relids, rel->relids); Since bms_union() creates a new set rather than reusing one of its input parameter sets, it makes this appear to be an inefficient way of accumulating the required set of relids. bms_add_members() seems better suited to this job. From what I can tell, there are 2 other places where we do something similar: markNullableIfNeeded() and substitute_phv_relids_walker(). These two are slightly different as they're not "accumulating" something in a loop like the above, but to me, they also look like they don't need to reallocate a completely new Bitmapset. I believe using bms_add_members() would only be an issue if there were multiple pointers pointing to the same set. In that case, modifying the set in-place might have an unintentional effect on the other pointers... However, we know that having multiple pointers pointing to the same set is "Trouble" as there can be a repalloc() when the set is modified and needs more Bitmapwords. That would cause issues unless the code was always careful to assign the modified set to all pointers. Since the call sites I've mentioned don't assign the result of bms_union() to multiple pointers, I think the attached patch is safe. Posting here to see if anyone knows a reason for not doing this that I've overlooked. David (For the record, the other two cases I found that don't seem valid to change are in create_nestloop_plan() and finalize_plan()).
Вложения
В списке pgsql-hackers по дате отправления: