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.etsuro@lab.ntt.co.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