On Thu, Mar 15, 2018 at 9:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Wouldn't a ProjectionPath just need the same additional translation
>> fields that I've bolted onto AppendPath to make it work properly?
>
> Well, I guess I'm not sure.
Sorry, hit send too soon there. I'm not sure I entirely understand
the purpose of those changes; I think the comments could use some
work. For example:
+ /*
+ * First we must record the translation expressions in the PlannerInfo.
+ * These need to be found when the expression translation is being done
+ * when the final plan is being assembled.
+ */
+ /*
+ * Record this child as having been promoted. Some places treat child
+ * relations in a special way, and this will give them a VIP ticket to
+ * adulthood, where required.
+ */
These comments just recapitulate what the code does, without really
explaining what problem we're trying to solve ("need" for unspecified
reasons, unspecified "special" handling).
That said, I gather that one problem is the path might contain
references to child varnos where we need to reference parent varnos.
That does seem like something we need to handle, but I'm not sure
whether this is really the right method. I wonder if we couldn't
deduce the necessary translations from the parent pointers of the
paths. That is, if we've got a proxy path associated with the parent
rel and the path it's proxying is associated with the child rel, then
we need to translate from realpath->parent->relids to
proxypath->parent_relids.
Not that this is an argument of overwhelming import, but note that
ExecSupportsMarkRestore wouldn't need to change if we used
ProjectionPath; that's already got the required handling.
If we stick with your idea of using AppendPath, do we actually need
generate_proxy_paths()? What paths get lost if we don't have a
special case for them here?
I find promote_child_rel_equivalences() kind of scary. It seems like
a change that might have unintended and hard-to-predict consequences.
Perhaps that's only my own lack of knowledge.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
