On Mon, 17 Jul 2023 at 15:31, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I also didn't do anything about ExtensibleNode types. I assume just
> > copying the ExtensibleNode isn't good enough. To flat copy the actual
> > node I think would require adding a new function to
> > ExtensibleNodeMethods.
>
> Yeah, the problem I've got with this approach is that flat-copying
> FDW and Custom paths would require extending the respective APIs.
> While that's a perfectly reasonable ask if we only need to do this
> in HEAD, it would be a nonstarter for released branches. Is it
> okay to only fix this issue in HEAD?
CustomPaths, I didn't think about those. That certainly makes it more
complex. I also now see the header comment for struct CustomPath
mentioning that we don't copy Paths:
* Core code must avoid assuming that the CustomPath is only as large as
* the structure declared here; providers are allowed to make it the first
* element in a larger structure. (Since the planner never copies Paths,
* this doesn't add any complication.) However, for consistency with the
* FDW case, we provide a "custom_private" field in CustomPath; providers
* may prefer to use that rather than define another struct type.
Are there any legitimate reasons to look at the input_rel's pathlist
again aside from debugging? I can't think of any. Perhaps back
branches can be fixed by just emptying the path lists and NULLifying
the cheapest paths as you mentioned last week.
> > I was also unsure what we should do when shallow copying a List.
>
> The proposal is to shallow-copy a Path node. List is not a kind
> of Path, so how does List get into it? (Lists below Paths would
> not get copied, by definition.)
The patch contained infrastructure to copy any Node type. Not just
Paths. Perhaps that's more than what's needed, but it seemed more
effort to limit it just to Path types than to make it "work" for all
Node types.
David.