David Rowley <david.row...@2ndquadrant.com> writes:
> On 12 June 2018 at 02:26, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> ... I'd previously tried having NULL subnodes in
>>> the Append and I thought it required too much hacking work in
>>> explain.c,
>> No, that was pretty much exactly what I was envisioning.
> What you're proposing exchanges logic that fits neatly into one
> function for special logic that will be scattered all over explain.c.
> I really don't think that's the better of the two evils.
As far as I can see, it'd involve about three or four lines' worth of
rewrite in one place-you-already-made-quite-ugly in explain.c, and an
added if-test in planstate_walk_members, and that'd be it. That seems
like a pretty cheap price for being able to vastly simplify the invariants
for the pruning functions. In fact, I doubt you'd even need two of them
anymore; just one with a bool flag for can-use-exec-params.
> I'd rather just see my last patch applied which simplifies the re-map
> code by removing the 2nd loop. Actually, even updating the
> present_parts to remove the unneeded subpartitioned table indexes is
> optional. We' just need to give up on the elog(ERROR, "partition
> missing from subplans"); error and assume that case is fine.
The fact that you added that loop and only later decided it was
unnecessary seems to me to support my point pretty strongly: that
code is overcomplicated.
regards, tom lane
