DBI, rows, and do

drrngrvy has asked for the wisdom of the Perl Monks concerning the following question:

Salutations sooth bearers,

I have a problem, which I'm nearly certain is one of oversight, but I'm a perl novice and apparently going blind so I'd really appreciate some quick advice.

The setup involves modularised dbi routines to delete a row from MySQL. I've managed to work (read: blunt trauma) my head around problems I had with all other types of SQL calls and dereferencing results, but I feel like DBI.pm has some abtuse, unspoken rule used to force us into the celebate life.

Anyway, the (edited) code:

sub remove {
    ( my @dat = @_ ) =~ /^\d+$/;
    my $statement = qq~DELETE FROM `gates-post` WHERE id = "$dat[1]"~;
    get( "affected" );
    return( undef ) unless( $rows > 0 );

sub get {
    my $opt = shift;
    ( $rows, @rtn ) = '';
    $dbh = DBI->connect( gates->url, gates->accessName, gates->passs, 
+{    AutoCommit => 1, RaiseError => 1 }) || die "DB connect failed: "
+ . $dbh->errstr;
.
.
.
    if ( $opt eq "affected" ) {
        $rows = $dbh->do( $statement ) or die "DB operation failed to 
+affect: " . $dbh->errstr;
        return;
    }
[download]

The problems:
Basically, nothing is deleted. However, I know that the statement is formatted properly (and will delete the right row if I stick it through phpMyAdmin for instance); I know that $rows (global var) is returned and has value 1; I'm also sure that the right method 'do' is called. I've tried the extended version of do but since it works for the other calls to it I don't understand why it pretends to succeed for DELETE statements.

I can't get the local 'ward' out of the corner of my eye, so any help would be much appreciated (as would any unrelated 'criticism', of course).

Cheers.

Comment on DBI, rows, and do Download Code

Replies are listed 'Best First'.
Re: DBI, rows, and do by jZed (Prior) on Jun 05, 2005 at 22:19 UTC
You must check for rows() > 0, not for truth of rows(). As per the docs, rows() returns the number of rows impacted or '0E0' (zero but true) if no rows were impacted or -1 for unknown number of rows. It only returns undef if the statement failed. A DELETE that finds no rows to delete is not a failure.	[reply]
Re: DBI, rows, and do by davidrw (Prior) on Jun 05, 2005 at 22:27 UTC
Are you sure the DSN is right? i.e., are you able to use $dbh for anything else? Since the do() returns 1, it sounds like a transaction issue, but i see that you have `AutoCommit => 1` in there.. a blind stab, but try `$dbh->commit` after the do() ... As for side notes, a couple things came to mind: Try to always use placeholders w/DBI: my $sql = qq~DELETE FROM `gates-post` WHERE id = ?~; my @bind = ( $dat[1] ); $dbh->do($sql, {}, @bind); [download] SQL::Abstract may be of interest to avoid writing your own SQL Same with Class::DBI -- treat your rows as objects and never write any sql -- just a few lines of CDBI code. (The last two may not be needed/appropriate in your case, but definitely very good tools to be aware of.) Update: I just read jZed's post, which has the very good point about $rows .. I guess i assumed it was 1 -- you should print it to confirm if it's 1 or 0E0 .. if the latter, then i would turn attention to $dat1 and confirm that it really has the desired value..	[reply] [d/l] [select]
Re: DBI, rows, and do by jpeg (Chaplain) on Jun 06, 2005 at 00:24 UTC
Just a troubleshooting tip: When a query works through phpMyadmin or the mysql client but not through DBI, you should check and see what your code is sending to mysql. in the [mysqld] section of my.cnf, add log = query_log and a plaintext log of connections and sql queries will be logged. (don't enable the query_log on production servers.) HTH -- jpg	[reply]
Re: DBI, rows, and do by Thilosophy (Curate) on Jun 06, 2005 at 00:10 UTC
I cannot parse your code ... Where is the closing bracket of `remove`? Is `get` defined inside of `remove`? If yes, that is unusual, but if no, how is `$statement` passed into `get`? Not using global variables to return data from subroutines (such as `$rows`) would really help, I think. Are you using strict and warnings?	[reply] [d/l] [select]
Re^2: DBI, rows, and do by holli (Abbot) on Jun 06, 2005 at 07:41 UTC
I cannot parse your code ... That is because the only thing that can parse Perl is perl. Sorry, couldn't restist :-P holli, /regexed monk/	[reply]
Re: DBI, rows, and do by drrngrvy (Novice) on Jun 05, 2005 at 22:43 UTC
Thanks for the help, but still no luck. One thing I didn't make clear was that both the subs remove and get are in a module seperate of the main one. Remove is called externally and unless it returns true, the main script reacts. The row at the end of that sub makes sure that unless $rows>0 it returns undef and causes the external script to fail (unless I made a drastic error there, although the same script works fine with the 10 other calls to DBI). Before I came here I had checked (ie. got proof of) as much as I could. $rows is definitely 1, $dat1 is the id I need and the exact same method works for INSERT and UPDATE calls properly so I don't think that's part of the problem. Advice taken about placeholders. Is the point so that you don't need to prepare every call?	[reply]
Re^2: DBI, rows, and do by davidrw (Prior) on Jun 05, 2005 at 23:00 UTC
hmm.. ok, the working for INSERT and UPDATe throws out my previous thoughts.. how is $rows declared? i see that it is a global.. maybe it's a scope issue? Can you (at least to verify this) have get() do `return $rows;` and have remove() do `my $rows = get("affected");` ? As for placeholders, being able to prepare calls for re-use is one advantage.. others include (perhaps best one last): legibility/maintenance of SQL -- not cluttered up with perl variables clarity of variables use -- seeing `my @bind = ($x, $y, $z);` is much eaiser to see and understand than mentally finding the $'s in the (esp if large) SQL statement. quoting/espacing -- DBI will take care of this for you, and save many headaches of dealing with backslashes/single quotes..	[reply] [d/l] [select]
Re: DBI, rows, and do by tlm (Prior) on Jun 06, 2005 at 00:45 UTC
What are you trying to do with `( my @dat = @_ ) =~ /^\d+$/;` [download] ? AFAICT, that regex is being matched against the size of `@_`, but you don't do anything with the results of the match (unless you use something like an evil $& in a part of the code that is not shown). the lowliest monk	[reply] [d/l]
Re: DBI, rows, and do by drrngrvy (Novice) on Jun 05, 2005 at 23:30 UTC
If placeholders sort out quoting issues, then surely my trek to the mountain was fruitful! Heh. As for localising $rows, I considered it (although not specifically for this problem), but didn't try it because I was too lazy/petrified to go through the whole module fixing it, but I did it now and still no luck. $rows is still returning 1 to. Bah.	[reply]
Re: DBI, rows, and do by trammell (Priest) on Jun 06, 2005 at 01:59 UTC
The line(s) `$dbh = DBI->connect(...) \|\| die "DB connect failed: " . $dbh->errstr;` [download] won't print anything useful upon failing to create `$dbh`, since you can't in this case call a method on an object whose construction failed. You probably want something more like: `$dbh = DBI->connect(...) \|\| die "DB connect failed: ", DBI->errstr;` [download] Update: jZed++!	[reply] [d/l] [select]
Re^2: DBI, rows, and do by jZed (Prior) on Jun 06, 2005 at 02:11 UTC
You're right in general, but in this case yours wouldn't do anything either :-). Since the OP included RaiseError=>1 in the connect statement, the connect, if it fails, will print the errstr and die before it reaches the or part of the statement.	[reply]
Re: DBI, rows, and do by drrngrvy (Novice) on Jun 06, 2005 at 02:31 UTC
Wow, I tried to explain myself properly, but I just realised how clumsily I did it. It's a good thing that you (all) pointed it out though because the problem was solved (mostly). I had `$statement` as a global var, but accidentally only assigned it `my` in `remove`. Thanks for pointing that out Thilosophy... I don't suppose you make the monocles too? That missing curly was there on the original. I'm still not sure why DBI::do() would return a 1 if is trying to do a delete without a statement though. I'll go poke about a bit more. t1m: I hoped that the line you quoted first assigned `@_` to `$id` (which it does) and then matched `$id` to the regex. It was to so the module (hopefully) isn't abused. Do I need to separate it onto two lines to do that? jpeg, trammell and jZed, your help is much appreciated. bows piously	[reply] [d/l] [select]