Re: bug in update tuple routing with foreign partitions

(2019/04/11 20:31), Etsuro Fujita wrote:
> (2019/04/11 14:33), Amit Langote wrote:
>> BTW, have you noticed that the RETURNING clause returns the same row
>> twice?
>
> I noticed this, but I didn't think it hard. :(
>
>> +update utrtest set a = 3 from (values (2), (3)) s(x) where a = s.x
>> returning *;
>> + a | b | x
>> +---+-------------------+---
>> + 3 | qux triggered ! | 2
>> + 3 | xyzzy triggered ! | 3
>> + 3 | qux triggered ! | 3
>> +(3 rows)
>>
>> You can see that the row that's moved into remp is returned twice (one
>> with "qux triggered!" in b column), whereas it should've been only once?
>
> Yeah, this is unexpected behavior, so will look into this.

I think the reason for that is: the row routed to remp is incorrectly 
fetched as a to-be-updated row when updating remp as a subplan 
targetrel.  The right way to fix this would be to have some way in 
postgres_fdw in which we don't fetch such rows when updating such 
subplan targetrels.  I tried to figure out a (simple) way to do that, 
but I couldn't.  One probably-simple solution I came up with is to sort 
subplan targetrels into the order in which foreign-table subplan 
targetrels get processed first in ExecModifyTable().  (Note: currently, 
rows can be moved from local partitions to a foreign-table partition, 
but rows cannot be moved from foreign-table partitions to another 
partition, so we wouldn't encounter this situation once we sort like 
that.)  But I think that's ugly, and I don't think it's a good idea to 
change the core, just for postgres_fdw.  So what I'm thinking is to 
throw an error for cases like this.  (Though, I think we should keep to 
allow rows to be moved from local partitions to a foreign-table subplan 
targetrel that has been updated already.)

What do you think about that?

Best regards,
Etsuro Fujita