Re: [BUG] Logical replica crash if there was an error in a function.

[Anton A. Melnikov]

Hello!

Thanks for reply!

On 24.09.2022 20:27, Tom Lane wrote:

I think you're solving the
problem in the wrong place.  The real issue is why are
we not setting up ActivePortal correctly when running
user-defined code in a logrep worker?

During a common query from the backend ActivePortal becomes defined
in the PortalRun and then AfterTriggerEndQuery starts with
non-NULL ActivePortal after ExecutorFinish.
When the logrep worker is applying messages there are neither
PortalStart nor PortalRun calls. And AfterTriggerEndQuery starts
with undefined ActivePortal after finish-edata().
May be it's normal behavior?

There is other code
that expects that to be set, eg EnsurePortalSnapshotExists.

When the logrep worker is applying message it doesn't have to use
ActivePortal in EnsurePortalSnapshotExists because ActiveSnapshot is already 
installed.
It is set at the beginning of each transaction in the begin_replication_step 
call.

On the other hand, function_parse_error_transpose() tries to get
the original query text  (INSERT INTO test VALUES ('1') in our case) from
the ActivePortal to clarify the location of the error.
But in the logrep worker there is no way to restore original query text
from the logrep message. There is only 'zipped' query equivalent to the 
original.
So any function_parse_error_transpose() call seems to be useless
in the logrep worker.

And it looks like we can simply omit match_prosrc_to_query() call there.
The attached patch does it.

Best wishes,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
commit bfaa02ac7a7cbeb793747be71962a7799c60c21c
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Sun Oct 9 11:56:20 2022 +0300

    Fix logical replica crash if there was an error in a user function.

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index a9fe45e347..1e8f1b1097 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -36,6 +36,7 @@
 #include "parser/parse_coerce.h"
 #include "parser/parse_type.h"
 #include "pgstat.h"
+#include "replication/logicalworker.h"
 #include "rewrite/rewriteHandler.h"
 #include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
@@ -1034,12 +1035,20 @@ function_parse_error_transpose(const char *prosrc)
 			return false;
 	}
 
-	/* We can get the original query text from the active portal (hack...) */
-	Assert(ActivePortal && ActivePortal->status == PORTAL_ACTIVE);
-	queryText = ActivePortal->sourceText;
+	/*
+	 * In the logical replication worker there is no way to restore original
+	 * query text from the logical replication message. There is only 'zipped'
+	 * query equivalent to the original text.
+	 */
+	if (!IsLogicalWorker())
+	{
+		/* We can get the original query text from the active portal (hack...) */
+		Assert(ActivePortal && ActivePortal->status == PORTAL_ACTIVE);
+		queryText = ActivePortal->sourceText;
 
-	/* Try to locate the prosrc in the original text */
-	newerrposition = match_prosrc_to_query(prosrc, queryText, origerrposition);
+		/* Try to locate the prosrc in the original text */
+		newerrposition = match_prosrc_to_query(prosrc, queryText, origerrposition);
+	}
 
 	if (newerrposition > 0)
 	{