On Tue, 12 Mar 2024 at 23:21, David Rowley <dgrowle...@gmail.com> wrote: > I've attached v3.
I spent quite a bit more time looking at this. I discovered that the dNumGroups wasn't being set as it should have been for INTERSECT and EXCEPT queries. There was a plan change as a result of this. I've fixed this by adjusting where dNumGroups is set. It must be delayed until after the setop child paths have been created. Aside from this, the changes I made were mostly cosmetic. However, I did notice that I wasn't setting the union child RelOptInfo's ec_indexes in add_setop_child_rel_equivalences(). I also discovered that we're not doing that properly for the top-level RelOptInfo for the UNION query prior to this change. The reason is that due to the Var.varno==0 for the top-level UNION targetlist. The code in get_eclass_for_sort_expr() effectively misses this relation due to "while ((i = bms_next_member(newec->ec_relids, i)) > 0)". This happens to be good because there is no root->simple_rel_array[0] entry, so happens to prevent that code crashing. It seems ok that the ec_indexes are not set for the top-level set RelOptInfo as get_eclass_for_sort_expr() does not make use of ec_indexes while searching for an existing EquivalenceClass. Maybe we should fix this varno == 0 hack and adjust get_eclass_for_sort_expr() so that it makes use of the ec_indexes. It's possible to see this happen with a query such as: SELECT oid FROM pg_class UNION SELECT oid FROM pg_class ORDER BY oid; I didn't see that as a reason not to push this patch as this occurs both with and without this change, so I've now pushed this patch. Thank you and Andy for reviewing this. David