RE: Any DBD::Pg Committers out there?

"Eric Simon" <[email protected]> Sun, 21 Nov 2010 22:07:42 -0600
Newsgroups gmane.comp.db.postgresql.dbdpg
Message-ID <F6082937769C4768B931EA3F49FD660D@Masterchief>
Ok, here's the patch for the cancel() feature with one test added to
03smethod.t (so that the assertion at the top of that test file would remain
true).  I tried to think of other ways cancel() could be tested, but there
just isn't any.  It is useful when another database handle is holding a lock
on a table/row, which I demonstrated in the test (when you call cancel() on
a statement handle that isn't blocking, it is just a no-op, so there's no
purpose in testing that either).

Just a heads-up, I had to 'use POSIX' in 03smethod.t as that is the only way
to reliably handle signals (DBI's doc even suggests this at the bottom of
its "Signal Handling and Canceling Operations" section).

-- 
Eric Simon
The IQ Group, Inc.


-----Original Message-----
From: David E. Wheeler [mailto:[email protected]] 
Sent: Tuesday, November 16, 2010 5:05 PM
To: Eric Simon
Cc: [email protected]
Subject: Re: Any DBD::Pg Committers out there?

On Nov 16, 2010, at 3:01 PM, Eric Simon wrote:

> I also patched the 04misc test because it was failing under Perl 5.12.  If
> you'd like me to break that out into a separate patch, let me know.

Yeah, that'd be great.

As for the cancel bit:

* I can't speak to the implementation, as I don't know XS at all. Can anyone
else have a look at comment?

* Would you mind adding some tests for the new feature?

Thanks,

David
dbd-pg-cancel.patch (application/octet-stream, 3.9 KB)
Index: dbdimp.c
===================================================================
--- dbdimp.c	(revision 14529)
+++ dbdimp.c	(working copy)
@@ -4909,7 +4909,40 @@
 } /* end of handle_old_async */
 
 
+/* ================================================================== */
+/* Attempt to cancel a synchronous query
+   Returns true if the cancel succeeded, and false if it did not */
+int dbd_st_cancel(SV *sth, imp_sth_t *imp_sth)
+{
+	dTHX;
+	D_imp_dbh_from_sth;
+	PGcancel *cancel;
+	char errbuf[256];
 
+	if (TSTART) TRC(DBILOGFP, "%sBegin dbd_st_cancel\n", THEADER);
+
+	/* Get the cancel structure */
+	TRACE_PQGETCANCEL;
+	cancel = PQgetCancel(imp_dbh->conn);
+
+	/* This almost always works. If not, free our structure and complain loudly */
+	TRACE_PQGETCANCEL;
+	if (!PQcancel(cancel, errbuf, sizeof(errbuf))) {
+		TRACE_PQFREECANCEL;
+		PQfreeCancel(cancel);
+		if (TRACEWARN) TRC(DBILOGFP, "%sPQcancel failed: %s\n", THEADER, errbuf);
+		pg_error(aTHX_ sth, PGRES_FATAL_ERROR, "PQcancel failed");
+		if (TEND) TRC(DBILOGFP, "%sEnd dbd_st_cancel (error: cancel failed)\n", THEADER);
+		return DBDPG_FALSE;
+	}
+	TRACE_PQFREECANCEL;
+	PQfreeCancel(cancel);
+
+	if (TEND) TRC(DBILOGFP, "%sEnd dbd_st_cancel\n", THEADER);
+	return DBDPG_TRUE;
+
+} /* end of dbd_st_cancel */
+
 /*
 Some information to keep you sane:
 typedef enum
Index: dbdimp.h
===================================================================
--- dbdimp.h	(revision 14529)
+++ dbdimp.h	(working copy)
@@ -166,9 +166,12 @@
 #define dbd_st_rows pg_st_rows
 int dbd_st_rows (SV * sth, imp_sth_t * imp_sth);
 
-#define dbd_st_finish  pg_st_finidh
+#define dbd_st_finish  pg_st_finish
 int dbd_st_finish (SV * sth, imp_sth_t * imp_sth);
 
+#define dbd_st_cancel pg_st_cancel
+int dbd_st_cancel (SV * sth, imp_sth_t * imp_sth);
+
 #define dbd_st_destroy  pg_st_destroy
 void dbd_st_destroy (SV * sth, imp_sth_t * imp_sth);
 
Index: Pg.xs
===================================================================
--- Pg.xs	(revision 14529)
+++ Pg.xs	(working copy)
@@ -796,6 +796,13 @@
 	D_imp_sth(sth);
 	ST(0) = pg_db_cancel_sth(sth, imp_sth) ? &PL_sv_yes : &PL_sv_no;
 
+void
+cancel(sth)
+	SV *sth
+	CODE:
+	D_imp_sth(sth);
+	ST(0) = dbd_st_cancel(sth, imp_sth) ? &PL_sv_yes : &PL_sv_no;
+
 #if PGLIBVERSION >= 80000
 
 void
Index: t/03smethod.t
===================================================================
--- t/03smethod.t	(revision 14529)
+++ t/03smethod.t	(working copy)
@@ -9,6 +9,7 @@
 use 5.006;
 use strict;
 use warnings;
+use POSIX qw(:signal_h);
 use Test::More;
 use DBI ':sql_types';
 use lib 't','.';
@@ -20,7 +21,7 @@
 if (! $dbh) {
 	plan skip_all => 'Connection to database failed, cannot continue testing';
 }
-plan tests => 96;
+plan tests => 97;
 
 isnt ($dbh, undef, 'Connect to database for statement handle method testing');
 
@@ -652,6 +653,36 @@
 $sth->fetchall_arrayref();
 is ($sth->{pg_current_row}, 0, $t);
 
+#
+# Test of the statement handle method "cancel"
+#
+
+$dbh->do('INSERT INTO dbd_pg_test (id) VALUES (?)',undef,1);
+$dbh->commit;
+$dbh->do('SELECT * FROM dbd_pg_test WHERE id = ? FOR UPDATE',undef,1);
+
+my $dbh2 = $dbh->clone;
+$dbh2->do('SET search_path TO ' . $dbh->selectrow_array('SHOW search_path'));
+
+my $oldaction;
+eval {
+	# This statement will block indefinitely becaue of the 'FOR UPDATE' clause,
+	# so we set up an alarm to cancel it after 2 seconds.
+	my $sth = $dbh2->prepare('SELECT * FROM dbd_pg_test WHERE id = ? FOR UPDATE');
+	$sth->{RaiseError} = 1;
+
+	my $action = POSIX::SigAction->new(sub {$sth->cancel},POSIX::SigSet->new(SIGALRM));
+	$oldaction = POSIX::SigAction->new;
+	POSIX::sigaction(SIGALRM,$action,$oldaction);
+
+	alarm(2); # seconds before alarm
+	$sth->execute(1);
+	alarm(0); # cancel alarm (if execute didn't block)
+};
+POSIX::sigaction(SIGALRM,$oldaction); # restore original signal handler
+like ($@,qr/canceling statement due to user request/,'cancel');
+$dbh2->disconnect();
+
 cleanup_database($dbh,'test');
 $dbh->rollback();
 $dbh->disconnect();