Beefy Boxes and Bandwidth Generously Provided by pair Networks
No such thing as a small change
 
PerlMonks  

DBI and finish

by wind (Priest)
on Jun 26, 2014 at 07:53 UTC ( [id://1091303]=perlquestion: print w/replies, xml ) Need Help??

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

Is it really THAT bad to call $sth->finish in legacy code?

As per the documentation for DBI->finish:

$rc  = $sth->finish;

Indicate that no more data will be fetched from this statement handle before it is either executed again or destroyed. You almost certainly do not need to call this method.

Adding calls to finish after loop that fetches all rows is a common mistake, don't do it, it can mask genuine problems like uncaught fetch errors.

When all the data has been fetched from a SELECT statement, the driver will automatically call finish for you. So you should not call it explicitly except when you know that you've not fetched all the data from a statement handle and the handle won't be destroyed soon.

Now, I get how it's not needed, but the warnings in this documentation appear a bit hyperbolic. Are these warnings exaggerated? Or is legacy code that still includes finish calls as an idiomatic way to communicate intent actually a “mistake” instead of merely wasted effort?

This only came up because of a comment on stackoverflow where I helped someone simplify their code by suggesting they loop instead of manually calling prepare, execute, finish for every SQL statement. I therefore researched for more info on this, but was only able to find the following resources:

Anyway, I aim to use DBIx::Class and other constructs instead of manual calls to DBI, so this rarely comes up for me. However, I would like to understand the actual risk of needlessly calling finish, or if this is just an effort to get people to streamline their code?

Replies are listed 'Best First'.
Re: DBI and finish (risk of calling finish twice)
by Anonymous Monk on Jun 26, 2014 at 09:34 UTC

    Now, I get how it's not needed, but the warnings in this documentation appear a bit hyperbolic. Are these warnings exaggerated?

    Nope, sounds rather tame :)

    Or is legacy code that still includes finish calls as an idiomatic way to communicate intent actually a “mistake” instead of merely wasted effort?

    Its both :) its also neither :)

    Its definitely not "idiomatic way" since the DBI docs tell you don't; Its part of the public API, the drivers have to implement it, but there isn't a single usage shown in the examples in DBI.pm, and you don't need or want it 99/100, so anytime it shows up its 99/100 a mistake , typically cargo-cult-ed

    Also, documenting intent, what intent? That you're finished with the statement handle? I think undef $sth; documents that more clearly and universally :)!

    However, I would like to understand the actual risk of needlessly calling finish, or if this is just an effort to get people to streamline their code?

    The risk is an error message gets lost (esp if you're not using RaiseError); Calling finish modifies $sth attributes ... https://metacpan.org/source/TIMB/DBI-1.631/DBI.xs, https://metacpan.org/source/TIMB/DBI-1.631/Driver.xst, https://metacpan.org/source/ISHIGAKI/DBD-SQLite-1.42/dbdimp.c

    I don't think its an effort to get people to streamline their code :) Because

    Its kind of like calling  close $fh; except that it isn't

    Its more kind of like calling  close $fh; close $fh; ... the $! gets obliterated

    Its not unlike calling  $foo->DESTROY when  undef $foo; will do that

    Its like using opendir/readdir/closedir ... its lower level interface you don't need most of the time

    Possible use case for finish, is user clicks cancel button in a gui, there are still many results left, so program calls finish ... but in reality 99/100 you'd simply destroy undef $sth ... and you'd be handling errors with RaiseError... not manually checking

    Yes, I've frequently written copy/pasted  $sth->finish even when all I really wanted was  $sth->commit; and  undef $sth; would have (probably) accomplished both (:IIRC...:); I'm not afraid to cargo-cult :)

      Actually, calling $sth->finish becomes important only on the cached handles when not all of the data was fetched. Otherwise, next "execute" on that handle may result in error. Though it all depends on the actual driver. Many (or is it all now?) drivers automatically do "finish" for the handles that are "executed" again. So, if one works with such driver, then one does not need "finish" at all.

      BTW, the above behavior of the drivers potentially masks errors of simultaneous use of the same handle :) YMMV.

      As to "dangers" of using "finish", this might be exaggerated. After all, one should check for errors right after the call to "fetch", if this is not done, then clearing that error state later by "finish" does not play any role. Mostly, these "finish" calls are just useless. IMHO.

      BTW. Doing "undef $sth" on statement with active transactions results in ROLLBACK, not COMMIT. At least this is what I was getting with SQLite :)

        ... cached ... Though it all depends on the actual driver ...

        I doubt that :) See inside DBI.pm look for "sub prepare_cached"

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://1091303]
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others cooling their heels in the Monastery: (5)
As of 2024-03-28 10:32 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found