Inherited UPDATE/DELETE vs async execution

Started by Etsuro Fujitaover 4 years ago11 messages

etsuro.fujita@gmail.com

over 4 years ago

1 attachment(s)

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

EXPLAIN (VERBOSE, COSTS OFF)
DELETE FROM async_pt;
QUERY PLAN
----------------------------------------------------------------
Delete on public.async_pt
Foreign Delete on public.async_p1 async_pt_1
Foreign Delete on public.async_p2 async_pt_2
Delete on public.async_p3 async_pt_3
-> Append
-> Async Foreign Delete on public.async_p1 async_pt_1
Remote SQL: DELETE FROM public.base_tbl1
-> Async Foreign Delete on public.async_p2 async_pt_2
Remote SQL: DELETE FROM public.base_tbl2
-> Seq Scan on public.async_p3 async_pt_3
Output: async_pt_3.tableoid, async_pt_3.ctid
(11 rows)

DELETE FROM async_pt;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost

The cause for this would be that direct-update plans are mistakenly
treated as async-capable ones, as shown in the EXPLAIN output. To
fix, I think we should modify postgresPlanDirectModify() so that it
clears the async-capable flag if it is set. Attached is a patch for
that. Maybe I am missing something, though.

Best regards,
Etsuro Fujita

Attachments:

fix-postgresPlanDirectModify.patchapplication/octet-stream; name=fix-postgresPlanDirectModify.patchDownload

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 8e1cc69508..49b2632295 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10127,6 +10127,60 @@ SELECT * FROM join_tbl ORDER BY a1;
 DELETE FROM join_tbl;
 RESET enable_mergejoin;
 RESET enable_hashjoin;
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+                                                QUERY PLAN                                                
+----------------------------------------------------------------------------------------------------------
+ Update on public.async_pt
+   Output: async_pt_1.a, async_pt_1.b, async_pt_1.c
+   Foreign Update on public.async_p1 async_pt_1
+   Foreign Update on public.async_p2 async_pt_2
+   Update on public.async_p3 async_pt_3
+   ->  Append
+         ->  Foreign Update on public.async_p1 async_pt_1
+               Remote SQL: UPDATE public.base_tbl1 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c
+         ->  Foreign Update on public.async_p2 async_pt_2
+               Remote SQL: UPDATE public.base_tbl2 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c
+         ->  Seq Scan on public.async_p3 async_pt_3
+               Output: (async_pt_3.c || async_pt_3.c), async_pt_3.tableoid, async_pt_3.ctid, NULL::record
+               Filter: (async_pt_3.b = 0)
+(13 rows)
+
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+  a   | b |    c     
+------+---+----------
+ 1000 | 0 | 00000000
+ 2000 | 0 | 00000000
+ 3000 | 0 | 00000000
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF)
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+                                        QUERY PLAN                                        
+------------------------------------------------------------------------------------------
+ Delete on public.async_pt
+   Output: async_pt_1.a, async_pt_1.b, async_pt_1.c
+   Foreign Delete on public.async_p1 async_pt_1
+   Foreign Delete on public.async_p2 async_pt_2
+   Delete on public.async_p3 async_pt_3
+   ->  Append
+         ->  Foreign Delete on public.async_p1 async_pt_1
+               Remote SQL: DELETE FROM public.base_tbl1 WHERE ((b = 0)) RETURNING a, b, c
+         ->  Foreign Delete on public.async_p2 async_pt_2
+               Remote SQL: DELETE FROM public.base_tbl2 WHERE ((b = 0)) RETURNING a, b, c
+         ->  Seq Scan on public.async_p3 async_pt_3
+               Output: async_pt_3.tableoid, async_pt_3.ctid
+               Filter: (async_pt_3.b = 0)
+(13 rows)
+
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+  a   | b |    c     
+------+---+----------
+ 1000 | 0 | 00000000
+ 2000 | 0 | 00000000
+ 3000 | 0 | 00000000
+(3 rows)
+
 -- Clean up
 DROP TABLE async_pt;
 DROP TABLE base_tbl1;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 8bcdc8d616..ad316850f7 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2530,6 +2530,12 @@ postgresPlanDirectModify(PlannerInfo *root,
 			rebuild_fdw_scan_tlist(fscan, returningList);
 	}
 
