chinazaike
chinazaike

Reputation: 517

How to properly avoid passing sql query in a loop via php

I want to check filename and its hash value from directory and database.

The code below works fine but I read some-where in a stackoverflow that its not good to pass sql query in a loop due to database performance issue.

Considering the working code below. please how do I re-structure the code so as to avoid passing sql queries in a loop or is my code good to go

$files = glob('C:/xampp/htdocs/test/*.{png}', GLOB_BRACE);

foreach ($files as $file) {
    $hash = md5($file);

    // check if files name and hash already exist
    include('dbpdo.php');
    $result = $db->prepare("SELECT md5_hash,filename FROM table_data where md5_hash=:md5_hash and filename=:filename");
    $result->execute(array(':filename' => $file, ':md5_hash' => $hash));

    $count = $result->rowCount();

    if ($count > 0) {
        echo "<br>Files already exist: $file<br>";
    } else {
        echo "<br>File Does not exist: $file<br>";
    }
}

Upvotes: 0

Views: 270

Answers (3)

tttony
tttony

Reputation: 5082

Since you are hashing the name of the file, there is no need to use it in the SELECT statement together with the name, just use the name itself

If you want to make sure that the file it's unique then you have to hash the content of the file but I would recommend hash_file with the algo sha256 or you can test which is the fastest for you, don't use md5

Apart from the @TheMouseMaster's answer you can try to get all the rows from the table fill an array using the name or the hash as key, create a foreach loop and isset($table_array[$hash_or_name]) to see if exists, but since it's getting all the rows from the table it can be slow

Upvotes: 0

TheMouseMaster
TheMouseMaster

Reputation: 298

Making a single DB statement to avoid DB overhead is a good practice, but to do so, you have to make sure you can get all of the search criteria data to the DB statement at one time. It is not required that you always do it this way.

By placing the Prepare statement outside of the loop, and then executing it within the loop 1 time per file, you will already be doing your performance a huge boost.

In order to actually do this with a single SQL call, you might have to take the entire list of files and their hashes, populate a temporary table with that data, and then compare the contents of that table against table_data

Upvotes: 1

chinazaike
chinazaike

Reputation: 517

Below is how I was able to resolve it. I just moved the include and the prepared out of the loop as suggested in the comments above

$files = glob('C:/xampp/htdocs/test/*.{png}', GLOB_BRACE);
// check if files name and hash already exist
include('dbpdo.php');
$result = $db->prepare("SELECT * FROM table_data");
        $result->execute(array());
$count = $result->rowCount();
foreach($files as $file) {
      if($count > 0) {
echo "<br>Files already exist: $file<br>";
}else{
echo "<br>File Does not exist: $file<br>";
}
}

Upvotes: 0

Related Questions