Re: SQL query using elements from array

[kennethk]

Rather than interpolating values directly into your SQL, you should probably get into the habit of using placeholders. In addition to improving security, it handles escaping for you. It also lets you only prepare your statement once, and use it multiple times. By moving that and your connect statement outside the loop, you'll likely find my rewrite runs substantially faster.

#!/usr/bin/perl
use DBI;
use strict;


  my @date1 = ('2013-08-01 00:00:00.000','2013-08-02 00:00:00.000');
  my @date2 = ('2013-08-02 00:00:00.000','2013-08-03 00:00:00.000');
  
my $dbh = DBI-> connect('dbi:ODBC:DSN=TEST-DB1;UID=SA;PWD=pass') or di
+e "CONNECT ERROR! :: $DBI::err $DBI::errstr $DBI::state $!\n"

my $sql2 = <<EOSQL;
SELECT Login_ID, AuditChrt_TimeStamp, Patient_ID
FROM TopsData.dbo.AUDT_AuditChrt
WHERE master.dbo.fn_sqlvarbasetostr(AuditChrt_TimeStamp) >= ?    
AND master.dbo.fn_sqlvarbasetostr(AuditChrt_TimeStamp) <= ?
EOSQL
my $sth = $dbh->prepare($sql2);

foreach my $i (0 .. @date1 - 1) {
    $sth->execute($date1[$i], $date2[$i]);
    my @row;
    while (@row = $sth->fetchrow_array) {  # retrieve one row at a tim
+e
        print join(", ", @row), "\n";
    }
} 

END {
    $dbh->disconnect if $dbh;
}

Other changes I made:

• I used a heredoc (Quote and Quote like Operators in perlop) to separate the SQL from the Perl code; IMHO, it makes things more legible

• I swapped to a foreach loop, since that structure is less bug prone than C-style loops -- fewer moving parts.

• I moved the disconnect to an END block (with conditional) so that no matter how your code exits, you won't get that pesky Issuing rollback warning.

Update: Corrected copy&paste bug, as per McA's comment below. I concur that an AoA is a better data structure here, but wanted to not get too far too fast...

[McA]

Hi

you've a little Copy&Paste bug in your code. After pointing out that placeholder variables are the better solution (++) you missed changing the timestamp variables to strings without single quotation mark which is only necessary when you do string interpolation while generating valid SQL statements:

my @date1 = ('2013-08-01 00:00:00.000','2013-08-02 00:00:00.000');

I would go one step further for clarification of the intention of the code. Why not doing the following to emphasis on the fact that the author wants to interate over several time intervals:

my @intervals = (
    ['2013-08-01 00:00:00.000', '2013-08-02 00:00:00.000'],
    ['2013-08-02 00:00:00.000', '2013-08-03 00:00:00.000'],
);
...
foreach my $interval (@intervals) {
    $sth->execute(@$interval);
...
}

And as soon as you look at the intervals and at the SQL statement you see that AllPaoTeam made a litte mistake concerning interval boundaries IMHO. Make one end of the interval inclusive the other end exclusive:

WHERE master.dbo.fn_sqlvarbasetostr(AuditChrt_TimeStamp) >= ?    
AND master.dbo.fn_sqlvarbasetostr(AuditChrt_TimeStamp) < ?

otherwise you would count entries on 2013-08-02 00:00:00.000 twice (except for being intended).

Regards
McA

[AllPaoTeam]

True, I did make that error, nice catch! Thanks to everyone with help me with this, Kudos to all!!