You should be preparing your "updating" statement with placeholders, and then executing within your foreach loop. That's (1) more secure, and (2) faster.

That's just one "off the top of my head" area where you can improve.

Thanks for your reply Dave.

But I'm sorry, I dont follow...
You're talking about this section??


# Do the updating...
foreach (keys (%Data)) {
  $dbh->do(" update new_table set newData=\"$Data{$_}\" where newID=$_
+ ");
} # end-foreach
[download]


This is the bit I dont follow...


preparing your "updating" statement with placeholders
[download]


Cheers,
Reagen

Yes, in that section, it should be more like this:

my $sth = $dbh->prepare("update new_table set newData=? where newID=?)
+;
foreach ( keys %Data ) {
    execute( $Data{$_}, $_ );
}
[download]

The idea is that you're preparing the update statement only once, so the database's SQL parser only has to examine it one time. Then as you execute it, the same statement is executed over and over again with the appropriate values inserted via the placeholders.

That snippet is untested, and just from memory, so do test it and check the docs. There's the DBI quote() method that does proper quoting for you.

See DBI for more info.




Dave

This is untested, but there are comments you may find helpful in the code.

#!/usr/bin/perl

use warnings;
use strict;
use DBI;

my $dbh;

#A lexical scope so that the db... variables go out of scope quickly.
{
        my $dbname = "database";
        my $dbuser = "user";
        my $dbpass = "password";
        my $dbhost = "localhost";
        my $dbport = 3306;
        my $dsn    = "DBI:mysql:database=$dbname;host=$dbhost;port=$db
+port";
        $dbh = DBI->connect($dsn, $dbuser, $dbpass, {  RaiseError => 1
+ })
                or die "Couldn't connect to the database: $!\n";
}


#Use placeholders here. Your DBD driver will then take care of
#appropriate quoting.
my $sth = $dbh->prepare("SELECT ID, Data FROM table_name WHERE Data=?"
+);
$sth->execute("value");


#You're executing the update query lots of times.
#Prepare it once, then execute it with different values.

my $update_sth = $dbh->prepare("UPDATE new_table SET newData=? WHERE n
+ewID=?");

#No point doing the looping over your DataHash twice, you've got the d
+ata
#once, why not use it here?
while(my ($id, $data) = $sth->fetchrow) {
        $update_sth->execute($data, $id);
}

#I never bother with these if it's at the end of a script personally.
#They'll happen automatically when the variables get DESTROY'ed
$sth->finish;
$update_sth->finish;
$dbh->disconnect;
[download]

# I never bother with these if it's at the end of a script personally. They'll happen automatically when the variables get DESTROY'ed

Have you ever actually tested those assumptions? Certainly on my system which uses RH 7.3 Perl 5.6.2, and the latest DBI and DBD::msyql with MySQL 4.0.xx if you fail to disconnect your connections will hang around until they either time out (8 hours default) or you run out of connections.

cheers

tachyon

Have you ever actually tested those assumptions?

No, I thought I'd read it. Good catch tachyon, cheers!
Sound of davis scrabbling furiously after some production code waving his hands...
Update: Ah, the bit I've read was about Apache::DBI overloading the disconnect methods. Either way, I was wrong to say what I said, and the finish/disconnect calls ought to be included

davis

It's not easy to juggle a pregnant wife and a troubled child, but somehow I managed to fit in eight hours of TV a day.

Ok them heres a question for you! Why bloody not? Unless I'm missunderstanding something, nobody can use these connections, right? So their hanging around has absolutely no purpose. Is there any point in not having them disconnect at the END?

Thanks davis.

That clears up a bit of my previous confusion as well.
I'll give it a go.

Cheers,
Reagen

Probably a paste-o from trimming your real code, but you don't declare or populate %Data anywhere before you loop over its keys. Your stricture would have caught that in running code.

The best opportunity for a speed increase I see is to follow davido's advice and use placeholders. Your UPDATE can be defined once, outside the loop, by saying

my $usth->'update new_table set newData=? where newID=?';
foreach (keys (%Data)) {
    $usth->execute( $Data{$_}, $_ );
);
[download]

You should check for errors each time you go to the database server. You can ease the pain of that by setting RaiseError=>1 in the connect call.

The biggest source of slowness in DBI is usually just grabbing too much data at once. Refining your demands will help that most.

If you want to understand what you are trying to do, rather than following blindly some exotic example, there is something for you in Tutorials, about using the DBI at its best:

• Speeding up the DBI
• DBI recipes

HTH

Thanks cchampion, i've had a look and learned something new already :)
Cheers,
Reagen

I could be wrong but it looks like you are running a select where the colum 'Data' = 'value', and then looping threw and storing the values of the Data colum.. Won't all the values of 'Data' be 'value' since thats what you queried for?

my %DataHash;
#get all values where the 'Data' colume = 'value'...?
my $sth = $dbh->prepare(" select ID, Data from table_name where Data=\
+"value\" ");
$sth->execute;
while ( my ( $ID, $Data) = $sth->fetchrow ) {
  $CountryDisplay{$ID} = "$Data";  # storing Data colum
} # end-while
$sth->finish
[download]

I don't see the point in this whole loop in that case. I dunno if this was just because of the way you made the script smaller or what but I thought i would point it out.

Hey Eric,

I was just using "value" as a descriptive term for what would be a variable.

Cheers,
Reagen