Bug #32868 for DBD-Pg: DBD-Pg-2.0.0_9: $dbh->quote(URI::URL->new('x')) error

Fri Feb 01 20:49:27 2008 cpan-070814 [...] bilteks.com - Ticket created

Subject:	DBD-Pg-2.0.0_9: $dbh->quote(URI::URL->new('x')) error
Date:	Sat, 2 Feb 2008 02:48:29 +0200
To:	bug-DBD-Pg [...] rt.cpan.org
From:	"Alexander Kirpa" <cpan-070814 [...] bilteks.com>

After upgrade DBD::Pg from DBD-Pg-1.49 to DBD-Pg-2.0.0_9 next script broken: use strict; use warnings; use DBI; use URI::URL; my $dbh=DBI->connect('DBI:Pg:dbname=test','postgres','', {RaiseError => 'on'}) or die "Cannot connect to DB" . $DBI::errstr; my $URI= URI::URL->new('x'); print "URI=[$URI]\n"; print 'quote=',$dbh->quote($URI),"\n"; -------- result from DBD-Pg-1.49 ---------- URI=[x] quote='x' -------- result from DBD-Pg-2.0.0_9 ------- perl z.pl URI=[x] Arrays must contain only scalars and other arrays at z.pl line 8. ------------------------------------------- Best regards, Alexander Kirpa

Sun Feb 03 19:15:02 2008 greg [...] turnstep.com - Correspondence added

From:

greg [...] turnstep.com

Show quoted text

> my $URI= URI::URL->new('x'); > print "URI=[$URI]\n"; > print 'quote=',$dbh->quote($URI),"\n"; > -------- result from DBD-Pg-1.49 ---------- > URI=[x] > quote='x' > -------- result from DBD-Pg-2.0.0_9 ------- > URI=[x] > Arrays must contain only scalars and other arrays at z.pl line 8.

While this may have worked in DBD-Pg-1.49, DBD-Pg-2.0 does the correct thing, in that $URI is not a string, and thus should not be quoted. For example, if the script above is changed to: my $URI= URI::URL->new('x'); print "URI=[$URI]\n"; use Data::Dumper; print Dumper $URI; It produces: URI=[x] $VAR1 = bless( [ bless( do{\(my $o = 'x')}, 'URI::_generic' ), undef ], 'URI::URL' ); The correct thing to do in this case is to quote the output of a method that supplies a stringified representation of the object. For examples: print 'quote=',$dbh->quote($URI->path),"\n";

Sun Feb 03 19:15:10 2008 The RT System itself - Status changed from 'new' to 'open'

