Use Placeholders!.

Instead of

my $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";
[download]

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, ...);
[download]

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

if($line =~ /^SendCleared~(.*)~(.*)~(.*)~(.*)~(.*)~(.*)/){
                my $tstamp = $1;
                my $cmnd_id = $2;
                my $alert_id = $3;
                my $send_id = $4;
                my $state = $5;
                my $svr_host = $6;
[download]

my ($tstamp, $cmnd_id, ...) = split m/~/, $line;
[download]

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
[download]

All your

if ($line =~...

...blocks are very similar.

I would probably suggest not using the heavy regular expressions as you are doing, but rather using the split operator. And I wouldn't treat the first token in the line any differently from the rest. I'd also be lazier than you about picking out each field by name. Something like this:

my ($type,@tokens)=split(/\~/,$line);

if ($type eq "SendFiltered")
{
  my ($tstamp,$cmnd_id,$alert_id,$send_id,$state,$svr_host)
   = @tokens[0-5];

  my $sql = "INSERT ...
             
}
elsif ($type eq "AlertError")
{
   ...
}
[download]

However even that isn't really sufficiently lazy. The SQL is still there, and it's still ugly. (This isn't your fault - SQL is just plain ugly.)

It seems to me that since you're not doing anything at all with all of these values except sticking them in the SQL statements, you don't actually need to bother naming them at all. Really all you need to know for each "first token" type is what the SQL statement should be, and which column numbers to plug in.

With that in mind, you could do something like this:

# %statements is an array of prepared SQL statement objects ready to h
+ave values plugged in

%statements=(
  "AlertError"=>$dbh->prepare("INSERT INTO ERRORS (TSTAMP,CMND_ID,ALER
+T_ID,SEND_ID,DESTINATION,MESSAGE,STATE,SVR_HOST) VALUES (?,?,?,?,?,?,
+?,?)"),
  "SendCleared"=>$dbh->prepare("UPDATE SENDS..."),
  "...
);

# %columns is an array of column position indicators corresponding to 
+the %statements above

%columns=(
 "AlertError"=>[0,1,2,3,4,5],
 "SendCleared"=>[...
);
[download]

Then your actual executable code becomes quite easy...

while ($line=...)
{
  my ($type,@tokens)=split(/\~/,$line);
  $statements{$type}->execute(@tokens[@{$columns{$type}}]);
}
[download]

There are various other ways you could organise the data, including using OOP. The key point is that the SQL statements and the column mappings really are data, not code, from the point of view of your Perl program.

#!/usr/local/bin/perl -w

use strict;
use DBI;
use File::Tail;

# query subs, fetch and name the parameters and call the prepared stat
+ement
sub insert_alerts
{
    my ($sth, $tstamp, $cmnd_id, $cmnd , $client_host, $client_user, $
+svr_host, $svr_host) = @_;
    
    $svr_host =~ s/\..*//g;
    $sth->execute( $tstamp, $cmnd_id, $cmnd , $client_host, $client_us
+er, $svr_host, $svr_host );
    
    return $DBI::errstr ? 0 : 1;
}

sub insert_filters
{
    my ($sth, $tstamp, $cmnd_id, $cmnd , $client_host, $client_user, $
+svr_host, $svr_host) = @_;
    
    $svr_host =~ s/\..*//g;
    $sth->execute( $tstamp, $cmnd_id, $cmnd , $client_host, $client_us
+er, $svr_host, $svr_host );

    return $DBI::errstr ? 0 : 1;
}

sub insert_sends
{
    my ($sth, $tstamp, $cmnd_id, $alert_id, $send_id, $user, $destinat
+ion, $pin, $to, $message, $state, $svr_host) = @_;
    
    $svr_host =~ s/\..*//g;
    $sth->execute( $tstamp, $cmnd_id, $alert_id, $send_id, $user, $des
+tination, $pin||$to, $message, $state, $svr_host, 'TRUE' );
    
    return $DBI::errstr ? 0 : 1;
}

sub update_sends
{
    my ($sth, $tstamp, $cmnd_id, $alert_id, $send_id, $state, $svr_hos
+t = $6) = @_;

    $svr_host =~ s/\..*//g;
    $sth->execute( $tstamp, $state, $cmnd_id, $alert_id, $send_it, $sv
+r_host );
    
    return $DBI::errstr ? 0 : 1;
}

sub insert_errors
{
    my ($sth, $tstamp, $cmnd_id, $cmnd , $client_host, $client_user, $
+svr_host, $svr_host) = @_;
    
    $svr_host =~ s/\..*//g;
    $sth->execute( $tstamp, $cmnd_id, $cmnd , $client_host, $client_us
+er, $svr_host, $svr_host );
    
    return $DBI::errstr ? 0 : 1;
}

# Create File::Tail object
my $file = File::Tail->new("/opt/app/superspy/tmp/commLogger.log");

# Create DB connection
my $dbh = DBI->connect("dbi:mysql:database=P0TA6E01;host=myhost02.fcc.
+hide.com:3306;user=xxxxxxxx;password=xxxxxxxx") || die "Couldn't conn
+ect to database: $DBI::errstr\n";

# dispatch table and prepared statenents for the logfile entries
# this whill be faster than preparing the query for each entry
my %dispatch =
(
    AlertCmd      => [ \&insert_alerts,  $dbh->prepare('INSERT INTO AL
+ERTS (TSTAMP,CMND_ID,CMND,CLIENTHOST,CLIENTUSER,SVR_HOST) VALUES (?,?
+,?,?,?,?);' ],
    SendFiltered  => [ \&insert_filters, $dbh->prepare('INSERT INTO FI
+LTERS (TSTAMP,CMND_ID,CMND,CLIENTHOST,CLIENTUSER,SVR_HOST) VALUES (?,
+?,?,?,?,?);' ],
    SendOffDuty   => [ \&insert_filters, $dbh->prepare('INSERT INTO FI
+LTERS (TSTAMP,CMND_ID,CMND,CLIENTHOST,CLIENTUSER,SVR_HOST) VALUES (?,
+?,?,?,?,?);' ],
    SendStarted   => [ \&insert_sends,   $dbh->prepare('INSERT INTO SE
+NDS (START_TIME,CMND_ID,ALERT_ID,SEND_ID,USER,DESTINATION,ADDRESS,MES
+SAGE,STATE,SVR_HOST,ACTIVE) VALUES (?,?,?,?,?,?,?,?,?,?,?)' ],
    SendCompleted => [ \&update_sends,   $dbh->prepare('UPDATE SENDS S
+ET END_TIME=?, STATE=?,ACTIVE=FALSE WHERE CMND_ID=? AND ALERT_ID=? AN
+D SEND_ID=? AND SVR_HOST=?;' ],
    SendCleared   => [ \&update_sends,   $dbh->prepare('UPDATE SENDS S
+ET END_TIME=?, STATE=?,ACTIVE=FALSE WHERE CMND_ID=? AND ALERT_ID=? AN
+D SEND_ID=? AND SVR_HOST=?;' ],
    AlertError    => [ \&insert_errors,  $dbh->prepare('INSERT INTO ER
+RORS (TSTAMP,CMND_ID,ALERT_ID,SEND_ID,DESTINATION,MESSAGE,STATE,SVR_H
+OST) VALUES (?,?,?,?,?,?,?,?)' ],
);

# read linewise
while (my $line = $file->read))
{
    chomp($line);

    # skipt reload
    next
        if $line =~ /^AlertCmd~.*~.*~-reload.*/;

    # check for interesting entries, save keyword and the rest in $1 a
+nd $2
    if ( $line =~ /^(AlertCmd|SendFiltered|SendOffDuty|SendStarted|Sen
+dCompleted|SendCleared|AlertError)~(.+)/ )
    {
        # the callback and the prepared statement from the dispatch ta
+ble that are appropriate for the entry
        my ($function, $sth) = @{ $dispatch->{$1} };
        
        # call the function and show error if something went wrong
        unless ( $function->( $sth, split /~/, $2 ) )
        {
            print "Error ($DBI::errstr) while processing this line:\n$
+line\n\n";
        }
    }
}
[download]

Hey Holli,
A picture is worth a thousand words...thanks for the refactor it explains the use of these concepts and I follwed all parts of the code. I will definately use these concepts in the future...
Many Thanks to Everyone,
Danny

• You can define $sql once, at the top, and have a single

       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";
  [download]

  after the conditionals if you have a non-null $sql.
• The general flow of logic will be more evident