On Sat, 12 Jan 2019 at 21:49, David Rowley <david.row...@2ndquadrant.com> wrote: > > Can you say what you think is wrong with this way of making a copy of the > > ECs? > > If you shallow copy with memcpy(new_ec, ec, > sizeof(EquivalenceClass));, then fields such as ec_relids will just > point to the same memory as the parent PlannerInfo's > EquivalenceClasses, so when you do your adjustment (as above) on the > child eclasses, you'll modify memory belonging to the parent. I'd > assume you'd not want to do that since you need to keep the parent > intact at least to make copies for other child rels. You'd have > gotten away with it before since you performed a list_copy() and only > were deleting the parent's EMs with list_delete_cell() which was > modifying the copy, not the original list. Since you were missing the > alteration to ec_relids, then you might not have seen issues with the > shallow copy, but now that you are changing that field, are you not > modifying the ec_relids field that still set in the parent's > PlannerInfo? In this instance, you've sidestepped that issue by using > bms_difference() which creates a copy of the Bitmapset and modifies > that. I think you'd probably see issues if you tried to use > bms_del_members(). I think not doing the deep copy is going to give > other people making changes in this are a hard time in the future.
Setting to waiting on author pending clarification about shallow vs deep copying of ECs. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
