[DBD::Pg] Fix for named placeholders: we have to take into account both the length of the potential match and the length of the item we are searching for. Add more tests to confirm.

dbdpg-commits-gENoVmVU/[email protected] Mon, 12 Mar 2012 16:38:58 -0400
Newsgroups gmane.comp.db.postgresql.dbdpg.cvs
Message-ID <[email protected]>
Committed by Greg Sabino Mullane <greg-O9fzpki4YnJWk0Htik3J/[email protected]>

Fix for named placeholders: we have to take into account both the length of the potential match and the length of the item we are searching for. Add more tests to confirm.

---
 dbdimp.c           |    8 +++++++-
 t/12placeholders.t |   30 +++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/dbdimp.c b/dbdimp.c
index 175ed97..84070d7 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -1911,7 +1911,13 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
 			sectionsize = currpos-sectionstop;
 			/* Have we seen this placeholder yet? */
 			for (xint=1,thisph=imp_sth->ph; NULL != thisph; thisph=thisph->nextph,xint++) {
-				if (0==strncmp(thisph->fooname, statement-sectionsize, sectionsize + 1)) {
+				/*
+				  Because we need to make sure :foobar does not match as a previous 
+				   hit when seeing :foobar2, we always use the greater of the two lengths:
+				   the length of the old name or the current name we are scanning
+				*/
+				if (0==strncmp(thisph->fooname, statement-sectionsize,
+							   strlen(thisph->fooname) > sectionsize ? strlen(thisph->fooname) : sectionsize)) {
 					newseg->placeholder = xint;
 					newseg->ph = thisph;
 					break;
diff --git a/t/12placeholders.t b/t/12placeholders.t
index 1a89d91..e274243 100644
--- a/t/12placeholders.t
+++ b/t/12placeholders.t
@@ -17,7 +17,7 @@ my $dbh = connect_database();
 if (! $dbh) {
 	plan skip_all => 'Connection to database failed, cannot continue testing';
 }
-plan tests => 240;
+plan tests => 243;
 
 my $t='Connect to database for placeholder testing';
 isnt ($dbh, undef, $t);
@@ -121,6 +121,25 @@ eval {
 };
 is ($@, q{}, $t);
 
+## Same, but fiddle with whitespace
+$sql = q{SELECT pname FROM dbd_pg_test WHERE pname = :foobar AND pname = :foobar2 AND pname = :foobar2};
+eval {
+	$sth = $dbh->prepare($sql);
+	$sth->bind_param(':foobar', 123);
+	$sth->bind_param(':foobar2', 456);
+	$sth->execute();
+};
+is ($@, q{}, $t);
+
+$sql = q{SELECT pname FROM dbd_pg_test WHERE pname = :foobar AND pname = :foobar AND pname = :foobar2 };
+eval {
+	$sth = $dbh->prepare($sql);
+	$sth->bind_param(':foobar', 123);
+	$sth->bind_param(':foobar2', 456);
+	$sth->execute();
+};
+is ($@, q{}, $t);
+
 $t='Execute with repeated named placeholders works';
 $sql = q{SELECT pname FROM dbd_pg_test WHERE pname = :foobar AND pname = :foobar };
 eval {
@@ -130,6 +149,15 @@ eval {
 };
 is ($@, q{}, $t);
 
+## Same thing, different whitespace
+$sql = q{SELECT pname FROM dbd_pg_test WHERE pname = :foobar AND pname = :foobar};
+eval {
+	$sth = $dbh->prepare($sql);
+	$sth->bind_param(':foobar', 123);
+	$sth->execute();
+};
+is ($@, q{}, $t);
+
 $t='Prepare with large number of parameters works';
 ## Test large number of placeholders
 $sql = 'SELECT 1 FROM dbd_pg_test WHERE id IN (' . '?,' x 300 . '?)';
-- 
1.7.1