[DBD::Pg 1/3] Further tweaking to the ping() and pg_ping() methods. We now use an empty query consisting of a single SQL comment sent via PQexec.

Committed by Greg Sabino Mullane <[email protected]>

Subject: [DBD::Pg 1/3] Further tweaking to the ping() and pg_ping() methods.
We now use an empty query consisting of a single SQL comment sent via PQexec.

---
 Pg.pm          |   5 +-
 dbdimp.c       |  35 ++++++-------
 t/03dbmethod.t | 163 +++++++++++++++++++++------------------------------------
 3 files changed, 79 insertions(+), 124 deletions(-)

diff --git a/Pg.pm b/Pg.pm
index de0eee2..703f882 100644
--- a/Pg.pm
+++ b/Pg.pm
@@ -2740,7 +2740,7 @@ server version 9.0 or higher.
 
 The C<ping> method determines if there is a working connection to an active 
 database server. It does this by sending a small query to the server, currently 
-B<SELECT 'DBD::Pg ping test'>. It returns 0 (false) if the connection is not valid, 
+B<'DBD::Pg ping test v3.5.0'>. It returns 0 (false) if the connection is not valid, 
 otherwise it returns a positive number (true). The value returned indicates the 
 current state:
 
@@ -2768,7 +2768,8 @@ return the following:
   --------------------------------------------------
    -1      There is no connection to the database at all (e.g. after disconnect)
    -2      An unknown transaction status was returned (e.g. after forking)
-   -3      The handle exists, but no data was returned from a test query.
+   -3      The test query failed (PQexec returned null)
+   -4      PQstatus returned a CONNECTION_BAD
 
 =head3 B<get_info>
 
diff --git a/dbdimp.c b/dbdimp.c
index 359865c..d437d4d 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -455,6 +455,7 @@ int dbd_db_ping (SV * dbh)
 	D_imp_dbh(dbh);
 	PGTransactionStatusType tstatus;
 	ExecStatusType          status;
+	PGresult              * result;
 
 	if (TSTART_slow) TRC(DBILOGFP, "%sBegin dbd_db_ping\n", THEADER_slow);
 
@@ -464,8 +465,6 @@ 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);
 
 	if (tstatus >= 4) { /* Unknown, so we err on the side of "bad" */
@@ -473,30 +472,30 @@ int dbd_db_ping (SV * dbh)
 		return -2;
 	}
 
-	/* No matter what state we are in, send a SELECT to the backend */
-	status = _result(aTHX_ imp_dbh, "SELECT 'DBD::Pg ping test'");
-
-	/* If we are idle or in a transaction, we should see tuples */
-	if (0 == tstatus || 2 == 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+tstatus;
-		}
+	/* No matter what state we are in, send an empty query to the backend */
+	result = PQexec(imp_dbh->conn, "/* DBD::Pg ping test v3.5.0 */");
+	if (NULL == result) {
+		/* Something very bad, usually indicating the backend is gone */
+		return -3;
+	}
+	status = PQresultStatus(result);
+	PQclear(result);
 
-		/* Something is wrong, so we return an error */
-		return -2;
+	/* We expect to see an empty query most times */
+	if (PGRES_EMPTY_QUERY == status) {
+		if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping (PGRES_EMPTY_QUERY)\n", THEADER_slow);
+		return 1+tstatus;
+		/* 0=idle 1=active 2=intrans 3=inerror 4=unknown */
 	}
 
-	/* We are in some other state. As a status of 7 could be a bad query or a dead database,
-	   we need to call PQstatus to be sure
-	*/
+	/* As a safety measure, check PQstatus as well */
 	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;
+		return -4;
 	}
 
 	if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_pg_ping\n", THEADER_slow);
+
 	return 1+tstatus;
 
 } /* end of dbd_db_ping */
diff --git a/t/03dbmethod.t b/t/03dbmethod.t
index e78f75d..0e5d94e 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 => 548;
+plan tests => 549;
 
 isnt ($dbh, undef, 'Connect to database for database handle method testing');
 
@@ -1876,118 +1876,73 @@ is ($@, q{}, $t);
 $dbh2->disconnect();
 
 #
-# Test of the "ping" database handle method
+# Test of the "ping" and "pg_ping" database handle methods
 #
 
-$t='DB handle method "ping" returns 1 on an idle connection';
-is ($dbh->ping(), 1, $t);
+my $mtvar; ## This is an implicit test of getcopydata: please leave this var undefined
 
