[DBD::Pg] First pass at solving the ping/pg_ping problem. RT #100648

Committed by Greg Sabino Mullane <greg-O9fzpki4YnJWk0Htik3J/[email protected]>

First pass at solving the ping/pg_ping problem. RT #100648

---
 Pg.pm          |  2 --
 dbdimp.c       | 31 +++++++++++++++++++------------
 t/03dbmethod.t | 37 ++++++++++++++++---------------------
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/Pg.pm b/Pg.pm
index 56110bc..831d768 100644
--- a/Pg.pm
+++ b/Pg.pm
@@ -2768,8 +2768,6 @@ return the following:
    -2      An unknown transaction status was returned (e.g. after forking)
    -3      The handle exists, but no data was returned from a test query.
 
-In practice, you should only ever see -1 and -2.
-
 =head3 B<get_info>
 
   $value = $dbh->get_info($info_type);
diff --git a/dbdimp.c b/dbdimp.c
index 022058b..334b19e 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -464,6 +464,7 @@ int dbd_db_ping (SV * dbh)
 	}
 
 	tstatus = pg_db_txn_status(aTHX_ imp_dbh);
+	/* 0=idle 1=active 2=intrans 3=inerror 4=unknown */
 
 	if (TRACE5_slow) TRC(DBILOGFP, "%sdbd_db_ping txn_status is %d\n", THEADER_slow, tstatus);
 
@@ -472,25 +473,31 @@ int dbd_db_ping (SV * dbh)
 		return -2;
 	}
 
-	if (tstatus != 0) {/* 2=active, 3=intrans, 4=inerror */
-		if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (result: %d)\n", THEADER_slow, 1+tstatus);
-		return 1+tstatus;
-	}
+	/* No matter what state we are in, send a SELECT to the backend */
+	status = _result(aTHX_ imp_dbh, "SELECT 'DBD::Pg ping test'");
 
-	/* Even though it may be reported as normal, we have to make sure by issuing a command */
+	if (0 == tstatus || 2 == tstatus) {
 
-	status = _result(aTHX_ imp_dbh, "SELECT 'DBD::Pg ping test'");
+		/* If we are simply idle or in a transaction, we should see tuples */
+
+		if (PGRES_TUPLES_OK == status) {
+			if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (result: 1 PGRES_TUPLES_OK)\n", THEADER_slow);
+			return 1+tstatus;
+		}
 
-	if (PGRES_TUPLES_OK == status) {
-		if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (result: 1 PGRES_TUPLES_OK)\n", THEADER_slow);
-		return 1;
 	}
 
-	if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (result: -3)\n", THEADER_slow);
-	return -3;
+	/* A status of 7 could indicate a failed query or a dead database. We need PQstatus to be sure */
+	if (CONNECTION_BAD == PQstatus(imp_dbh->conn)) {
+		if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (PQstatus returned CONNECTION_BAD)\n", THEADER_slow);
+		return -3;
+	}
 
-} /* end of dbd_db_ping */
+	if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping\n", THEADER_slow);
+	return 1+tstatus;
 
+} /* end of dbd_db_ping */
+ 
 
 /* ================================================================== */
 static PGTransactionStatusType pg_db_txn_status (pTHX_ imp_dbh_t * imp_dbh)
diff --git a/t/03dbmethod.t b/t/03dbmethod.t
index dffee0e..fc3bd05 100644
--- a/t/03dbmethod.t
+++ b/t/03dbmethod.t
@@ -26,7 +26,7 @@ my $dbh = connect_database();
 if (! $dbh) {
 	plan skip_all => 'Connection to database failed, cannot continue testing';
 }
-plan tests => 547;
+plan tests => 546;
 
 isnt ($dbh, undef, 'Connect to database for database handle method testing');
 
@@ -1882,10 +1882,9 @@ $dbh2->disconnect();
 $t='DB handle method "ping" returns 1 on an idle connection';
 is ($dbh->ping(), 1, $t);
 
-$t='DB handle method "ping" returns 1 for a good connection inside a transaction';
+$t='DB handle method "ping" returns 3 for a good connection inside a transaction';
 $dbh->do('SELECT 123');
-$result = 1;
-is ($result, $dbh->ping(), $t);
+is ($dbh->ping(), 3, $t);
 
 $t='DB handle method "ping" returns 1 on an idle connection';
 $dbh->commit();
@@ -1899,21 +1898,20 @@ $dbh->do('COPY dbd_pg_test(id,pname) TO STDOUT');
 	local $SIG{__WARN__} = sub {};
 	$dbh->pg_getline($mtvar,100);
 }
-#is ($dbh->ping(), 2, $t);
-1 while $dbh->pg_getline($mtvar,1000);
-
-$t='DB handle method "ping" returns 1 for a good connection inside a transaction';
-$dbh->do('SELECT 123');
-is ($dbh->ping(), 1, $t);
+is ($dbh->ping(), 2, $t);
+## This has sent a SELECT, which messes up our COPY state!
+$dbh->rollback();
 
 $t='DB handle method "ping" returns a 4 when inside a failed transaction';
 eval {
 	$dbh->do('DBD::Pg creating an invalid command for testing');
 };
 is ($dbh->ping(), 4, $t);
+$dbh->rollback();
 
-
-exit;
+$t='DB handle method "ping" returns 1 on an idle connection';
+$dbh->commit();
+is ($dbh->ping(), 1, $t);
 
 $t='DB handle method "ping" fails (returns 0) on a disconnected handle';
 $dbh->disconnect();
@@ -1930,9 +1928,9 @@ isnt ($dbh, undef, $t);
 $t='DB handle method "pg_ping" returns 1 on an idle connection';
 is ($dbh->pg_ping(), 1, $t);
 
-$t='DB handle method "pg_ping" returns 1 for a good connection inside a transaction';
+$t='DB handle method "pg_ping" returns 3 for a good connection inside a transaction';
 $dbh->do('SELECT 123');
-is ($dbh->pg_ping(), 1, $t);
+is ($dbh->pg_ping(), 3, $t);
 
 $t='DB handle method "pg_ping" returns 1 on an idle connection';
 $dbh->commit();
@@ -1942,14 +1940,11 @@ $t='DB handle method "pg_ping" returns 2 when in COPY IN state';
 $dbh->do('COPY dbd_pg_test(id,pname) TO STDOUT');
 $dbh->pg_getline($mtvar,100);
 is ($dbh->pg_ping(), 2, $t);
+$dbh->rollback();
 
-$t='DB handle method "pg_ping" returns 2 immediately after COPY IN state';
-1 while $dbh->pg_getline($mtvar,1000);
-is ($dbh->pg_ping(), 2, $t);
-
-$t='DB handle method "pg_ping" returns 1 for a good connection inside a transaction';
+$t='DB handle method "pg_ping" returns 3 for a good connection inside a transaction';
 $dbh->do('SELECT 123');
-is ($dbh->pg_ping(), 1, $t);
+is ($dbh->pg_ping(), 3, $t);
 
 $t='DB handle method "pg_ping" returns a 4 when inside a failed transaction';
 eval {
@@ -1957,7 +1952,7 @@ eval {
 };
 is ($dbh->pg_ping(), 4, $t);
 
-$t='DB handle method "pg_ping" fails (returns 0) on a disconnected handle';
+$t='DB handle method "pg_ping" fails (returns -1) on a disconnected handle';
 cleanup_database($dbh,'test');
 $dbh->disconnect();
 is ($dbh->pg_ping(), -1, $t);
-- 
1.8.4