(2014/08/25 21:58), Albe Laurenz wrote:
> Here is my review:
Thank you for the review!
> I played with it, and apart from Hanada's comments I have found the following:
>
> test=> EXPLAIN (ANALYZE, VERBOSE) UPDATE rtest SET val=NULL WHERE id > 3;
> QUERY PLAN
>
----------------------------------------------------------------------------------------------------------------------------------
> Update on laurenz.rtest (cost=100.00..14134.40 rows=299970 width=10) (actual time=0.005..0.005 rows=0 loops=1)
> -> Foreign Scan on laurenz.rtest (cost=100.00..14134.40 rows=299970 width=10) (actual time=0.002..0.002
rows=299997loops=1)
> Output: id, val, ctid
> Remote SQL: UPDATE laurenz.test SET val = NULL::text WHERE ((id > 3))
> Planning time: 0.179 ms
> Execution time: 3706.919 ms
> (6 rows)
>
> Time: 3708.272 ms
>
> The "actual time" readings are surprising.
> Shouldn't these similar to the actual execution time, since most of the time is spent
> in the foreign scan node?
I was also thinkng that this is confusing to the users. I think this is
because the patch executes the UPDATE/DELETE statement during
postgresBeginForeignScan, not postgresIterateForeignScan, as you
mentioned below:
> Reading the code, I noticed that the pushed down UPDATE or DELETE statement is executed
> during postgresBeginForeignScan rather than during postgresIterateForeignScan.
> It probably does not matter, but is there a reason to do it different from the normal scan?
I'll modify the patch so as to execute the statement during
postgresIterateForeignScan.
> It is not expected that postgresReScanForeignScan is called when the UPDATE/DELETE
> is pushed down, right? Maybe it would make sense to add an assertion for that.
IIUC, that is right. As ModifyTable doesn't support rescan currently,
postgresReScanForeignScan needn't to be called in the update pushdown
case. The assertion is a good idea. I'll add it.
Thanks,
Best regards,
Etsuro Fujita