Sun Feb 03 19:44:02 2008 dwheeler [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #32868] DBD-Pg-2.0.0_9: $dbh->quote(URI::URL->new('x')) error
Date:	Sun, 3 Feb 2008 16:43:31 -0800
To:	bug-DBD-Pg [...] rt.cpan.org
From:	"David E. Wheeler" <dwheeler [...] cpan.org>

On Feb 3, 2008, at 16:15, Greg Sabino Mullane via RT wrote: Show quoted text

>> my $URI= URI::URL->new('x'); >> print "URI=[$URI]\n"; >> print 'quote=',$dbh->quote($URI),"\n"; >> -------- result from DBD-Pg-1.49 ---------- >> URI=[x] >> quote='x' >> -------- result from DBD-Pg-2.0.0_9 ------- >> URI=[x] >> Arrays must contain only scalars and other arrays at z.pl line 8.

> > While this may have worked in DBD-Pg-1.49, DBD-Pg-2.0 does the > correct thing, in that $URI is not a string, and thus should not > be quoted. For example, if the script above is changed to: > > my $URI= URI::URL->new('x'); > print "URI=[$URI]\n"; > use Data::Dumper; > print Dumper $URI; > > It produces: > > URI=[x] > $VAR1 = bless( [ > bless( do{\(my $o = 'x')}, 'URI::_generic' ), > undef > ], 'URI::URL' );

Hrm. Doesn't it make sense to use the string overloading, if it's available, to quote Perl objects? Best, David

Sun Feb 03 20:00:32 2008 greg [...] turnstep.com - Correspondence added

Show quoted text

> Hrm. Doesn't it make sense to use the string overloading, if it's > available, to quote Perl objects?

I'm not sure I'm comfortable with DBD::Pg doing that be default. Seems to me it is the callers responsibility to provide what the DBI API specifies, a 'string literal'. Does anyone have other examples of trying to quote an object that used to work and should still?

Sun Feb 03 23:15:33 2008 dwheeler [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #32868] DBD-Pg-2.0.0_9: $dbh->quote(URI::URL->new('x')) error
Date:	Sun, 3 Feb 2008 20:15:15 -0800
To:	bug-DBD-Pg [...] rt.cpan.org
From:	"David E. Wheeler" <dwheeler [...] cpan.org>

On Feb 3, 2008, at 17:00, Greg Sabino Mullane via RT wrote: Show quoted text

> I'm not sure I'm comfortable with DBD::Pg doing that be default. Seems > to me it is the callers responsibility to provide what the DBI API > specifies, a 'string literal'. Does anyone have other examples of > trying > to quote an object that used to work and should still?

I think that this is a DBI standard: use strict; use warnings; use DBI; use URI::URL; my $dbh=DBI->connect('DBI:ExampleP:dummy','','', { RaiseError => 'on' }) or die "Cannot connect to DB" . $DBI::errstr; my $URI= URI::URL->new('x'); print "URI=[$URI]\n"; print 'quote=',$dbh->quote($URI),"\n"; Outputs: URI=[x] quote='x' So if we want to be compliant with how the DBI does it by default, we should probably pay attention to double-quoted string operator overloading. At least, I assume that's what the DBI is doing. Best, David

Tue Feb 05 00:20:40 2008 greg [...] turnstep.com - Correspondence added

Show quoted text

> So if we want to be compliant with how the DBI does it by default, we > should probably pay attention to double-quoted string operator > overloading. At least, I assume that's what the DBI is doing.

I think it's just a side effect of the default way that DBI does it - by simply treating anything passed in as a string and wrapping quotes around it. The URI::URL example is not a great example - look what happens when we feed $dbh or an XML::Parser object or other things in: ... print 'quote=',$dbh->quote($dbh),"\n"; use XML::Parser; my $xml = XML::Parser->new(); print 'quote=',$dbh->quote($xml),"\n"; print 'quote=',$dbh->quote(\[1,2,3]),"\n"; print 'quote=',$dbh->quote(*STDIN),"\n"; gives: quote='DBI::db=HASH(0x81767e4)' quote='XML::Parser=HASH(0x83e2018)' quote='REF(0x8530f2c)' quote='*main::STDIN' I submit it is better to force the user to actually give us a string, rather than try to coerce we-dont-know-what into one. Principle of least surprise, and safer for things that will more than likely get stuck into the database in the next few lines of code.

Tue Feb 05 00:38:31 2008 dwheeler [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #32868] DBD-Pg-2.0.0_9: $dbh->quote(URI::URL->new('x')) error
Date:	Mon, 4 Feb 2008 21:38:02 -0800
To:	bug-DBD-Pg [...] rt.cpan.org
From:	"David E. Wheeler" <dwheeler [...] cpan.org>

On Feb 4, 2008, at 21:20, Greg Sabino Mullane via RT wrote: Show quoted text

> I think it's just a side effect of the default way that DBI does it > - by > simply treating anything passed in as a string and wrapping quotes > around it. The URI::URL example is not a great example - look what > happens when we feed $dbh or an XML::Parser object or other things in: > > ... > print 'quote=',$dbh->quote($dbh),"\n"; > use XML::Parser; > my $xml = XML::Parser->new(); > print 'quote=',$dbh->quote($xml),"\n"; > print 'quote=',$dbh->quote(\[1,2,3]),"\n"; > print 'quote=',$dbh->quote(*STDIN),"\n"; > > gives: > > quote='DBI::db=HASH(0x81767e4)' > quote='XML::Parser=HASH(0x83e2018)' > quote='REF(0x8530f2c)' > quote='*main::STDIN'

That's because those objects do not have stringification overloaded. Show quoted text

> I submit it is better to force the user to actually give us a string, > rather than try to coerce we-dont-know-what into one. Principle of > least > surprise, and safer for things that will more than likely get stuck > into > the database in the next few lines of code.

I submit that if you have "" overloaded, then you can be effectively passing in a string. But in the end, it's a minor thing. I'd always make it explicit, myself. But maybe ask for feedback on dbi-dev@perl.org? Best, David

Tue Feb 05 09:13:31 2008 greg [...] turnstep.com - Correspondence added

From:

greg [...] turnstep.com

Ah, I think you are right, I was getting confused because URI::URL returns an overloaded array (grumble, grumble, standard HASH ref convention). I just made a change in r10694. Not sure that it handles utf8-ness or stringified nulls correctly, but it at least does the original poster's expected behavior for the test case. An interesting thread for the archives: http://groups.google.de/group/perl.perl5.porters/msg/eb4d556b2b048b02

Thu Feb 21 11:24:32 2008 greg [...] turnstep.com - Fixed in 2.0.0 added

Thu Feb 21 11:24:32 2008 greg [...] turnstep.com - Broken in 1.49 added

Thu Feb 21 11:24:33 2008 greg [...] turnstep.com - Status changed from 'open' to 'resolved'