Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.

(2018/07/13 23:05), Ashutosh Bapat wrote:
> On Thu, Jul 12, 2018 at 4:32 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> In this example, the value of the whole-row reference to the child table
>> ptp1 for that record is ('foo',1), and that of the index expression for that
>> record is (1,'foo'). Those have different column orders, but the latter
>> could be mapped to the former by a technique like do_convert_tuple.

> The expression in this case would look like ptp1::pt::ptp1 which won't
> match targetlist expression ptp1. I am also doubtful that the planner
> will be able to deduce that it need to apply an inverse function of
> ::pt and what exactly such an inverse function is. So index only scan
> won't be picked.

>> we could support index-only scans
>> with such an index in the case where we have the whole-row reference in the
>> targetlist, not the index expression itself.
>
> Can you please show an index only scan path being created in this case?

We currently don't consider index-only scan with index expressions, so I
haven't thought in detail yet about how the planner would work. But
once we have that index-only scan, I think we could extend that to the
case mentioned above, by adding this to the planner: if the index
expression is of the form var::parenttype, consider that (not only the
expression itself but) var can be returned from the index. I think the
expression like ptp1::pt::ptp1 would be useful to get the value of ptp1
from the index at execution time.

>>> There's a patch in an adjacent thread started by David Rowley to rip
>>> out Append/MergeAppend when there is only one subplan. So, your
>>> solution won't work there.
>>
>>
>> Thanks for sharing that information! I skimmed the thread. I haven't yet
>> caught up with all the discussions there, so I think I'm missing something,
>> but it looks like that we haven't yet reached any consensus on the way to
>> go. In my opinion, I like the approach mentioned in [1]. And if we go that
>> way, my patch seems to fit into that, because in that approach the
>> Append/MergeAppend could be removed after adjusting the targetlists for its
>> subplans in create_append_plan/create_merge_append_plan. Anyway, I'd like
>> to join in that work for PG12.
>
> Whatever may be the outcome of that work, I think what we fix here
> shouldn't require to reverted in a few months from now, just so that
> that patch works.

I think we could add that optimization without reverting this change
because the essential part of this change is to make create_plan adjust
the tlists of the subplans based on the instruction stored into the
subplans' RelOptInfos (ie, need_adjust_tlist in the second version of
the patch). I think this technique could be extended even to the case
where we have that optimization.

Best regards,
Etsuro Fujita