Buglets in equivclass.c
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: David Rowley <dgrowleyml@gmail.com>
Date: 2020-10-04T17:29:41Z
Lists: pgsql-hackers
Attachments
- fix-equivclass-buglets.patch (text/x-diff) patch
While hacking on a patch that touches equivclass.c, I came across a couple of things that seemed wrong, and are fixed by the attached proposed patch. First, get_eclass_for_sort_expr() computes expr_relids and nullable_relids too soon. This is a waste of a not-really-trivial number of cycles in the common cases where it finds an existing eclass or is told not to make a new one. More subtly, the bitmapsets are computed in the caller's context. If we do use them, they will be attached to an EquivalenceClass that lives in the potentially-longer-lived root->planner_cxt, allowing the EC's pointers to them to become dangling. This would be a live bug if get_eclass_for_sort_expr() could be called with create_it = true during GEQO planning. So far as I can find, it is not; but both its API spec and its internal comments certainly give the impression that that's allowed. Second, generate_join_implied_equalities() uses inner_rel->relids to look up relevant eclasses, but given the surrounding code it seems like it ought to be using nominal_inner_relids. The code accidentally works because a child RelOptInfo will always have exactly the same eclass_indexes as its top parent; but if it did not, we'd risk either missing some relevant eclasses or hitting the assertion that claims that all the eclasses we find overlap nominal_join_relids. (I noticed this while speculating that maybe we needn't bother maintaining eclass_indexes for child RelOptInfos. This code is one place that would fail if we didn't.) I'm unsure whether to back-patch either of these. They both seem to be just latent bugs so far as the core code is concerned, but the first one in particular seems like something that could bite extension code. Thoughts? regards, tom lane
Commits
-
Fix two latent(?) bugs in equivclass.c.
- e3bd026fbb91 9.5.24 landed
- f6a23344f8c8 9.6.20 landed
- d5109a163f46 11.10 landed
- d1c23d726d50 13.1 landed
- 74dae41e5ac3 10.15 landed
- 53c6daff4364 14.0 landed
- 1f94d76856bc 12.5 landed