+	/*
+	 * Finally, unset the "async capable" flag if it is set.
+	 */
+	if (fscan->scan.plan.async_capable)
+		fscan->scan.plan.async_capable = false;
+
 	table_close(rel, NoLock);
 	return true;
 }
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index dcd36a9753..194e5dcfe3 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3219,6 +3219,13 @@ DELETE FROM join_tbl;
 RESET enable_mergejoin;
 RESET enable_hashjoin;
 
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+EXPLAIN (VERBOSE, COSTS OFF)
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+
 -- Clean up
 DROP TABLE async_pt;
 DROP TABLE base_tbl1;

Justin Pryzby

pryzby@telsasoft.com

over 4 years ago

In reply to: Etsuro Fujita (#1)

Re: Inherited UPDATE/DELETE vs async execution

On Sat, May 08, 2021 at 01:20:51AM +0900, Etsuro Fujita wrote:

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

DELETE FROM async_pt;
server closed the connection unexpectedly

Confirmed, +Tomas, and added at
https://wiki.postgresql.org/wiki/PostgreSQL_14_Open_Items

--
Justin

Etsuro Fujita

etsuro.fujita@gmail.com

over 4 years ago

In reply to: Justin Pryzby (#2)

Re: Inherited UPDATE/DELETE vs async execution

On Mon, May 10, 2021 at 9:20 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

On Sat, May 08, 2021 at 01:20:51AM +0900, Etsuro Fujita wrote:

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

DELETE FROM async_pt;
server closed the connection unexpectedly

Confirmed, +Tomas, and added at
https://wiki.postgresql.org/wiki/PostgreSQL_14_Open_Items

Thanks for that!

Maybe my explanation was not good, but actually, this is a follow-up
for commits 27e1f1456 and 86dc90056, which were independently
discussed and committed, and IIUC, the batch-insert work by Tomas
would not be related to this. So I’ll work on it unless Tom (or
anyone else) wants to.

Best regards,
Etsuro Fujita

Amit Langote

amitlangote09@gmail.com

over 4 years ago

In reply to: Etsuro Fujita (#1)

Re: Inherited UPDATE/DELETE vs async execution

Fujita-san,

On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

EXPLAIN (VERBOSE, COSTS OFF)
DELETE FROM async_pt;
QUERY PLAN
----------------------------------------------------------------
Delete on public.async_pt
Foreign Delete on public.async_p1 async_pt_1
Foreign Delete on public.async_p2 async_pt_2
Delete on public.async_p3 async_pt_3
-> Append
-> Async Foreign Delete on public.async_p1 async_pt_1
Remote SQL: DELETE FROM public.base_tbl1
-> Async Foreign Delete on public.async_p2 async_pt_2
Remote SQL: DELETE FROM public.base_tbl2
-> Seq Scan on public.async_p3 async_pt_3
Output: async_pt_3.tableoid, async_pt_3.ctid
(11 rows)

DELETE FROM async_pt;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost

The cause for this would be that direct-update plans are mistakenly
treated as async-capable ones, as shown in the EXPLAIN output.

I guess that happens because the ForeignScan nodes responsible for
scanning or direct-updating/deleting from child foreign tables appear
under an Append as of 86dc90056, whereas before they would appear as
child plans of a ModifyTable node. IIUC, it's the Append that causes
the async_capable flag to be set in those ForeignScan nodes.

To
fix, I think we should modify postgresPlanDirectModify() so that it
clears the async-capable flag if it is set. Attached is a patch for
that. Maybe I am missing something, though.

I see that your patch is to disable asynchronous execution in
ForeignScan nodes responsible for direct update/delete, but why not do
the same for other ForeignScan nodes too? Or the other way around --
is it because fixing the crash that occurs in the former's case would
be a significant undertaking for little gain?

--
Amit Langote
EDB: http://www.enterprisedb.com

Etsuro Fujita

etsuro.fujita@gmail.com

over 4 years ago

In reply to: Amit Langote (#4)

Re: Inherited UPDATE/DELETE vs async execution

Amit-san,

On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

EXPLAIN (VERBOSE, COSTS OFF)
DELETE FROM async_pt;
QUERY PLAN
----------------------------------------------------------------
Delete on public.async_pt
Foreign Delete on public.async_p1 async_pt_1
Foreign Delete on public.async_p2 async_pt_2
Delete on public.async_p3 async_pt_3
-> Append
-> Async Foreign Delete on public.async_p1 async_pt_1
Remote SQL: DELETE FROM public.base_tbl1
-> Async Foreign Delete on public.async_p2 async_pt_2
Remote SQL: DELETE FROM public.base_tbl2
-> Seq Scan on public.async_p3 async_pt_3
Output: async_pt_3.tableoid, async_pt_3.ctid
(11 rows)

DELETE FROM async_pt;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost

The cause for this would be that direct-update plans are mistakenly
treated as async-capable ones, as shown in the EXPLAIN output.

I guess that happens because the ForeignScan nodes responsible for
scanning or direct-updating/deleting from child foreign tables appear
under an Append as of 86dc90056, whereas before they would appear as
child plans of a ModifyTable node. IIUC, it's the Append that causes
the async_capable flag to be set in those ForeignScan nodes.

That's right.

The inherited update/delete work is great! Thanks for that!

To
fix, I think we should modify postgresPlanDirectModify() so that it
clears the async-capable flag if it is set. Attached is a patch for
that. Maybe I am missing something, though.

I see that your patch is to disable asynchronous execution in
ForeignScan nodes responsible for direct update/delete, but why not do
the same for other ForeignScan nodes too?

I just thought it would be better to execute other ForeignScan nodes
asynchronously for performance, if they are async-capable.

Or the other way around --
is it because fixing the crash that occurs in the former's case would
be a significant undertaking for little gain?

Yeah, I think it would be a good idea to support "Async Foreign
Delete" in the former's case. And actually, I tried to do so, but I
didn't, because it seemed to take time. I might be missing something,
though.

Thanks!

Best regards,
Etsuro Fujita

Amit Langote

amitlangote09@gmail.com

over 4 years ago

In reply to: Etsuro Fujita (#5)

Re: Inherited UPDATE/DELETE vs async execution

Fujita-san,

On Tue, May 11, 2021 at 5:56 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

I noticed this while working on the
EXPLAIN-ANALYZE-for-async-capable-nodes issue:

EXPLAIN (VERBOSE, COSTS OFF)
DELETE FROM async_pt;
QUERY PLAN
----------------------------------------------------------------
Delete on public.async_pt
Foreign Delete on public.async_p1 async_pt_1
Foreign Delete on public.async_p2 async_pt_2
Delete on public.async_p3 async_pt_3
-> Append
-> Async Foreign Delete on public.async_p1 async_pt_1
Remote SQL: DELETE FROM public.base_tbl1
-> Async Foreign Delete on public.async_p2 async_pt_2
Remote SQL: DELETE FROM public.base_tbl2
-> Seq Scan on public.async_p3 async_pt_3
Output: async_pt_3.tableoid, async_pt_3.ctid
(11 rows)

DELETE FROM async_pt;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost

The cause for this would be that direct-update plans are mistakenly
treated as async-capable ones, as shown in the EXPLAIN output.

I guess that happens because the ForeignScan nodes responsible for
scanning or direct-updating/deleting from child foreign tables appear
under an Append as of 86dc90056, whereas before they would appear as
child plans of a ModifyTable node. IIUC, it's the Append that causes
the async_capable flag to be set in those ForeignScan nodes.

That's right.

The inherited update/delete work is great! Thanks for that!

Thanks.

To
fix, I think we should modify postgresPlanDirectModify() so that it
clears the async-capable flag if it is set. Attached is a patch for
that. Maybe I am missing something, though.

I see that your patch is to disable asynchronous execution in
ForeignScan nodes responsible for direct update/delete, but why not do
the same for other ForeignScan nodes too?

I just thought it would be better to execute other ForeignScan nodes
asynchronously for performance, if they are async-capable.

Okay, so I take it that making these ForeignScan nodes (that only
fetch the data) asynchronous doesn't interfere with update/delete
subsequently being performed over presumably the same connection to
the remote server.

Or the other way around --
is it because fixing the crash that occurs in the former's case would
be a significant undertaking for little gain?

Yeah, I think it would be a good idea to support "Async Foreign
Delete" in the former's case. And actually, I tried to do so, but I
didn't, because it seemed to take time.

Ah I see. I guess it makes sense to prevent such cases in v14 as your
patch does, and revisit this in the future.
--
Amit Langote
EDB: http://www.enterprisedb.com

Etsuro Fujita

etsuro.fujita@gmail.com

over 4 years ago

In reply to: Amit Langote (#6)

1 attachment(s)

Re: Inherited UPDATE/DELETE vs async execution

Amit-san,

On Tue, May 11, 2021 at 9:53 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Tue, May 11, 2021 at 5:56 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

To
fix, I think we should modify postgresPlanDirectModify() so that it
clears the async-capable flag if it is set. Attached is a patch for
that. Maybe I am missing something, though.

I see that your patch is to disable asynchronous execution in
ForeignScan nodes responsible for direct update/delete, but why not do
the same for other ForeignScan nodes too?

I just thought it would be better to execute other ForeignScan nodes
asynchronously for performance, if they are async-capable.

Okay, so I take it that making these ForeignScan nodes (that only
fetch the data) asynchronous doesn't interfere with update/delete
subsequently being performed over presumably the same connection to
the remote server.

Good point! I don't think it would interfere with the update/delete,
because in that case postgres_fdw would actually perform the
update/delete and the asynchronous foreign scans serially rather than
concurrently. (They wouldn't be perfomed in parallel unless they use
different connections, in other words.)

Or the other way around --
is it because fixing the crash that occurs in the former's case would
be a significant undertaking for little gain?

Yeah, I think it would be a good idea to support "Async Foreign
Delete" in the former's case. And actually, I tried to do so, but I
didn't, because it seemed to take time.

Ah I see. I guess it makes sense to prevent such cases in v14 as your
patch does, and revisit this in the future.

Here is a rebased version of the patch. I'm planning to apply this tommorow.

Thanks for the comment!

Best regards,
Etsuro Fujita

Attachments:

fix-postgresPlanDirectModify-v2.patchapplication/octet-stream; name=fix-postgresPlanDirectModify-v2.patchDownload

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0b0c45f0d9..7df30010f2 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10154,6 +10154,61 @@ DROP TABLE base_tbl3;
 DROP TABLE base_tbl4;
 RESET enable_mergejoin;
 RESET enable_hashjoin;
+-- Test that UPDATE/DELETE with inherited target works with async_capable enabled
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+                                                QUERY PLAN                                                
+----------------------------------------------------------------------------------------------------------
+ Update on public.async_pt
+   Output: async_pt_1.a, async_pt_1.b, async_pt_1.c
+   Foreign Update on public.async_p1 async_pt_1
+   Foreign Update on public.async_p2 async_pt_2
+   Update on public.async_p3 async_pt_3
+   ->  Append
+         ->  Foreign Update on public.async_p1 async_pt_1
+               Remote SQL: UPDATE public.base_tbl1 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c
+         ->  Foreign Update on public.async_p2 async_pt_2
+               Remote SQL: UPDATE public.base_tbl2 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c
+         ->  Seq Scan on public.async_p3 async_pt_3
+               Output: (async_pt_3.c || async_pt_3.c), async_pt_3.tableoid, async_pt_3.ctid, NULL::record
+               Filter: (async_pt_3.b = 0)
+(13 rows)
+
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+  a   | b |    c     
+------+---+----------
+ 1000 | 0 | 00000000
+ 2000 | 0 | 00000000
+ 3000 | 0 | 00000000
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF)
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+                                        QUERY PLAN                                        
+------------------------------------------------------------------------------------------
+ Delete on public.async_pt
+   Output: async_pt_1.a, async_pt_1.b, async_pt_1.c
+   Foreign Delete on public.async_p1 async_pt_1
+   Foreign Delete on public.async_p2 async_pt_2
+   Delete on public.async_p3 async_pt_3
+   ->  Append
+         ->  Foreign Delete on public.async_p1 async_pt_1
+               Remote SQL: DELETE FROM public.base_tbl1 WHERE ((b = 0)) RETURNING a, b, c
+         ->  Foreign Delete on public.async_p2 async_pt_2
+               Remote SQL: DELETE FROM public.base_tbl2 WHERE ((b = 0)) RETURNING a, b, c
+         ->  Seq Scan on public.async_p3 async_pt_3
+               Output: async_pt_3.tableoid, async_pt_3.ctid
+               Filter: (async_pt_3.b = 0)
+(13 rows)
+
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+  a   | b |    c     
+------+---+----------
+ 1000 | 0 | 00000000
+ 2000 | 0 | 00000000
+ 3000 | 0 | 00000000
+(3 rows)
+
 -- Check EXPLAIN ANALYZE for a query that scans empty partitions asynchronously
 DELETE FROM async_p1;
 DELETE FROM async_p2;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index ee93ee07cc..75b6950973 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2530,6 +2530,12 @@ postgresPlanDirectModify(PlannerInfo *root,
 			rebuild_fdw_scan_tlist(fscan, returningList);
 	}
 
+	/*
+	 * Finally, unset the async-capable flag if it is set.
+	 */
+	if (fscan->scan.plan.async_capable)
+		fscan->scan.plan.async_capable = false;
+
 	table_close(rel, NoLock);
 	return true;
 }
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 53adfe2abc..78379bdea5 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3237,6 +3237,14 @@ DROP TABLE base_tbl4;
 RESET enable_mergejoin;
 RESET enable_hashjoin;
 
+-- Test that UPDATE/DELETE with inherited target works with async_capable enabled
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
+EXPLAIN (VERBOSE, COSTS OFF)
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+DELETE FROM async_pt WHERE b = 0 RETURNING *;
+
 -- Check EXPLAIN ANALYZE for a query that scans empty partitions asynchronously
 DELETE FROM async_p1;
 DELETE FROM async_p2;

Amit Langote

amitlangote09@gmail.com

over 4 years ago

In reply to: Etsuro Fujita (#7)

Re: Inherited UPDATE/DELETE vs async execution

On Wed, May 12, 2021 at 6:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Tue, May 11, 2021 at 9:53 PM Amit Langote <amitlangote09@gmail.com> wrote:

Okay, so I take it that making these ForeignScan nodes (that only
fetch the data) asynchronous doesn't interfere with update/delete
subsequently being performed over presumably the same connection to
the remote server.

Good point! I don't think it would interfere with the update/delete,
because in that case postgres_fdw would actually perform the
update/delete and the asynchronous foreign scans serially rather than
concurrently. (They wouldn't be perfomed in parallel unless they use
different connections, in other words.)

I see, that makes sense.

Or the other way around --
is it because fixing the crash that occurs in the former's case would
be a significant undertaking for little gain?

Yeah, I think it would be a good idea to support "Async Foreign
Delete" in the former's case. And actually, I tried to do so, but I
didn't, because it seemed to take time.

Ah I see. I guess it makes sense to prevent such cases in v14 as your
patch does, and revisit this in the future.

+1

Here is a rebased version of the patch. I'm planning to apply this tommorow.

+   /*
+    * Finally, unset the async-capable flag if it is set.
+    */

Would it make sense to expand here even just a bit on why we must do this?

--
Amit Langote
EDB: http://www.enterprisedb.com

Etsuro Fujita

etsuro.fujita@gmail.com

over 4 years ago

In reply to: Amit Langote (#8)

Re: Inherited UPDATE/DELETE vs async execution

On Thu, May 13, 2021 at 3:32 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Wed, May 12, 2021 at 6:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Here is a rebased version of the patch. I'm planning to apply this tommorow.
+   /*
+    * Finally, unset the async-capable flag if it is set.
+    */
Would it make sense to expand here even just a bit on why we must do this?

+1 How about something like this?

"Finally, unset the async-capable flag if it is set, as we currently
don't support asynchronous execution of direct modifications."

Best regards,
Etsuro Fujita

#10

Etsuro Fujita

etsuro.fujita@gmail.com

over 4 years ago

In reply to: Etsuro Fujita (#9)

Re: Inherited UPDATE/DELETE vs async execution

On Thu, May 13, 2021 at 5:00 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Thu, May 13, 2021 at 3:32 PM Amit Langote <amitlangote09@gmail.com> wrote:
On Wed, May 12, 2021 at 6:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Here is a rebased version of the patch. I'm planning to apply this tommorow.
+   /*
+    * Finally, unset the async-capable flag if it is set.
+    */
Would it make sense to expand here even just a bit on why we must do this?
+1 How about something like this?

"Finally, unset the async-capable flag if it is set, as we currently
don't support asynchronous execution of direct modifications."

Pushed after modifying the comment as such. I think we could improve
it later. :-)

Thanks for the comment!

Will close this in the open items list.

Best regards,
Etsuro Fujita

#11

Amit Langote

amitlangote09@gmail.com

over 4 years ago

In reply to: Etsuro Fujita (#10)

Re: Inherited UPDATE/DELETE vs async execution

On Thu, May 13, 2021 at 8:10 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Thu, May 13, 2021 at 5:00 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
On Thu, May 13, 2021 at 3:32 PM Amit Langote <amitlangote09@gmail.com> wrote:
On Wed, May 12, 2021 at 6:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Here is a rebased version of the patch. I'm planning to apply this tommorow.
+   /*
+    * Finally, unset the async-capable flag if it is set.
+    */
Would it make sense to expand here even just a bit on why we must do this?
+1 How about something like this?

"Finally, unset the async-capable flag if it is set, as we currently
don't support asynchronous execution of direct modifications."
Pushed after modifying the comment as such. I think we could improve
it later. :-)

Looks good as pushed, thank you.

--
Amit Langote
EDB: http://www.enterprisedb.com