CODEWITHSUNDEEP
perl
prgrm
prgrm

Reputation: 3833

Perl won't execute SQL statement in a while loop

I wrote this piece of code: it is supposed to keep checking names until it finds one that does not exist, and then it should go on.

 while ($repeating == 1) {
            $new_name = $i . "_" . $file;
            my $sql= "SELECT file_name FROM PDFdocument WHERE user_id = '$id' AND file_name = '$new_name' ";
            my $sth = $dbh->prepare($sql);
            $sth->execute();
            while (my @row = $sth->fetchrow_array) {
                  //never enters here
                if ($new_name ne $row[0]) {

                    $repeating = 0;

                }
            }
            $i++;
        }

It never enters the second while loop, so it gets stuck in this repeating loop. I don't know why it does not work; I do some other sql statements before and they all work. This is the only one that does not work.

Any help?

Upvotes: 0

Views: 386

Answers (2)

ikegami
ikegami

Reputation: 385655

When you finally find the $i you should use, $sth->fetchrow_array returns an empty list, so = returns 0, so the loop isn't entered.

Solution 1:

my $new_name;
for (my $i=1; ; ++$i) {
   $new_name = $i . "_" . $file;
   $dbh->selectrow_arrayref(
      "SELECT 1 FROM `PDFdocument` WHERE `user_id` = ? AND `file_name` = ?",
      undef,
      $id, $new_name,
   )
      and last;
}

Solution 2:

my $i = $dbh->selectrow_array(
   "
      SELECT CAST(LEFT(`file_name`, LOCATE("_", `file_name`)-1) AS INT) AS `i`
        FROM `PDFdocument`
       WHERE `user_id` = ?
         AND `file_name` LIKE ?
       ORDER BY DESC `i`
       LIMIT 1
   ",
   undef,
   $id, "%\\_\Q$file\E"
);
++$i;
my $new_name = $i . "_" . $file;

Note the use of placeholders. Your buggy way of building the SQL statement leaves you vulnerable to malfunctions if not attacks.

Upvotes: 2

janh
janh

Reputation: 2972

The problem is that you won't get any rows back if the name does not exist. The solution is to just check if you do get any rows - otherwise the file name mus be unused. BTW, let DBI escape stuff you sent to the database. This should work:

while ($repeating == 1) {
        $new_name = $i . "_" . $file;
        # the question marks are placeholders
        my $sql= "SELECT file_name FROM PDFdocument WHERE user_id = ? AND file_name = ? ";
        my $sth = $dbh->prepare($sql);
        # filling the placeholders while executing
        $sth->execute($id, $new_name); 
        if(!$sth->fetch) {
        # no rows found? this name must be fresh
              $repeating = 0;
        }
        $i++;
    }

Edit: As @ikegami mentioned in a comment, the behavior of $sth->rows depends on the driver, so it might return different values for different database engines when dealing with SELECT statements (see also the DBI docs. Asking the driver to fetch a row should work the same on all drivers.

Keep in mind that this is susceptible to race conditions, i.e. if two scripts run at the same time, they might both chose the same "unused" filename. Make sure you're using some kind of locking mechanism to avoid that.

Upvotes: 2

Related Questions