Re: Another WaitEventSet resource leakage in back branches

2024-04-12 Thread Etsuro Fujita
Hi Andres, On Fri, Apr 12, 2024 at 1:29 AM Andres Freund wrote: > On 2024-03-22 21:15:45 +0900, Etsuro Fujita wrote: > > While working on [1], I noticed $SUBJECT: WaitLatchOrSocket in back > > branches is ignoring the possibility of failing partway through, too. > > I adde

Re: Comment about handling of asynchronous requests in postgres_fdw.c

2024-04-11 Thread Etsuro Fujita
On Wed, Apr 10, 2024 at 8:51 PM Etsuro Fujita wrote: > We updated $SUBJECT in back branches to make it clear (see commit > f6f61a4bd), so I would like to propose to do so in HEAD as well for > consistency. Attached is a patch for that. Pushed. Best regards, Etsuro Fujita

Re: Another WaitEventSet resource leakage in back branches

2024-04-11 Thread Etsuro Fujita
On Fri, Apr 5, 2024 at 7:55 PM Etsuro Fujita wrote: > I am planning to back-patch these next week. Done. Best regards, Etsuro Fujita

Comment about handling of asynchronous requests in postgres_fdw.c

2024-04-10 Thread Etsuro Fujita
Hi, We updated $SUBJECT in back branches to make it clear (see commit f6f61a4bd), so I would like to propose to do so in HEAD as well for consistency. Attached is a patch for that. Best regards, Etsuro Fujita postgres-fdw-comment.patch Description: Binary data

Re: postgres_fdw fails because GMT != UTC

2024-04-10 Thread Etsuro Fujita
nk the first is desirable for reasons of general sanity, and the > second for best compatibility with old versions. > > So I vote for "both". +1 for both (assuming that the latter does not make the postgres_fdw code complicated). Best regards, Etsuro Fujita

Re: Another WaitEventSet resource leakage in back branches

2024-04-05 Thread Etsuro Fujita
On Fri, Mar 22, 2024 at 9:15 PM Etsuro Fujita wrote: > While working on [1], I noticed $SUBJECT: WaitLatchOrSocket in back > branches is ignoring the possibility of failing partway through, too. > I added a PG_FAINALLY block to that function, like commit 555276f85. > Patch attached

Re: postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()

2024-04-04 Thread Etsuro Fujita
On Fri, Mar 22, 2024 at 9:30 PM Etsuro Fujita wrote: > If there are no objections, I will apply the patch to HEAD only. Done. Best regards, Etsuro Fujita

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-04-04 Thread Etsuro Fujita
On Fri, Mar 22, 2024 at 9:09 PM Etsuro Fujita wrote: > On Fri, Mar 22, 2024 at 7:23 PM Alexander Pyhalov > wrote: > > The updated patch still looks good to me. > > I am planning to apply the patch to the back branches next week. Pushed. Sorry for the delay. Best regards, Etsuro Fujita

postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()

2024-03-22 Thread Etsuro Fujita
an assertion ensuring that the query string is non-NULL. (I added the assertion to pgfdw_exec_cleanup_query_begin() as well.) Attached is a patch for that. If there are no objections, I will apply the patch to HEAD only. Best regards, Etsuro Fujita [1] https://www.postgresql.org/message-id

Another WaitEventSet resource leakage in back branches

