Use Placeholders!.
Instead of
writemy $sql = "INSERT INTO ERRORS (TSTAMP,CMND_ID,ALERT_ID,SEND_ID,DESTIN +ATION,MESSAGE,STATE,SVR_HOST) VALUES (\'$tstamp\',\'$cmnd_id\',\'$ale +rt_id\',\'$send +_id\',\'$destination\',\'$message\',\'$state\',\'$svr_host\')\;\n"; my $statement = $db_handle->prepare($sql) or die "Couldn't prepare query '$sql': $DBI::errst +r\n"; $statement->execute() or die "Couldn't execute query '$sql': $DBI::errst +r\n";
my $sql = qq{"INSERT INTO ERRORS (TSTAMP,CMND_ID,ALERT_ID,SEND_ID,DEST +INATION,MESSAGE,STATE,SVR_HOST) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?); my $statement = $dbh->prepare($sql); $statement->execute($tstamp, $cmnd_id, ...);
Also make sure to pass the {RaiseError =>1} argument to DBI->connect, then you don't need the custom error handlers.
Even more important: you can prepare the statements once outside the loop, and then call execute them inside the loops as many times as you need them. That way you'll gain much performance (if you have many iterations).
Instead of
you can first check if the line starts with SendCleared and then useif($line =~ /^SendCleared~(.*)~(.*)~(.*)~(.*)~(.*)~(.*)/){ my $tstamp = $1; my $cmnd_id = $2; my $alert_id = $3; my $send_id = $4; my $state = $5; my $svr_host = $6;
my ($tstamp, $cmnd_id, ...) = split m/~/, $line;
There are actually even better ways to handle the dispatch with a hash: you can store these sql queries in a has with keys SendOffDuty, AlertCmd and so on, and then do something like that:
# assume %q contains the quries my ($query, @args) = split $line, '~'; $q{$query}->execute(@args); # assuming the log files are well formed
In reply to Re: Need Critique (Good or Bad)
by moritz
in thread Need Critique (Good or Bad)
by onegative
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |