Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-20 Thread Ashutosh Bapat
The subject of this thread is not directly related to this discussion and we have a new thread [1] for relevant discussion. So, let's discuss this further on that thread. [1] https://www.postgresql.org/message-id/CAF1DzPU8Kx%2BfMXEbFoP289xtm3bz3t%2BZfxhmKavr98Bh-C0TqQ%40mail.gmail.com On Tue, A

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-17 Thread Kyotaro HORIGUCHI
At Tue, 18 Apr 2017 09:12:07 +0530, Ashutosh Bapat wrote in > On Tue, Apr 18, 2017 at 8:12 AM, Kyotaro HORIGUCHI > wrote: > > At Mon, 17 Apr 2017 17:50:58 +0530, Ashutosh Bapat > > wrote in > > > >> On Mon, Apr 17, 2017 at 1:53 PM, Kyotaro HORIGUCHI > >> wrote: > >> > At Thu, 13 Apr 2017

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-17 Thread Ashutosh Bapat
On Tue, Apr 18, 2017 at 8:12 AM, Kyotaro HORIGUCHI wrote: > At Mon, 17 Apr 2017 17:50:58 +0530, Ashutosh Bapat > wrote in > >> On Mon, Apr 17, 2017 at 1:53 PM, Kyotaro HORIGUCHI >> wrote: >> > At Thu, 13 Apr 2017 13:04:12 -0400, Robert Haas >> > wrote in >> > >> >> On Thu, Apr 21, 2016 at

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-17 Thread Kyotaro HORIGUCHI
At Mon, 17 Apr 2017 17:50:58 +0530, Ashutosh Bapat wrote in > On Mon, Apr 17, 2017 at 1:53 PM, Kyotaro HORIGUCHI > wrote: > > At Thu, 13 Apr 2017 13:04:12 -0400, Robert Haas > > wrote in > > > >> On Thu, Apr 21, 2016 at 10:53 AM, Robert Haas > >> wrote: > >> > On Thu, Apr 21, 2016 at 8:4

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-17 Thread Ashutosh Bapat
On Mon, Apr 17, 2017 at 1:53 PM, Kyotaro HORIGUCHI wrote: > At Thu, 13 Apr 2017 13:04:12 -0400, Robert Haas wrote > in >> On Thu, Apr 21, 2016 at 10:53 AM, Robert Haas wrote: >> > On Thu, Apr 21, 2016 at 8:44 AM, Michael Paquier >> > wrote: >> >> On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-17 Thread Kyotaro HORIGUCHI
At Thu, 13 Apr 2017 13:04:12 -0400, Robert Haas wrote in > On Thu, Apr 21, 2016 at 10:53 AM, Robert Haas wrote: > > On Thu, Apr 21, 2016 at 8:44 AM, Michael Paquier > > wrote: > >> On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita > >> wrote: > >>> Attached is an updated version of the patch, wh

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2017-04-13 Thread Robert Haas
On Thu, Apr 21, 2016 at 10:53 AM, Robert Haas wrote: > On Thu, Apr 21, 2016 at 8:44 AM, Michael Paquier > wrote: >> On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita >> wrote: >>> Attached is an updated version of the patch, which modified Michael's >>> version of the patch, as I proposed in [1] (s

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-05-26 Thread Etsuro Fujita
On 2016/05/17 0:25, Robert Haas wrote: On Wed, May 11, 2016 at 3:20 AM, Etsuro Fujita wrote: Thanks for the review! I'll add this to the next CF. I think this should be addressed in advance of the release of 9.6, though. I agree. Committed. Thanks! Best regards, Etsuro Fujita -- Se

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-05-16 Thread Robert Haas
On Wed, May 11, 2016 at 3:20 AM, Etsuro Fujita wrote: > Thanks for the review! > > I'll add this to the next CF. I think this should be addressed in advance > of the release of 9.6, though. I agree. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-05-11 Thread Etsuro Fujita
On 2016/04/28 13:45, Michael Paquier wrote: On Wed, Apr 27, 2016 at 12:16 PM, Etsuro Fujita wrote: On 2016/04/26 21:45, Etsuro Fujita wrote: While re-reviewing the fix, I noticed that since PQcancel we added to pgfdw_xact_callback to cancel a DML pushdown query isn't followed by a ROLLBACK, th

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-27 Thread Michael Paquier
On Wed, Apr 27, 2016 at 12:16 PM, Etsuro Fujita wrote: > On 2016/04/26 21:45, Etsuro Fujita wrote: >> While re-reviewing the fix, I noticed that since PQcancel we added to >> pgfdw_xact_callback to cancel a DML pushdown query isn't followed by a >> ROLLBACK, the connection to the remote server wil

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-26 Thread Etsuro Fujita
On 2016/04/26 21:45, Etsuro Fujita wrote: While re-reviewing the fix, I noticed that since PQcancel we added to pgfdw_xact_callback to cancel a DML pushdown query isn't followed by a ROLLBACK, the connection to the remote server will be discarded at the end of the while loop in that function, whi

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-26 Thread Etsuro Fujita
Hi, While re-reviewing the fix, I noticed that since PQcancel we added to pgfdw_xact_callback to cancel a DML pushdown query isn't followed by a ROLLBACK, the connection to the remote server will be discarded at the end of the while loop in that function, which will cause a FATAL error of "co

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-21 Thread Etsuro Fujita
On 2016/04/22 9:22, Michael Paquier wrote: On Thu, Apr 21, 2016 at 11:53 PM, Robert Haas wrote: I have committed this patch. Thanks for working on this. Sorry for the delay. Thanks for the push. open_item--; Thank you, Robert and Michael. Best regards, Etsuro Fujita -- Sent via pgsq

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-21 Thread Michael Paquier
On Thu, Apr 21, 2016 at 11:53 PM, Robert Haas wrote: > On Thu, Apr 21, 2016 at 8:44 AM, Michael Paquier > wrote: >> On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita >> wrote: >>> Attached is an updated version of the patch, which modified Michael's >>> version of the patch, as I proposed in [1] (s

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-21 Thread Robert Haas
On Thu, Apr 21, 2016 at 8:44 AM, Michael Paquier wrote: > On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita > wrote: >> Attached is an updated version of the patch, which modified Michael's >> version of the patch, as I proposed in [1] (see "Other changes:"). I >> modified comments for pgfdw_get_re

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-21 Thread Michael Paquier
On Thu, Apr 21, 2016 at 5:22 PM, Etsuro Fujita wrote: > Attached is an updated version of the patch, which modified Michael's > version of the patch, as I proposed in [1] (see "Other changes:"). I > modified comments for pgfdw_get_result/pgfdw_exec_query also, mainly because > words like "non-blo

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-21 Thread Etsuro Fujita
On 2016/04/19 14:25, Etsuro Fujita wrote: On 2016/04/19 13:59, Michael Paquier wrote: But huge objection to that because this fragilizes the current logic postgres_fdw is based on: PQexec returns the last result to caller, I'd rather not break that logic for 9.6 stability's sake. IIUC, I thin

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Etsuro Fujita
On 2016/04/19 13:59, Michael Paquier wrote: On Tue, Apr 19, 2016 at 1:14 PM, Etsuro Fujita wrote: What do you think about that? + /* Wait for the result */ + res = pgfdw_get_result(conn, query); + if (res == NULL) + pgfdw_report_error(ERROR, NULL, conn, false, query); + last_re

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Michael Paquier
On Tue, Apr 19, 2016 at 1:14 PM, Etsuro Fujita wrote: > The comment "We don't use a PG_TRY block here ..." seems to be wrongly > placed, so I moved that comment. Also, I think it'd be better to call > pgfdw_report_error() with the clear argument set to false, not true, since > we don't need to cl

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Etsuro Fujita
On 2016/04/19 12:45, Etsuro Fujita wrote: On 2016/04/19 12:26, Michael Paquier wrote: Note for Robert: pgfdw_get_result copycats PQexec by discarding all PQresult received except the last one. I think that's fine for the purposes of postgres_fdw, but perhaps you have a different opinion on the m

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Etsuro Fujita
On 2016/04/19 12:26, Michael Paquier wrote: On Tue, Apr 19, 2016 at 12:16 PM, Noah Misch wrote: On Sat, Apr 16, 2016 at 08:59:40AM +0900, Michael Paquier wrote: Here is a new version. I just recalled that I forgot a PQclear() call to clean up results. Thanks for updating the patch! Rober

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Michael Paquier
On Tue, Apr 19, 2016 at 12:16 PM, Noah Misch wrote: > On Sat, Apr 16, 2016 at 08:59:40AM +0900, Michael Paquier wrote: >> On Fri, Apr 15, 2016 at 9:46 PM, Michael Paquier >> wrote: >> > On Fri, Apr 15, 2016 at 8:25 PM, Etsuro Fujita >> > wrote: >> >> How about doing something similar for PQprepa

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-18 Thread Noah Misch
On Sat, Apr 16, 2016 at 08:59:40AM +0900, Michael Paquier wrote: > On Fri, Apr 15, 2016 at 9:46 PM, Michael Paquier > wrote: > > On Fri, Apr 15, 2016 at 8:25 PM, Etsuro Fujita > > wrote: > >> How about doing something similar for PQprepare/PQexecPrepared in > >> postgresExecForeignInsert, postgre

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-15 Thread Michael Paquier
On Fri, Apr 15, 2016 at 9:46 PM, Michael Paquier wrote: > On Fri, Apr 15, 2016 at 8:25 PM, Etsuro Fujita > wrote: >> How about doing something similar for PQprepare/PQexecPrepared in >> postgresExecForeignInsert, postgresExecForeignUpdate, and >> postgresExecForeignDelete? > > Yes, I hesitated to

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-15 Thread Michael Paquier
On Fri, Apr 15, 2016 at 8:25 PM, Etsuro Fujita wrote: > How about doing something similar for PQprepare/PQexecPrepared in > postgresExecForeignInsert, postgresExecForeignUpdate, and > postgresExecForeignDelete? Yes, I hesitated to touch those, but they are good candidates for this new interface,

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-15 Thread Etsuro Fujita
On 2016/04/15 14:31, Michael Paquier wrote: On Thu, Apr 14, 2016 at 10:44 AM, Etsuro Fujita wrote: On 2016/04/13 21:50, Michael Paquier wrote: On Wed, Apr 13, 2016 at 9:46 PM, Robert Haas wrote: On Tue, Apr 12, 2016 at 10:24 PM, Etsuro Fujita wrote: How about we encapsulate the while (PQi

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-14 Thread Michael Paquier
On Thu, Apr 14, 2016 at 10:44 AM, Etsuro Fujita wrote: > On 2016/04/13 21:50, Michael Paquier wrote: >> On Wed, Apr 13, 2016 at 9:46 PM, Robert Haas >> wrote: >>> On Tue, Apr 12, 2016 at 10:24 PM, Etsuro Fujita >>> wrote: > > How about we encapsulate the while (PQisBusy(...)) loop into a

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-13 Thread Etsuro Fujita
On 2016/04/13 21:50, Michael Paquier wrote: On Wed, Apr 13, 2016 at 9:46 PM, Robert Haas wrote: On Tue, Apr 12, 2016 at 10:24 PM, Etsuro Fujita wrote: How about we encapsulate the while (PQisBusy(...)) loop into a new function pgfdw_get_result(), which can be called after first calling PQsend

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-13 Thread Michael Paquier
On Wed, Apr 13, 2016 at 9:46 PM, Robert Haas wrote: > On Tue, Apr 12, 2016 at 10:24 PM, Etsuro Fujita > wrote: >>> How about we encapsulate the while (PQisBusy(...)) loop into a new >>> function pgfdw_get_result(), which can be called after first calling >>> PQsendQueryParams()? So then this cod

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-13 Thread Robert Haas
On Tue, Apr 12, 2016 at 10:24 PM, Etsuro Fujita wrote: >> How about we encapsulate the while (PQisBusy(...)) loop into a new >> function pgfdw_get_result(), which can be called after first calling >> PQsendQueryParams()? So then this code will say dmstate->result = >> pgfdw_get_result(dmstate->co

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-12 Thread Michael Paquier
On Wed, Apr 13, 2016 at 11:24 AM, Etsuro Fujita wrote: > On 2016/04/13 3:14, Robert Haas wrote: >> >> I'm wondering why we are fixing this specific case and not any of the >> other calls to PQexec() or PQexecParams() in postgres_fdw.c. >> >> I mean, many of those instances are cases where the quer

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-12 Thread Etsuro Fujita
On 2016/04/13 3:14, Robert Haas wrote: I'm wondering why we are fixing this specific case and not any of the other calls to PQexec() or PQexecParams() in postgres_fdw.c. I mean, many of those instances are cases where the query isn't likely to run for very long, but certainly "FETCH %d FROM c%u"

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-12 Thread Robert Haas
On Mon, Apr 11, 2016 at 9:45 PM, Michael Paquier wrote: > On Mon, Apr 11, 2016 at 5:16 PM, Etsuro Fujita > wrote: >> On 2016/04/11 12:30, Michael Paquier wrote: >>> >>> + if ((cancel = PQgetCancel(entry->conn))) >>> + { >>> + P

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-11 Thread Michael Paquier
On Mon, Apr 11, 2016 at 5:16 PM, Etsuro Fujita wrote: > On 2016/04/11 12:30, Michael Paquier wrote: >> >> + if ((cancel = PQgetCancel(entry->conn))) >> + { >> + PQcancel(cancel, errbuf, sizeof(errbuf)); >> +

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-11 Thread Etsuro Fujita
On 2016/04/11 12:30, Michael Paquier wrote: + if ((cancel = PQgetCancel(entry->conn))) + { + PQcancel(cancel, errbuf, sizeof(errbuf)); + PQfreeCancel(cancel); + } Wouldn't it be b

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-10 Thread Michael Paquier
On Mon, Apr 11, 2016 at 11:30 AM, Etsuro Fujita wrote: > I agree with Rushabh. Thanks for updating the patch! Yes, not using a PG_TRY/CATCH block is better. We are not doing anything that need to clean up PGresult in case of an unplanned failure. > Another thing I'd like to propose to revise th

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-10 Thread Etsuro Fujita
On 2016/04/08 22:21, Rushabh Lathia wrote: On Fri, Apr 8, 2016 at 6:28 PM, Robert Haas mailto:robertmh...@gmail.com>> wrote: The comment just before the second hunk in the patch says: * We don't use a PG_TRY block here, so be careful not to throw error * withou

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-08 Thread Rushabh Lathia
On Fri, Apr 8, 2016 at 6:28 PM, Robert Haas wrote: > On Fri, Apr 8, 2016 at 3:05 AM, Etsuro Fujita > wrote: > >> What do you think? This open item's seven-day deadline has passed. It > >> would > >> help keep things moving to know whether you consider your latest patch > >> optimal > >> or whe

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-08 Thread Robert Haas
On Fri, Apr 8, 2016 at 3:05 AM, Etsuro Fujita wrote: >> What do you think? This open item's seven-day deadline has passed. It >> would >> help keep things moving to know whether you consider your latest patch >> optimal >> or whether you wish to change it the way Michael described. > > I wish to

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-08 Thread Etsuro Fujita
On 2016/04/08 13:42, Noah Misch wrote: On Tue, Apr 05, 2016 at 03:22:03PM +0900, Etsuro Fujita wrote: On 2016/04/04 20:35, Michael Paquier wrote: On Mon, Apr 4, 2016 at 7:49 PM, Etsuro Fujita wrote: Here is a patch to fix this issue. As proposed by Michael, I modified execute_dml_stmt so tha

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-07 Thread Noah Misch
On Tue, Apr 05, 2016 at 03:22:03PM +0900, Etsuro Fujita wrote: > On 2016/04/04 20:35, Michael Paquier wrote: > >On Mon, Apr 4, 2016 at 7:49 PM, Etsuro Fujita > > wrote: > >>Here is a patch to fix this issue. As proposed by Michael, I modified > >>execute_dml_stmt so that it uses PQsendQueryParams,

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-04 Thread Etsuro Fujita
On 2016/04/04 20:35, Michael Paquier wrote: On Mon, Apr 4, 2016 at 7:49 PM, Etsuro Fujita wrote: Here is a patch to fix this issue. As proposed by Michael, I modified execute_dml_stmt so that it uses PQsendQueryParams, not PQexecParams. Any comments are welcome. + * This is based on pqSock

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-04 Thread Michael Paquier
On Mon, Apr 4, 2016 at 7:49 PM, Etsuro Fujita wrote: > On 2016/03/31 16:38, Etsuro Fujita wrote: >> >> On 2016/03/31 14:07, Noah Misch wrote: >>> >>> On Thu, Mar 24, 2016 at 01:02:57PM +0900, Etsuro Fujita wrote: On 2016/03/24 11:14, Michael Paquier wrote: > > On Wed, Mar 23, 201

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-04-04 Thread Etsuro Fujita
On 2016/03/31 16:38, Etsuro Fujita wrote: On 2016/03/31 14:07, Noah Misch wrote: On Thu, Mar 24, 2016 at 01:02:57PM +0900, Etsuro Fujita wrote: On 2016/03/24 11:14, Michael Paquier wrote: On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: I've noticed that you now can't cancel a query if th

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-31 Thread Etsuro Fujita
On 2016/03/31 14:07, Noah Misch wrote: On Thu, Mar 24, 2016 at 01:02:57PM +0900, Etsuro Fujita wrote: On 2016/03/24 11:14, Michael Paquier wrote: On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: I've noticed that you now can't cancel a query if there's DML pushdown to a foreign server. Th

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-30 Thread Noah Misch
On Thu, Mar 24, 2016 at 01:02:57PM +0900, Etsuro Fujita wrote: > On 2016/03/24 11:14, Michael Paquier wrote: > >On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: > >>I've noticed that you now can't cancel a query if there's DML pushdown > >>to a foreign server. This previously worked while it w

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-24 Thread Michael Paquier
On Fri, Mar 25, 2016 at 3:10 PM, Michael Paquier wrote: > On Thu, Mar 24, 2016 at 1:02 PM, Etsuro Fujita > wrote: >> Thanks for the report, Thom! Thanks for the advice, Michael! > > I am adding that to the list of open items of 9.6 to not forget about it. My bad. Thom did it already :p -- Mich

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-24 Thread Michael Paquier
On Thu, Mar 24, 2016 at 1:02 PM, Etsuro Fujita wrote: > On 2016/03/24 11:14, Michael Paquier wrote: >> >> On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: >>> >>> I've noticed that you now can't cancel a query if there's DML pushdown >>> to a foreign server. This previously worked while it wa

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-23 Thread Etsuro Fujita
On 2016/03/24 11:14, Michael Paquier wrote: On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: I've noticed that you now can't cancel a query if there's DML pushdown to a foreign server. This previously worked while it was sending individual statements as it interrupted and rolled it back. H

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-23 Thread Michael Paquier
On Wed, Mar 23, 2016 at 10:05 PM, Thom Brown wrote: > I've noticed that you now can't cancel a query if there's DML pushdown > to a foreign server. This previously worked while it was sending > individual statements as it interrupted and rolled it back. > > Here's what the local server sees when

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-23 Thread Thom Brown
On 22 March 2016 at 02:30, Etsuro Fujita wrote: > On 2016/03/19 3:30, Robert Haas wrote: >> >> On Fri, Mar 18, 2016 at 5:15 AM, Etsuro Fujita >> wrote: >>> >>> Attached is the updated version of the patch. I've noticed that you now can't cancel a query if there's DML pushdown to a foreign server

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-21 Thread Etsuro Fujita
On 2016/03/19 3:30, Robert Haas wrote: On Fri, Mar 18, 2016 at 5:15 AM, Etsuro Fujita wrote: Attached is the updated version of the patch. Committed. Thank you. Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your su

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-19 Thread Etsuro Fujita
On 2016/03/10 2:56, Robert Haas wrote: I see that you went and changed all of the places that tested for != CMD_SELECT and made them test for == CMD_INSERT || == CMD_UPDATE || == CMD_DELETE instead. I think that's the wrong direction. I think that we should use the != CMD_SELECT version of the

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-19 Thread Robert Haas
On Fri, Mar 18, 2016 at 5:15 AM, Etsuro Fujita wrote: > On 2016/03/10 2:56, Robert Haas wrote: >> I see that you went and changed all of the places that tested for != >> CMD_SELECT and made them test for == CMD_INSERT || == CMD_UPDATE || == >> CMD_DELETE instead. I think that's the wrong directio

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-10 Thread Etsuro Fujita
On 2016/03/10 19:51, Robert Haas wrote: On Wed, Mar 9, 2016 at 3:30 PM, Tom Lane wrote: Robert Haas writes: Just taking a guess here, you might be thinking that instead of something like this... Update on public.ft2 -> Foreign Update on public.ft2 We could boil it all the way d

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-10 Thread Robert Haas
On Wed, Mar 9, 2016 at 3:30 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Mar 9, 2016 at 1:12 PM, Tom Lane wrote: >>> I hadn't been paying any attention to this thread, but I wonder whether >>> this entire approach isn't considerably inferior to what we can do now >>> with the planner pat

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-09 Thread Tom Lane
Robert Haas writes: > On Wed, Mar 9, 2016 at 1:12 PM, Tom Lane wrote: >> I hadn't been paying any attention to this thread, but I wonder whether >> this entire approach isn't considerably inferior to what we can do now >> with the planner pathification patch. To quote from the new docs: > Well,

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-09 Thread Robert Haas
On Wed, Mar 9, 2016 at 1:12 PM, Tom Lane wrote: > Robert Haas writes: >> Overall, I think this is looking pretty good. > > I hadn't been paying any attention to this thread, but I wonder whether > this entire approach isn't considerably inferior to what we can do now > with the planner pathificat

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-09 Thread Tom Lane
Robert Haas writes: > Overall, I think this is looking pretty good. I hadn't been paying any attention to this thread, but I wonder whether this entire approach isn't considerably inferior to what we can do now with the planner pathification patch. To quote from the new docs: PlanForeignModi

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-09 Thread Robert Haas
On Wed, Mar 9, 2016 at 3:30 AM, Etsuro Fujita wrote: > Great! I changed the naming. I also updated docs as proposed by you in a > previous email, and rebased the patch to the latest HEAD. Please find > attached an updated version of the patch. Thanks. The new naming looks much better (and bet

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-09 Thread Etsuro Fujita
On 2016/03/08 2:35, Robert Haas wrote: On Mon, Mar 7, 2016 at 7:53 AM, Etsuro Fujita wrote: Another option to avoid such a hazard would be to remove the two changes from ExecInitModifyTable and create ExecAuxRowMarks and junk filters even in the pushdown case. I made the changes because we won

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-07 Thread Robert Haas
On Mon, Mar 7, 2016 at 7:53 AM, Etsuro Fujita wrote: > Another option to avoid such a hazard would be to remove the two changes > from ExecInitModifyTable and create ExecAuxRowMarks and junk filters even in > the pushdown case. I made the changes because we won't use ExecAuxRowMarks > in that cas

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-07 Thread Etsuro Fujita
On 2016/03/05 5:45, Robert Haas wrote: Some comments on the latest version. I haven't reviewed the postgres_fdw changes in detail here, so this is just about the core changes. Thank you for taking the time to review the patch! I see that show_plan_tlist checks whether the operation is any of

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-04 Thread Robert Haas
On Tue, Feb 23, 2016 at 1:18 AM, Etsuro Fujita wrote: > Thanks again for updating the patch and fixing the issues! Some comments on the latest version. I haven't reviewed the postgres_fdw changes in detail here, so this is just about the core changes. I see that show_plan_tlist checks whether t

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-03-01 Thread Etsuro Fujita
On 2016/02/22 20:13, Rushabh Lathia wrote: PFA update patch, which includes changes into postgresPlanDMLPushdown() to check for join condition before target columns and also fixed couple of whitespace issues. For pushing down an UPDATE/DELETE on a foreign join to the remote, I created a WIP pa

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-22 Thread Etsuro Fujita
On 2016/02/22 20:13, Rushabh Lathia wrote: I did another round of review for the latest patch and well as performed the sanity test, and haven't found any functional issues. Found couple of issue, see in-line comments for the same. Thanks! On Thu, Feb 18, 2016 at 3:15 PM, Etsuro Fujita mailto

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-22 Thread Rushabh Lathia
I did another round of review for the latest patch and well as performed the sanity test, and haven't found any functional issues. Found couple of issue, see in-line comments for the same. On Thu, Feb 18, 2016 at 3:15 PM, Etsuro Fujita wrote: > On 2016/02/12 21:19, Etsuro Fujita wrote: > >> Comm

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-18 Thread Etsuro Fujita
On 2016/02/12 21:19, Etsuro Fujita wrote: Comments on specific points follow. This seems to need minor rebasing in the wake of the join pushdown patch. Will do. Done. + /* Likewise for ForeignScan that has pushed down INSERT/UPDATE/DELETE */ + if (IsA(plan, ForeignScan) && +

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-15 Thread Etsuro Fujita
On 2016/02/15 15:20, Rushabh Lathia wrote: On Fri, Feb 12, 2016 at 5:40 PM, Etsuro Fujita mailto:fujita.ets...@lab.ntt.co.jp>> wrote: As a result of our discussions, we reached a conclusion that the DML pushdown APIs should be provided together with existing APIs such as ExecForeig

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-14 Thread Rushabh Lathia
On Fri, Feb 12, 2016 at 5:40 PM, Etsuro Fujita wrote: > Hi Rushabh and Thom, > > Thanks for the review! > > On 2016/02/10 22:37, Thom Brown wrote: > >> On 10 February 2016 at 08:00, Rushabh Lathia >> wrote: >> >>> Fujita-san, I am attaching update version of the patch, which added >>> the docume

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-14 Thread Etsuro Fujita
On 2016/02/12 21:46, Robert Haas wrote: On Fri, Feb 12, 2016 at 7:19 AM, Etsuro Fujita wrote: I think that displaying target lists would be confusing for users. Here is an example: EXPLAIN (verbose, costs off) DELETE FROM rem1; -- can be pushed down QUERY PLA

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-12 Thread Robert Haas
On Fri, Feb 12, 2016 at 7:19 AM, Etsuro Fujita wrote: > I think that displaying target lists would be confusing for users. Here is > an example: > > EXPLAIN (verbose, costs off) > DELETE FROM rem1; -- can be pushed down > QUERY PLAN > -

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-12 Thread Etsuro Fujita
Hi Robert, Thanks for the review! On 2016/02/11 5:43, Robert Haas wrote: On Wed, Feb 10, 2016 at 3:00 AM, Rushabh Lathia wrote: Fujita-san, I am attaching update version of the patch, which added the documentation update. Once we finalize this, I feel good with the patch and think that we co

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-12 Thread Etsuro Fujita
Hi Rushabh and Thom, Thanks for the review! On 2016/02/10 22:37, Thom Brown wrote: On 10 February 2016 at 08:00, Rushabh Lathia wrote: Fujita-san, I am attaching update version of the patch, which added the documentation update. Thanks for updating the patch! Once we finalize this, I feel

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-10 Thread Robert Haas
On Wed, Feb 10, 2016 at 3:00 AM, Rushabh Lathia wrote: > Fujita-san, I am attaching update version of the patch, which added > the documentation update. > > Once we finalize this, I feel good with the patch and think that we > could mark this as ready for committer. It would be nice to hear from

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-10 Thread Thom Brown
On 10 February 2016 at 08:00, Rushabh Lathia wrote: > Fujita-san, I am attaching update version of the patch, which added > the documentation update. > > Once we finalize this, I feel good with the patch and think that we > could mark this as ready for committer. I find this wording a bit confusi

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-10 Thread Rushabh Lathia
Fujita-san, I am attaching update version of the patch, which added the documentation update. Once we finalize this, I feel good with the patch and think that we could mark this as ready for committer. On Fri, Feb 5, 2016 at 5:33 PM, Rushabh Lathia wrote: > > > On Fri, Feb 5, 2016 at 4:46 PM, R

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-05 Thread Rushabh Lathia
On Fri, Feb 5, 2016 at 4:46 PM, Rushabh Lathia wrote: > > > On Wed, Feb 3, 2016 at 3:31 PM, Etsuro Fujita > wrote: > >> On 2016/01/28 15:20, Rushabh Lathia wrote: >> >>> On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita >>> mailto:fujita.ets...@lab.ntt.co.jp>> >>> wrote: >>> >>> On 2016/01/27

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-05 Thread Rushabh Lathia
On Wed, Feb 3, 2016 at 3:31 PM, Etsuro Fujita wrote: > On 2016/01/28 15:20, Rushabh Lathia wrote: > >> On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita >> mailto:fujita.ets...@lab.ntt.co.jp>> wrote: >> >> On 2016/01/27 21:23, Rushabh Lathia wrote: >> >> If I understood correctly, above

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-02-04 Thread Etsuro Fujita
On 2016/02/05 12:28, Robert Haas wrote: On Thu, Jan 28, 2016 at 7:36 AM, Etsuro Fujita wrote: Attached is that version of the patch. I think that postgres_fdw might be able to insert a tableoid value in the returned slot in e.g., postgresExecForeignInsert if AFTER ROW Triggers or RETURNING exp

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-04 Thread Etsuro Fujita
On 2016/02/04 19:44, Rushabh Lathia wrote: I just started reviewing this and realized that patch is not getting applied cleanly on latest source, it having some conflicts. Can you please upload the correct version of patch. I rebased the patch to the latest HEAD. Attached is a rebased version

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-02-04 Thread Robert Haas
On Thu, Jan 28, 2016 at 7:36 AM, Etsuro Fujita wrote: > Attached is that version of the patch. > > I think that postgres_fdw might be able to insert a tableoid value in the > returned slot in e.g., postgresExecForeignInsert if AFTER ROW Triggers or > RETURNING expressions reference that value, but

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-04 Thread Rushabh Lathia
On Wed, Feb 3, 2016 at 3:31 PM, Etsuro Fujita wrote: > On 2016/01/28 15:20, Rushabh Lathia wrote: > >> On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita >> mailto:fujita.ets...@lab.ntt.co.jp>> wrote: >> >> On 2016/01/27 21:23, Rushabh Lathia wrote: >> >> If I understood correctly, above

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-02-03 Thread Etsuro Fujita
On 2016/01/28 15:20, Rushabh Lathia wrote: On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita mailto:fujita.ets...@lab.ntt.co.jp>> wrote: On 2016/01/27 21:23, Rushabh Lathia wrote: If I understood correctly, above documentation means, that if FDW have DMLPushdown APIs t

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-28 Thread Etsuro Fujita
On 2016/01/28 12:58, Robert Haas wrote: On Thu, Jan 21, 2016 at 4:05 AM, Etsuro Fujita wrote: By the way, I'm not too sure I understand the need for the core changes that are part of this patch, and I think that point merits some discussion. Whenever you change core like this, you're changing

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-27 Thread Etsuro Fujita
On 2016/01/28 15:20, Rushabh Lathia wrote: On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita mailto:fujita.ets...@lab.ntt.co.jp>> wrote: On 2016/01/27 21:23, Rushabh Lathia wrote: If I understood correctly, above documentation means, that if FDW have DMLPushdown APIs t

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-27 Thread Rushabh Lathia
On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita wrote: > On 2016/01/27 21:23, Rushabh Lathia wrote: > >> If I understood correctly, above documentation means, that if FDW have >> DMLPushdown APIs that is enough. But in reality thats not the case, we >> need ExecForeignInsert, ExecForeignUpdate,

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-27 Thread Etsuro Fujita
On 2016/01/27 21:23, Rushabh Lathia wrote: If I understood correctly, above documentation means, that if FDW have DMLPushdown APIs that is enough. But in reality thats not the case, we need ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete in case DML is not pushable. And here fact is

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-27 Thread Robert Haas
On Thu, Jan 21, 2016 at 4:05 AM, Etsuro Fujita wrote: >> By the way, I'm not too sure I understand the need for the core >> changes that are part of this patch, and I think that point merits >> some discussion. Whenever you change core like this, you're changing >> the contract between the FDW an

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-27 Thread Rushabh Lathia
On Wed, Jan 27, 2016 at 2:50 PM, Etsuro Fujita wrote: > On 2016/01/27 12:20, Etsuro Fujita wrote: > >> On 2016/01/26 22:57, Rushabh Lathia wrote: >> >>> On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita >>> mailto:fujita.ets...@lab.ntt.co.jp>> >>> wrote: >>> >>> On 2016/01/25 17:03, Rushabh Lath

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-27 Thread Etsuro Fujita
On 2016/01/27 12:20, Etsuro Fujita wrote: On 2016/01/26 22:57, Rushabh Lathia wrote: On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita mailto:fujita.ets...@lab.ntt.co.jp>> wrote: On 2016/01/25 17:03, Rushabh Lathia wrote: int IsForeignRelUpdatable (Relation rel);

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Etsuro Fujita
On 2016/01/26 22:57, Rushabh Lathia wrote: On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita mailto:fujita.ets...@lab.ntt.co.jp>> wrote: On 2016/01/25 17:03, Rushabh Lathia wrote: int IsForeignRelUpdatable (Relation rel); Documentation for IsForeignUpdatable() need

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Rushabh Lathia
On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita wrote: > On 2016/01/25 17:03, Rushabh Lathia wrote: > >> Here are couple of comments: >> > > 1) >> >> int >> IsForeignRelUpdatable (Relation rel); >> > > Documentation for IsForeignUpdatable() need to change as it says: >> >> If the IsForeignRelUpdat

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Etsuro Fujita
On 2016/01/25 17:03, Rushabh Lathia wrote: Here are couple of comments: 1) int IsForeignRelUpdatable (Relation rel); Documentation for IsForeignUpdatable() need to change as it says: If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are assumed to be insertable, updatabl

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-25 Thread Rushabh Lathia
On Thu, Jan 21, 2016 at 3:20 PM, Etsuro Fujita wrote: > On 2016/01/20 19:57, Rushabh Lathia wrote: > >> Overall I am quite done with the review of this patch. Patch is in good >> shape and covered most of the things which been discussed earlier >> or been mentioned during review process. Patch pa

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-21 Thread Etsuro Fujita
On 2016/01/19 19:04, Thom Brown wrote: On 12 January 2016 at 11:49, Etsuro Fujita wrote: On 2016/01/12 20:36, Thom Brown wrote: On 8 January 2016 at 05:08, Etsuro Fujita wrote: On 2016/01/06 20:37, Thom Brown wrote: I've run into an issue: *# UPDATE master_customers SET id = 22 WHERE

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-21 Thread Etsuro Fujita
On 2016/01/20 19:57, Rushabh Lathia wrote: Overall I am quite done with the review of this patch. Patch is in good shape and covered most of the things which been discussed earlier or been mentioned during review process. Patch pass through the make check and also includes good test coverage. T

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-21 Thread Etsuro Fujita
On 2016/01/21 5:06, Robert Haas wrote: On Wed, Jan 20, 2016 at 4:50 AM, Etsuro Fujita wrote: My concern about that is that would make the code in deparseTargetList() complicated. Essentially, I think your propossal needs a two-pass algorithm for deparseTargetList; (1) create an integer List of

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 4:50 AM, Etsuro Fujita wrote: > My concern about that is that would make the code in deparseTargetList() > complicated. > > Essentially, I think your propossal needs a two-pass algorithm for > deparseTargetList; (1) create an integer List of the columns being retrieved > fr

  1   2   >