-$t='DB handle method "ping" returns 3 for a good connection inside a transaction';
-$dbh->do('SELECT 123');
-is ($dbh->ping(), 3, $t);
-
-$t='DB handle method "ping" returns 1 on an idle connection';
-$dbh->commit();
-is ($dbh->ping(), 1, $t);
-
-my $mtvar; ## This is an implicit test of getline: please leave this var undefined
-
-$t='DB handle method "ping" returns 2 when in COPY IN state';
-$dbh->do('COPY dbd_pg_test(id,pname) TO STDOUT');
-{
-	local $SIG{__WARN__} = sub {};
-	$dbh->pg_getline($mtvar,100);
-}
-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();
-
-$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();
-is ($dbh->ping(), 0, $t);
-
-$t='Able to reconnect to the database after disconnect';
-$dbh = connect_database({nosetup => 1});
-isnt ($dbh, undef, $t);
-
-$t='DB handle method "ping" returns 1 on an idle connection';
-$dbh->commit();
-is ($dbh->ping(), 1, $t);
-
-$t='DB handle method "ping" returns 0 after a lost network connection (outside transaction)';
-my $fd = $dbh->{pg_socket} or die "Could not determine socket";
-open(DBH_PG_FH, "<&=".$fd) or die "Could not open socket: $!";
-close DBH_PG_FH or die "Could not close socket: $!";
-is ($dbh->ping(), 0, $t);
-
-## Reconnect, and try the same thing but inside a transaction
-$t='DB handle method "ping" returns 0 after a lost network connection (inside transaction)';
-diag "About to call...";
-$dbh->disconnect();
-$dbh = connect_database({nosetup => 1});
-$dbh->do("SELECT 'DBD::Pg testing'");
-$fd = $dbh->{pg_socket} or die "Could not determine socket";
-open(DBH_PG_FH, "<&=".$fd) or die "Could not open socket: $!";
-close DBH_PG_FH or die "Could not close socket: $!";
-is ($dbh->ping(), 0, $t);
-
-$t='DB handle method "ping" returns 0 after a lost network connection (inside failed transaction)';
-$dbh->{InactiveDestroy} = 1;
-undef $dbh;
-$dbh = connect_database({nosetup => 1});
-eval { $dbh->do("SELECT 'DBD::Pg testing, will fail'::int"); };
-$fd = $dbh->{pg_socket} or die "Could not determine socket";
-open(DBH_PG_FH, "<&=".$fd) or die "Could not open socket: $!";
-close DBH_PG_FH or die "Could not close socket: $!";
-is ($dbh->ping(), 0, $t);
-
-$dbh->disconnect();
-$dbh = connect_database({nosetup => 1});
+for my $type (qw/ ping pg_ping /) {
 
+	$t=qq{DB handle method "$type" returns 1 on an idle connection};
+	$dbh->commit();
+	is ($dbh->$type(), 1, $t);
 
-#
-# Test of the "pg_ping" database handle method
-#
+	$t=qq{DB handle method "$type" returns 2 when in COPY IN state};
+	$dbh->do('COPY dbd_pg_test(id,pname) TO STDOUT');
+	$dbh->pg_getcopydata($mtvar);
+	is ($dbh->$type(), 2, $t);
+	## the ping messes up the copy state, so all we can do is rollback
+	$dbh->rollback();
 
-$t='DB handle method "pg_ping" returns 1 on an idle connection';
-is ($dbh->pg_ping(), 1, $t);
+	$t=qq{DB handle method "$type" returns 2 when in COPY IN state};
+	$dbh->do('COPY dbd_pg_test(id,pname) FROM STDIN');
+	$dbh->pg_putcopydata("123\tfoobar\n");
+	is ($dbh->$type(), 2, $t);
+	$dbh->rollback();
 
-$t='DB handle method "pg_ping" returns 3 for a good connection inside a transaction';
-$dbh->do('SELECT 123');
-is ($dbh->pg_ping(), 3, $t);
+	$t=qq{DB handle method "$type" returns 3 for a good connection inside a transaction};
+	$dbh->do('SELECT 123');
+	is ($dbh->$type(), 3, $t);
 
-$t='DB handle method "pg_ping" returns 1 on an idle connection';
-$dbh->commit();
-is ($dbh->pg_ping(), 1, $t);
+	$t=qq{DB handle method "$type" returns a 4 when inside a failed transaction};
+	eval {
+		$dbh->do('DBD::Pg creating an invalid command for testing');
+	};
+	is ($dbh->$type(), 4, $t);
+	$dbh->rollback();
 
-$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();
+	my $val = $type eq 'ping' ? 0 : -1;
+	$t=qq{DB handle method "type" fails (returns $val) on a disconnected handle};
+	$dbh->disconnect();
+	is ($dbh->$type(), $val, $t);
+
+	$t='Able to reconnect to the database after disconnect';
+	$dbh = connect_database({nosetup => 1});
+	isnt ($dbh, undef, $t);
+
+	$val = $type eq 'ping' ? 0 : -3;
+	$t=qq{DB handle method "$type" returns $val after a lost network connection (outside transaction)};
+	socket_fail($dbh);
+	is ($dbh->$type(), $val, $t);
+
+	## Reconnect, and try the same thing but inside a transaction
+	$val = $type eq 'ping' ? 0 : -3;
+	$t=qq{DB handle method "$type" returns $val after a lost network connection (inside transaction)};
+	$dbh = connect_database({nosetup => 1});
+	$dbh->do("SELECT 'DBD::Pg testing'");
+	socket_fail($dbh);
+	is ($dbh->$type(), $val, $t);
+
+	$type eq 'ping' and $dbh = connect_database({nosetup => 1});
+}
 
-$t='DB handle method "pg_ping" returns 3 for a good connection inside a transaction';
-$dbh->do('SELECT 123');
-is ($dbh->pg_ping(), 3, $t);
+exit;
 
-$t='DB handle method "pg_ping" returns a 4 when inside a failed transaction';
-eval {
-	$dbh->do('DBD::Pg creating an invalid command for testing');
-};
-is ($dbh->pg_ping(), 4, $t);
+sub socket_fail {
+	my $dbh = shift;
+	$dbh->{InactiveDestroy} = 1;
+	my $fd = $dbh->{pg_socket} or die "Could not determine socket";
+	open(DBH_PG_FH, "<&=".$fd) or die "Could not open socket: $!";
+	close DBH_PG_FH or die "Could not close socket: $!";
+}
 
-$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