Etsuro Fujita wrote:
> Done.  (I've left deparseDirectUpdateSql/deparseDirectDeleteSql as-is,
> though.)
> 
> Other changes:
> 
> * Address the comments from Eitoku-san.
> * Add regression tests.
> * Fix a bug, which fails to show the actual row counts in EXPLAIN
> ANALYZE for UPDATE/DELETE without a RETURNING clause.
> * Rebase to HEAD.
> 
> Please find attached an updated version of the patch.

Here is my review:

The patch Applies fine, Builds without warning and passes make Check,
so the ABC of patch reviewing is fine.

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=299997 loops=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?

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?

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.

I ran a simple performance test and found that performance is improved as 
expected;
updating 100000 rows took 1000 rather than 8000 ms, and DELETING the same amount
took 200 instead of 6500 ms.

Yours,
Laurenz Albe

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to