Re: Partition-wise join for join between (declaratively) partitioned tables

On 2017/09/11 16:23, Ashutosh Bapat wrote:
> On Sat, Sep 9, 2017 at 6:28 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> I'm a bit suspicious about the fact that there are now executor
>> changes related to the PlanRowMarks. If the rowmark's prti is now the
>> intermediate parent's RT index rather than the top-parent's RT index,
>> it'd seem like that'd matter somehow. Maybe it doesn't, because the
>> code that cares about prti seems to only care about whether it's
>> different from rti. But if that's true everywhere, then why even
>> change this? I think we might be well off not to tinker with things
>> that don't need to be changed.
>
> In the definition of ExecRowMark, I see
> Index prti; /* parent range table index, if child */
> It just says parent, by which I take as direct parent. For
> inheritance, which earlier flattened inheritance hierarchy, direct
> parent was top parent. So, probably nobody thought whether a parent is
> direct parent or top parent. But now that we have introduced that
> concept we need to interpret this comment anew. And I think
> interpreting it as direct parent is non-lossy.

But then we also don't have anything to say about why we're making that
change. If you could describe what non-lossy is in this context, then
fine. But that we'd like to match with what we're going to do for
AppendRelInfos does not seem to be a sufficient explanation for this change.

> If we set top parent's
> index, parent RTI in AppendRelInfo and PlanRowMark would not agree.
> So, it looks quite natural that we set the direct parent's index in
> PlanRowMark.

They would not agree, yes, but aren't they unrelated? If we have a reason
for them to agree, (for example, row-locking breaks in the inherited table
case if we didn't), then we should definitely make them agree.

Updating the comment for prti definition might be something that this
patch could (should?) do, but I'm not quite sure about that too.

Thanks,
Amit