2024-03-22 Thread Etsuro Fujita
Hi, While working on [1], I noticed $SUBJECT: WaitLatchOrSocket in back branches is ignoring the possibility of failing partway through, too. I added a PG_FAINALLY block to that function, like commit 555276f85. Patch attached. Best regards, Etsuro Fujita [1] https://www.postgresql.org/message

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-03-22 Thread Etsuro Fujita
Hi Alexander, On Fri, Mar 22, 2024 at 7:23 PM Alexander Pyhalov wrote: > The updated patch still looks good to me. Great! I am planning to apply the patch to the back branches next week. Thanks for the review! Best regards, Etsuro Fujita

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-03-21 Thread Etsuro Fujita
On Sun, Feb 25, 2024 at 6:34 PM Etsuro Fujita wrote: > > On Fri, Feb 23, 2024 at 01:21:14PM +0300, Alexander Pyhalov wrote: > > > Recent commit 555276f8594087ba15e0d58e38cd2186b9f39f6d introduced final > > > cleanup of node->as_eventset in ExecAppendAsyncEventWait(

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-03-11 Thread Etsuro Fujita
Hi Michael-san, On Mon, Mar 11, 2024 at 8:12 AM Michael Paquier wrote: > On Mon, Feb 26, 2024 at 04:29:44PM +0900, Etsuro Fujita wrote: > > Will do. (I was thinking you would get busy from now on.) > > Fujita-san, have you been able to look at this thread? Yeah, I took a look;

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-02-25 Thread Etsuro Fujita
resource leaks in my commit 27e1f1456. > Anyway, if you want to address it yourself, feel free to go ahead, > thanks! I would have done it but I've been busy with life stuff for > the last couple of days. Will do. (I was thinking you would get busy from now on.) Best regards, Etsuro Fujita

Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack

2024-02-25 Thread Etsuro Fujita
I'll look into fixing that where appropriate. Thanks for taking care of this, Michael-san! This would result originally from my fault, so If you don't mind, could you let me do that? Best regards, Etsuro Fujita

Re: Incorrect file reference in comment in procarray.c

2023-11-13 Thread Etsuro Fujita
On Fri, Nov 3, 2023 at 6:53 PM Etsuro Fujita wrote: > On Thu, Nov 2, 2023 at 10:20 PM Daniel Gustafsson wrote: > > > On 2 Nov 2023, at 13:40, Etsuro Fujita wrote: > > > Attached is a small patch for that: s/heapam_visibility.c/snapmgr.c/. > > > > No ob

Re: Incorrect file reference in comment in procarray.c

2023-11-03 Thread Etsuro Fujita
On Thu, Nov 2, 2023 at 10:20 PM Daniel Gustafsson wrote: > > On 2 Nov 2023, at 13:40, Etsuro Fujita wrote: > > Attached is a small patch for that: s/heapam_visibility.c/snapmgr.c/. > > No objections to the patch, the change is correct. However, with git grep and >

Incorrect file reference in comment in procarray.c

2023-11-02 Thread Etsuro Fujita
/snapmgr.c/. Best regards, Etsuro Fujita fix-file-reference-in-comment.patch Description: Binary data

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-23 Thread Etsuro Fujita
eturn a > negative value, which may occur on some production servers. > > Fix by changing the Assertion into a real test, to protect the > simple_rel_array array. Thanks for the report and patch! I will review the patch. Best regards, Etsuro Fujita

Re: Comment about set_join_pathlist_hook()

2023-09-21 Thread Etsuro Fujita
Hi, On Wed, Sep 20, 2023 at 7:49 PM David Rowley wrote: > On Wed, 20 Sept 2023 at 22:06, Etsuro Fujita wrote: > > So I would like to propose to extend the comment to explain what they > > can do, as in the comment about set_rel_pathlist_hook() in allpaths.c. > >

Re: Comment about set_join_pathlist_hook()

2023-09-20 Thread Etsuro Fujita
Hi, On Thu, Sep 21, 2023 at 11:49 AM Lepikhov Andrei wrote: > On Wed, Sep 20, 2023, at 5:05 PM, Etsuro Fujita wrote: > > What I am concerned about from the report [1] is that this comment is > > a bit too terse; it might cause a misunderstanding that extensions can > > do

Comment about set_join_pathlist_hook()

2023-09-20 Thread Etsuro Fujita
for that. Best regards, Etsuro Fujita [1] https://www.postgresql.org/message-id/CACawEhV%3D%2BQ0HXrcDergbTR9EkVFukgRPMTZbRFL-YK5CRmvYag%40mail.gmail.com update-set_join_pathlist_hook-comment.patch Description: Binary data

Re: Missing comments/docs about custom scan path

2023-08-30 Thread Etsuro Fujita
Hi Richard, On Wed, Aug 30, 2023 at 11:05 AM Richard Guo wrote: > On Tue, Aug 29, 2023 at 5:08 PM Etsuro Fujita wrote: >> Another thing I would like to propose is minor adjustments to the docs >> related to parallel query: >> >> A custom scan provider will typ

Re: Test case for parameterized remote path in postgres_fdw

2023-08-30 Thread Etsuro Fujita
On Wed, Aug 16, 2023 at 6:45 PM Etsuro Fujita wrote: > On Wed, Aug 16, 2023 at 9:41 AM Richard Guo wrote: > > On Tue, Aug 15, 2023 at 7:50 PM Etsuro Fujita > > wrote: > >> So we should have modified the second one as well? Attached is a > >> small patch for th

Re: Missing comments/docs about custom scan path

2023-08-29 Thread Etsuro Fujita
On Thu, Aug 3, 2023 at 6:01 PM Etsuro Fujita wrote: > On Mon, Jul 31, 2023 at 7:05 PM Etsuro Fujita wrote: > > While working on [1], I noticed $SUBJECT: Another thing I would like to propose is minor adjustments to the docs related to parallel query: A custom scan provider will

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-29 Thread Etsuro Fujita
Reverting the commit would resolve your issue, but re-introduce the issue mentioned upthread to extensions that use the hook properly, so I do not think that reverting the commit would be a fair action. Sorry for the delay. Best regards, Etsuro Fujita

Re: list of acknowledgments for PG16

2023-08-23 Thread Etsuro Fujita
list. I think they are all in the right order (ie, the given-name-followed-by-surname order). Thanks! Best regards, Etsuro Fujita

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-21 Thread Etsuro Fujita
Sorry, I hit the send button by mistake. On Sun, Aug 20, 2023 at 4:34 AM Andres Freund wrote: > On 2023-08-19 20:09:25 +0900, Etsuro Fujita wrote: > > * The problem we had with the set_join_pathlist_hook hook is that in > > such a typical use case, previously, if the replace

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-21 Thread Etsuro Fujita
Hi, On Sun, Aug 20, 2023 at 4:34 AM Andres Freund wrote: > > Hi, > > On 2023-08-19 20:09:25 +0900, Etsuro Fujita wrote: > > Maybe my explanation was not enough, so let me explain: > > > > * I think you could use the set_join_pathlist_hook hook as you like

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-19 Thread Etsuro Fujita
uess are the majority of the hook extensions, need not be modified/recompiled. I think it is unfortunate that that breaks the use case of the Citus extension, though. BTW: commit 9e9931d2b removed the restriction on the call to the hook extensions, so you might want to back-patch it. Though, I think it would be better if the hook was well implemented from the beginning. Best regards, Etsuro Fujita

Re: Test case for parameterized remote path in postgres_fdw

2023-08-16 Thread Etsuro Fujita
Hi Richard, On Wed, Aug 16, 2023 at 9:41 AM Richard Guo wrote: > On Tue, Aug 15, 2023 at 7:50 PM Etsuro Fujita wrote: >> So we should have modified the second one as well? Attached is a >> small patch for that. > Agreed, nice catch! +1 to the patch. Thanks for looking! Be

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-16 Thread Etsuro Fujita
iliar with the Citus extension, but such pseudoconstant clauses are handled within the Citus extension? Thanks for the report! Best regards, Etsuro Fujita

Test case for parameterized remote path in postgres_fdw

2023-08-15 Thread Etsuro Fujita
c6, r1.c7, r1.c8, r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8 FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1.c2 = r2."C 1")) AND ((r1."C 1" = 47 (4 rows) So we should have modified the second one as

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-15 Thread Etsuro Fujita
On Tue, Aug 8, 2023 at 6:30 PM Richard Guo wrote: > On Tue, Aug 8, 2023 at 4:40 PM Etsuro Fujita wrote: >> I modified the code a bit further to use an if-test to avoid a useless >> function call, and added/tweaked comments and docs further. Attached >> is a new versio

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-08-08 Thread Etsuro Fujita
Hi Richard, On Mon, Jul 31, 2023 at 5:52 PM Richard Guo wrote: > On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita wrote: >> here is a rebased version of the second patch, in which I modified the >> ForeignPath and CustomPath cases in reparameterize_path_by_child() to >> re

Re: Missing comments/docs about custom scan path

2023-08-03 Thread Etsuro Fujita
On Mon, Jul 31, 2023 at 7:05 PM Etsuro Fujita wrote: > While working on [1], I noticed $SUBJECT: commit e7cb7ee14 failed to > update comments for the CustomPath struct in pathnodes.h, and commit > f49842d1e failed to update docs about custom scan path callbacks in > custom-sca

Missing comments/docs about custom scan path

2023-07-31 Thread Etsuro Fujita
separately for ease of review (patch update-custom-scan-path-comments.patch for the former and patch update-custom-scan-path-docs.patch for the latter). In the second patch I used almost the same text as for the ReparameterizeForeignPathByChild callback function in fdwhandler.sgml. Best regards, Etsuro

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-07-28 Thread Etsuro Fujita
Hi Richard, On Mon, Jul 24, 2023 at 11:45 AM Richard Guo wrote: > On Fri, Jul 21, 2023 at 8:51 PM Etsuro Fujita wrote: >> * In this bit I changed the last argument to NIL, which would be >> nitpicking, though. >> >> @@ -1038,7 +1038,7 @@ postgresGetForeignPaths(Plann

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-07-21 Thread Etsuro Fujita
Hi Richard, On Sun, Jun 25, 2023 at 3:05 PM Richard Guo wrote: > On Wed, Jun 14, 2023 at 2:49 PM Etsuro Fujita wrote: >> On Mon, Jun 5, 2023 at 10:19 PM Etsuro Fujita >> wrote: >> > To avoid this issue, I am wondering if we should modify >> > add_paths

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-06-14 Thread Etsuro Fujita
On Mon, Jun 5, 2023 at 10:19 PM Etsuro Fujita wrote: > To avoid this issue, I am wondering if we should modify > add_paths_to_joinrel() in back branches so that it just disallows the > FDW to consider pushing down joins when the restrictlist has > pseudoconstant clauses. Attache

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-06-08 Thread Etsuro Fujita
> type of query execution won't make any difference. No comparison of plans to > be selected based on total cost of two plans old (Nested Loop with Foreign > Scans) & new (Only Foreign Scan) will be done, because we are avoiding the > call to "postgresGetForeignJoinPaths()" up front when we have pseudo > constants. Thanks for looking! Best regards, Etsuro Fujita

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-06-08 Thread Etsuro Fujita
Hi Richard, On Tue, Jun 6, 2023 at 12:20 PM Richard Guo wrote: > On Mon, Jun 5, 2023 at 9:19 PM Etsuro Fujita wrote: >> To avoid this issue, I am wondering if we should modify >> add_paths_to_joinrel() in back branches so that it just disallows the >> FDW to consider pu

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-06-05 Thread Etsuro Fujita
down joins when the restrictlist has pseudoconstant clauses. Attached is a patch for that. My apologies for not reviewing your patch and the long long delay. Best regards, Etsuro Fujita disallow-join-pushdown-if-pseudoconstants.patch Description: Binary data

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-04-24 Thread Etsuro Fujita
On Mon, Apr 24, 2023 at 3:31 PM Nishant Sharma wrote: > Any updates? -- did you get a chance to look into this? Sorry, I have not looked into this yet, because I have been busy with some other work recently. I plan to do so early next week. Best regards, Etsuro Fujita

Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply

2023-04-17 Thread Etsuro Fujita
On Fri, Apr 14, 2023 at 11:28 PM Fujii Masao wrote: > On 2023/04/14 18:59, Etsuro Fujita wrote: > >> The primary message basically should avoid reference to implementation > >> details such as specific structure names like PGcancel, shouldn't it, as > >> per

Re: postgres_fdw: wrong results with self join + enable_nestloop off

2023-04-14 Thread Etsuro Fujita
ause for this issue would be in the create_scan_plan handling of pseudoconstant quals when creating a foreign-join (or custom-join) plan. Anyway, I will look at your patch closely, first. Best regards, Etsuro Fujita

Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply

2023-04-14 Thread Etsuro Fujita
On Fri, Apr 14, 2023 at 3:19 AM Fujii Masao wrote: > On 2023/04/13 15:13, Etsuro Fujita wrote: > > I am not 100% sure that it is a good idea to use the same error > > message "could not send cancel request" for the PQgetCancel() and > > PQcancel() cases, because the

Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply

2023-04-13 Thread Etsuro Fujita
tructure” or something like that, for the former case, so we can distinguish the former error from the latter? Best regards, Etsuro Fujita

Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

2023-04-06 Thread Etsuro Fujita
On Tue, Apr 4, 2023 at 7:28 PM Etsuro Fujita wrote: > The parallel-abort patch received a review from David, and I addressed > his comments. Also, he tested with the patch, and showed that it > reduces time taken to abort remote transactions. So, if there are no > objections, I

Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

2023-04-04 Thread Etsuro Fujita
On Wed, Jan 18, 2023 at 8:06 PM Etsuro Fujita wrote: > I rebased the patch. Attached is an updated patch. The parallel-abort patch received a review from David, and I addressed his comments. Also, he tested with the patch, and showed that it reduces time taken to abort remote transactions.

Re: Commitfest 2023-03 starting tomorrow!

2023-03-25 Thread Etsuro Fujita
one wants to work on this, please do so; if not, I want to in the next development cycle for v17. Best regards, Etsuro Fujita

Re: Doc: Improve note about copying into postgres_fdw foreign tables in batch

2023-03-23 Thread Etsuro Fujita
of the patch. > > LGTM. Cool! Pushed. Thanks again! Best regards, Etsuro Fujita

Re: Doc: Improve note about copying into postgres_fdw foreign tables in batch

2023-03-23 Thread Etsuro Fujita
seems more appropriate to me as well in this context, so I left it alone. Attached is an updated version of the patch. Thanks for looking, Daniel and Ishii-san! Best regards, Etsuro Fujita postgres-fdw-batch-insert-doc-v2.patch Description: Binary data

Re: Doc: Improve note about copying into postgres_fdw foreign tables in batch

2023-03-22 Thread Etsuro Fujita
On Fri, Feb 17, 2023 at 5:45 PM Etsuro Fujita wrote: > Here is a small patch to improve the note, which was added by commit > 97da48246 ("Allow batch insertion during COPY into a foreign table."), > by adding an explanation about how the actual number of rows > postgr

Re: Comment in preptlist.c

2023-03-22 Thread Etsuro Fujita
On Wed, Mar 22, 2023 at 4:50 PM David Rowley wrote: > And now it just clicked with me why Tom left this. Sorry for stepping > on your toes here. No problem at all. Best regards, Etsuro Fujita

Re: Comment in preptlist.c

2023-03-22 Thread Etsuro Fujita
On Wed, Mar 22, 2023 at 4:59 AM David Rowley wrote: > On Tue, 21 Mar 2023 at 22:41, Etsuro Fujita wrote: > > I think that “planner/rewriter” should be parser/rewriter. Attached > > is a patch for that. > > Pushed. Thanks for picking this up, David! Thanks for looking, T

Comment in preptlist.c

2023-03-21 Thread Etsuro Fujita
a heap row. (The planner * adds these; they're not in what we receive from the planner/rewriter.) I think that “planner/rewriter” should be parser/rewriter. Attached is a patch for that. Best regards, Etsuro Fujita fix-comment.patch Description: Binary data

Re: postgres_fdw: Useless if-test in GetConnection()

2023-03-17 Thread Etsuro Fujita
On Wed, Mar 15, 2023 at 7:18 PM Etsuro Fujita wrote: > This would be harmless, so I am planning to apply the patch to HEAD only. I forgot to mention that this was added in v14. Done that way. Best regards, Etsuro Fujita

Re: postgres_fdw: Useless if-test in GetConnection()

2023-03-16 Thread Etsuro Fujita
On Wed, Mar 15, 2023 at 7:58 PM Daniel Gustafsson wrote: > > On 15 Mar 2023, at 11:18, Etsuro Fujita wrote: > > While working on something else, I noticed that the “if (entry->conn > > == NULL)” test after doing disconnect_pg_server() when re-establishing > > a given

Re: postgres_fdw: Useless if-test in GetConnection()

2023-03-16 Thread Etsuro Fujita
On Wed, Mar 15, 2023 at 7:40 PM Richard Guo wrote: > On Wed, Mar 15, 2023 at 6:18 PM Etsuro Fujita wrote: >> While working on something else, I noticed that the “if (entry->conn >> == NULL)” test after doing disconnect_pg_server() when re-establishing >> a given con

postgres_fdw: Useless if-test in GetConnection()

2023-03-15 Thread Etsuro Fujita
tached is a patch for that. I think we could instead add an assertion, but I did not, because we already have it in make_new_connection(). This would be harmless, so I am planning to apply the patch to HEAD only. Best regards, Etsuro Fujita remove-useless-if-test.patch Description: Binary data

Doc: Improve note about copying into postgres_fdw foreign tables in batch

2023-02-17 Thread Etsuro Fujita
limitation that does not apply to the INSERT case. I will add this to the next CF. Best regards, Etsuro Fujita postgres-fdw-batch-insert-doc.patch Description: Binary data

Re: Some revises in adding sorting path

2023-02-16 Thread Etsuro Fujita
ge-join path of a higher-level foreign join, so shouldn't we keep this step as much as simple and save cycles even a little? Sorry for being late to the party. Best regards, Etsuro Fujita

Re: Refactoring postgres_fdw/connection.c

2023-02-15 Thread Etsuro Fujita
On Tue, Jan 31, 2023 at 3:44 PM Fujii Masao wrote: > On 2023/01/29 19:31, Etsuro Fujita wrote: > > I agree that if the name of an existing function was bad, we should > > rename it, but I do not think the name pgfdw_get_cleanup_result is > > bad; I think it is good in t

Re: Refactoring postgres_fdw/connection.c

2023-01-29 Thread Etsuro Fujita
Hi Fujii-san, On Thu, Sep 15, 2022 at 12:17 AM Fujii Masao wrote: > On 2022/09/05 15:17, Etsuro Fujita wrote: > > I'm not sure it's a good idea to change the function's name, because > > that would make backpatching hard. To avoid that, how about > > introducing

Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

2023-01-18 Thread Etsuro Fujita
Hi Vignesh, On Wed, Jan 4, 2023 at 9:19 PM vignesh C wrote: > On Tue, 1 Nov 2022 at 15:54, Etsuro Fujita wrote: > > Attached is a rebased version of the patch. > > The patch does not apply on top of HEAD as in [1], please post a rebased > patch: I rebased the patch. Atta

Re: Allow batched insert during cross-partition updates

2022-12-20 Thread Etsuro Fujita
Hi Amit-san, On Wed, Dec 14, 2022 at 10:29 PM Amit Langote wrote: > On Wed, Dec 14, 2022 at 6:45 PM Etsuro Fujita wrote: > > One thing I noticed is this bit: > > > > -- Clean up > > -DROP TABLE batch_table, batch_cp_upd_test, batch_table_p0, > > batch_

Re: Allow batched insert during cross-partition updates

2022-12-14 Thread Etsuro Fujita
On Thu, Dec 8, 2022 at 8:01 PM Etsuro Fujita wrote: > On Thu, Dec 8, 2022 at 5:00 PM Amit Langote wrote: > > Updated patch attached. > > I will review the patch a bit more, but I think > it would be committable. One thing I noticed is this bit: -- Clean up -DROP

Re: Allow batched insert during cross-partition updates

2022-12-08 Thread Etsuro Fujita
Amit-san, On Thu, Dec 8, 2022 at 5:00 PM Amit Langote wrote: > On Tue, Dec 6, 2022 at 6:47 PM Etsuro Fujita wrote: > > * In postgresGetForeignModifyBatchSize(): > > > > /* > > -* Should never get called when the insert is being performed as part >

Re: postgres_fdw: batch inserts vs. before row triggers

2022-12-07 Thread Etsuro Fujita
On Fri, Dec 2, 2022 at 4:54 PM Etsuro Fujita wrote: > On Sun, Nov 27, 2022 at 12:11 AM Tom Lane wrote: > > OK, as long as there's a reason for doing it that way, it's OK > > by me. I don't think that adding a field at the end of EState > > is an ABI problem. > > >

Re: Allow batched insert during cross-partition updates

2022-12-06 Thread Etsuro Fujita
means that any fmstate should have fmstate->aux_fmstate=NULL. * Also in that function: - if (fmstate) + if (fmstate != NULL) This is correct, but I would vote for leaving that as-is, to make back-patching easy. That is all I have for now. I will mark this as Waiting on Author. Best

Re: postgres_fdw: batch inserts vs. before row triggers

2022-12-01 Thread Etsuro Fujita
On Sun, Nov 27, 2022 at 12:11 AM Tom Lane wrote: > Etsuro Fujita writes: > > On Sat, Nov 26, 2022 at 1:57 AM Tom Lane wrote: > >> Couldn't we add the field to ModifyTableState, instead? > > > We could probably do so, but I thought having a global list would be &

Re: postgres_fdw: batch inserts vs. before row triggers

2022-11-26 Thread Etsuro Fujita
ving a global list would be more efficient to handle pending buffered inserts than that. Anyway I will work on this further. Thanks for looking at this! Best regards, Etsuro Fujita

Re: postgres_fdw: batch inserts vs. before row triggers

2022-11-25 Thread Etsuro Fujita
On Thu, Nov 24, 2022 at 8:19 PM Etsuro Fujita wrote: > Here is an updated patch. In the attached, I added an assertion to > ExecInsert(). Also, I tweaked comments and test cases a little bit, > for consistency. Also, I noticed a copy-and-pasteo in a comment in > ExecBatchInsert()

Re: postgres_fdw: batch inserts vs. before row triggers

2022-11-24 Thread Etsuro Fujita
On Fri, Nov 18, 2022 at 8:46 PM Etsuro Fujita wrote: > Attached is a patch for fixing these issues. Here is an updated patch. In the attached, I added an assertion to ExecInsert(). Also, I tweaked comments and test cases a little bit, for consistency. Also, I noticed a copy-and-pas

Re: postgres_fdw: batch inserts vs. before row triggers

2022-11-18 Thread Etsuro Fujita
g inserts to the foreign tables. Best regards, Etsuro Fujita fix-handling-of-pending-inserts.patch Description: Binary data

Re: Error for row-level triggers with transition tables on partitioned tables

2022-11-04 Thread Etsuro Fujita
On Thu, Nov 3, 2022 at 2:20 AM Tom Lane wrote: > Etsuro Fujita writes: > > We do not currently allow row-level triggers on partitions to have > > transition tables either, and the error message for that is “ROW > > triggers with transition tables are not supported on partit

Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

2022-11-01 Thread Etsuro Fujita
Hi David, On Sat, Oct 1, 2022 at 5:54 AM David Zhang wrote: > After rebase the file `postgres_fdw.out` and applied to master branch, > make and make check are all ok for postgres_fdw. Thanks for testing! Attached is a rebased version of the patch. Best regards, Etsuro Fujita v10-po

Error for row-level triggers with transition tables on partitioned tables

2022-10-31 Thread Etsuro Fujita
supported on partitions.”. How about changing the DETAIL message to something similar to this like “ROW triggers with transition tables are not supported on partitioned tables.”, to avoid confusion? Patch attached. Will add this to the upcoming CF. Best regards, Etsuro Fujita fix-trigger-error-mes

Re: Fast COPY FROM based on batch insert

2022-10-31 Thread Etsuro Fujita
On Fri, Oct 28, 2022 at 7:53 PM Andrey Lepikhov wrote: > On 28/10/2022 16:12, Etsuro Fujita wrote: > > I think there is another patch that improves performance of COPY FROM > > for foreign tables using COPY FROM STDIN, but if Andrey (or anyone > > else) want to work on it ag

Re: Fast COPY FROM based on batch insert

2022-10-28 Thread Etsuro Fujita
On Thu, Oct 13, 2022 at 6:58 PM Etsuro Fujita wrote: > I have committed the patch after tweaking comments a little bit further. I think there is another patch that improves performance of COPY FROM for foreign tables using COPY FROM STDIN, but if Andrey (or anyone else) want to work on it ag

Re: havingQual vs hasHavingQual buglets

2022-10-18 Thread Etsuro Fujita
gt; preprocessing. It should be the right one to check with. +1 HEAD only seems reasonable. Best regards, Etsuro Fujita

Re: Fast COPY FROM based on batch insert

2022-10-13 Thread Etsuro Fujita
On Thu, Oct 13, 2022 at 1:38 PM Andrey Lepikhov wrote: > On 10/12/22 07:56, Etsuro Fujita wrote: > > On Tue, Oct 11, 2022 at 3:06 PM Andrey Lepikhov > > wrote: > >> I reviewed the patch one more time. Only one question: bistate and > >> ri_FdwRoutine are st

Re: Fast COPY FROM based on batch insert

2022-10-11 Thread Etsuro Fujita
above. I think it is a good idea to add such assertions. How about adding them to CopyMultiInsertBufferFlush() and CopyMultiInsertBufferCleanup() like the attached? In the attached I updated comments a bit further as well. Thanks for reviewing! Best regards, Etsuro Fujita v4-0001-Imple

Re: Fast COPY FROM based on batch insert

2022-10-07 Thread Etsuro Fujita
On Tue, Sep 27, 2022 at 6:03 PM Etsuro Fujita wrote: > I will review the patch a bit more, but I feel that it is > in good shape. One thing I noticed is this bit added to CopyMultiInsertBufferFlush() to run triggers on the foreign table. + /* Run AFTER ROW INSERT tr

Re: Obsolete comment in ExecInsert()

2022-09-29 Thread Etsuro Fujita
Hi, On Wed, Sep 28, 2022 at 11:42 PM Tom Lane wrote: > Etsuro Fujita writes: > > I think the “or a tuple has come for a different relation than that > > for the accumulated tuples" part in the comment is a leftover from an > > earlier version of the patch [1].

Obsolete comment in ExecInsert()

2022-09-28 Thread Etsuro Fujita
hat for the accumulated tuples" part in the comment is a leftover from an earlier version of the patch [1]. As the code shows, we do not handle that case anymore, so I think we should remove that part from the comment. Attached is a patch for that. Best regards, Etsuro Fujita [1] https://w

Re: Fast COPY FROM based on batch insert

2022-09-27 Thread Etsuro Fujita
On Wed, Aug 10, 2022 at 5:30 PM Etsuro Fujita wrote: > On Wed, Aug 10, 2022 at 1:06 AM Zhihong Yu wrote: > > + /* If any rows were inserted, run AFTER ROW INSERT triggers. */ > > ... > > + for (i

Re: Fast COPY FROM based on batch insert

2022-09-27 Thread Etsuro Fujita
On Tue, Aug 23, 2022 at 2:58 PM Andrey Lepikhov wrote: > On 22/8/2022 11:44, Etsuro Fujita wrote: > > I think the latter is more consistent with the existing error context > > information when in CopyMultiInsertBufferFlush(). Actually, I thought > > this too, and I think

Re: Multi-insert related comment in CopyFrom()

2022-09-22 Thread Etsuro Fujita
On Wed, Sep 21, 2022 at 4:39 PM Etsuro Fujita wrote: > While working on the “Fast COPY FROM based on batch insert” patch, I > noticed this: > > else if (proute != NULL && resultRelInfo->ri_TrigDesc != NULL && > resultRelInfo-&

Multi-insert related comment in CopyFrom()

2022-09-21 Thread Etsuro Fujita
ent level insert triggers are on the same relation. */ insertMethod = CIM_SINGLE; } I think there is a thinko in the comment; “before” should be after. Patch attached. Best regards, Etsuro Fujita Fix-thinko-in-comment.patch Description: Binary data

Re: list of acknowledgments for PG15

2022-09-11 Thread Etsuro Fujita
ng order etc. (Note that the current standard is given name followed > by surname, independent of cultural origin.) Thanks as usual! I think these are Japanese names that are in the surname-followed-by-given-name order: Kamigishi Rei Kawamoto Masaya Okano Naoki Best regards, Etsuro Fujita

Re: Asynchronous execution support for Custom Scan

2022-09-06 Thread Etsuro Fujita
> The actual CustomScan that supports asynchronous execution will > start processing in CustomScanAsyncRequest, > configure to detect completion via file descriptor in > CustomScanAsyncConfigureWait, > and receive the result in CustomScanAsyncNotify. Ok, thanks! Best regards, Etsuro Fujita

Re: Asynchronous execution support for Custom Scan

2022-09-05 Thread Etsuro Fujita
mance. Right? Anyway, that version seems to be useful for testing that the proposed APIs works well. So I'll review the proposed patches with it. I'm not Fujii-san, though. :-) Best regards, Etsuro Fujita

Re: Refactoring postgres_fdw/connection.c

2022-09-05 Thread Etsuro Fujita
> That is, pgfdw_finish_pre_commit_cleanup() and > > pgfdw_finish_pre_subcommit_cleanup() > > are no longer necessary and 0003 patch removes them. > > It gives the same feeling with 0002. I have to agree with Horiguchi-san on this as well; the existing single-purpose functions are easy to understand, so I'd vote for leaving them alone. Sorry for being late to the party. Best regards, Etsuro Fujita

Re: struct Trigger definition in trigger.sgml

2022-09-02 Thread Etsuro Fujita
Hi Richard, On Thu, Sep 1, 2022 at 4:29 PM Richard Guo wrote: > On Thu, Sep 1, 2022 at 2:18 PM Etsuro Fujita wrote: >> While working on something else, I noticed that commit 487e9861d added >> a new field to struct Trigger, but failed to update $SUBJECT to match. >> Atta

struct Trigger definition in trigger.sgml

2022-09-01 Thread Etsuro Fujita
Hi, While working on something else, I noticed that commit 487e9861d added a new field to struct Trigger, but failed to update $SUBJECT to match. Attached is a small patch for that. Best regards, Etsuro Fujita doc-trigger-def.patch Description: Binary data

Re: Asynchronous execution support for Custom Scan

2022-08-26 Thread Etsuro Fujita
ure > API works and small enough for reviewing. Seems like a good idea. Thanks! Best regards, Etsuro Fujita

Re: Asynchronous execution support for Custom Scan

2022-08-22 Thread Etsuro Fujita
ple. Could you provide it? I think a simple example is better for ease of review. Best regards, Etsuro Fujita

Re: Fast COPY FROM based on batch insert

2022-08-22 Thread Etsuro Fujita
On Mon, Aug 15, 2022 at 2:29 PM Andrey Lepikhov wrote: > On 8/9/22 16:44, Etsuro Fujita wrote: > >>> -1 foo > >>> 1 bar > >>> \. > > ERROR: new row for relation "t1" violates check constraint "t1_f1positive" > > DETAIL: F

  1   2   3   4   5   6   7   8   >