Mixup with prepared statement handles

[pepl]

With the upgrade to the latest release, we've been running in troubles with server side prepared statements and async usage: Statement handles got mixed up in the same processes and errors looked like this one

DBD::Pg::st execute failed: ERROR:  prepared statement "dbdpg_p239066_1" already exists

or mismatch of bind variables count.

Two work-arounds have been
(1) Disable pg_server_prepare
(2) Do not use prepare_cached (in a setup where async and non-async queries are mixed)

The issue seems to be related to prepare_number not being incremented with 48bf171 in the async case which this patch does address:

host:dbdpg✔ [master]$ git diff dbdimp.c
diff --git a/dbdimp.c b/dbdimp.c
index cd6e093..4d42032 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -2552,9 +2552,11 @@ static int pg_st_prepare_statement (pTHX_ SV * sth, imp_sth_t * imp_sth)
         TRACE_PQSENDPREPARE;
         status = PQsendPrepare(imp_dbh->conn, imp_sth->prepare_name, statement, params,
                                imp_sth->PQoids);
-        if (status)
+        if (status) {
             imp_sth->async_status = STH_ASYNC_PREPARE;
-        else {
+            imp_sth->prepared_by_us = DBDPG_TRUE;
+            imp_dbh->prepare_number++;
+        } else {
             status = PGRES_FATAL_ERROR;
             _fatal_sqlstate(aTHX_ imp_dbh);
         }

As I am neither an expert in dbdimp.c nor could I reproduce this with updating 08async.t (maybe related to our local PgBouncer usage?), I am opening this a bug ticket instead of PR.

[esabol]

That would be a major regression, certainly!

The patch looks promising, but what is really needed is some way to reproduce the bug. Any kind of standalone Perl program would do. We can worry about integrating it into the test suite later.

[esabol]

I used an AI to come up with the following test file (rename it to dbdpg_issue_183.t after downloading):

dbdpg_issue_183.txt

It seems to reproduce the issue? It's a bit hefty... Unfortunately, the suggested patch to dbdimp.c doesn't seem to get the tests to pass. I'm not sure if the problem is with the test file or the patch.

I use the following Dockerfile (which I came up with on my own!) for testing (rename to Dockerfile after downloading, of course):

Dockerfile.txt

docker build -t dbdpg-testing:latest .

Patched dbdimp.c for the Docker image:

dbdimp.c

[esabol]

Oh, duh, the AI hallucinated the pg_async syntax in a couple places. Here's my revised test file:

dbdpg_issue_183.txt

It still fails though, unfortunately, and I'm not sure why.

#   Failed test 'prepare_cached() + non-async execute immediately after async path works (no mixup) - correct result'
#   at t/90issue_183.t line 141.
#          got: '120'
#     expected: '130'
DBD::Pg::st pg_ready failed: No asynchronous query is running at t/90issue_183.t line 46.
# Looks like your test exited with 255 just after 43.
t/90issue_183.t ........ 
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 40/82 subtests 

[pepl]

I've tried to slim down our specific usage in dbdpg_issue_183.t.txt but the script works out even with the unpatched dbdimp.c version even though confirming the if (status) { branch is hit without incrementing prepare_number there.

In our case $dbh_select and $dbh are two different databases.

[pepl]

This one can be used to reproduce the error message and check that the patch fixes this:

dbdpg_issue_reproducable-183.t.txt