Re: EquivalenceClasses and subqueries and PlaceHolderVars, oh my
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yeb Havinga <yebhavinga@gmail.com>, pgsql-hackers@postgresql.org
Date: 2012-03-16T15:16:14Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix pushing of index-expression qualifications through UNION ALL.
- b28ffd0fcc58 9.2.0 cited
-
Wrap appendrel member outputs in PlaceHolderVars in additional cases.
- 57664ed25e5d 9.2.0 cited
Attachments
- placeholdervar-fix-this-time-for-sure.patch (text/x-patch) patch
I wrote: > So I now propose reverting the earlier two patches (but not their > regression test cases of course) and instead hacking MergeAppend plan > building as per (2). Attached is a draft patch for that. There are several things going on here: * Revert the preceding two patches (except for their regression test cases). * Install defenses to ensure that child EC members are ignored except in very specific contexts, and improve documentation around that. The most significant change is that get_eclass_for_sort_expr now takes a Relids argument, and won't consider child members unless they match the Relids. It turns out that the places that were trying to match indexes to ECs already acted that way; which I think I had done just as a speed optimization, but it turns out to be important for correctness too. * Rewrite prepare_sort_from_pathkeys() to allow required sort column numbers to be passed in, thus fixing Teodor's original problem more directly. As part of that, I removed createplan.c's add_sort_column() code, which was meant as a last-ditch check that we don't build sorts with useless duplicate sort columns. As far as I can tell, that's dead code and has been for a long time, because we already eliminate duplicate pathkeys or SortGroupClause list entries far upstream of this. Even if there are some corner cases where it does something useful, that's rare enough to not really justify expending the cycles. I had to remove it because if it did fire and remove a sort column, the outer loop in prepare_sort_from_pathkeys() wouldn't be in sync with the input column-number array. * While testing this I noticed that planagg.c failed to optimize min/max aggregates on appendrels that are made from UNION ALL subqueries rather than inherited relations. So this patch also tweaks planagg.c to handle such cases. Viewing the problem this way, the only known symptom is the originally reported "MergeAppend child's targetlist doesn't match MergeAppend", which of course is not an issue before 9.1, so I'm not going to try to back-patch further than 9.1. It is possible that we need some of the child EC member defenses further back, but I'll await some evidence of user-visible bugs first. regards, tom lane