0

I'm checking a project and I found this code that is suppose to delete records from MySQL, despite this is not the best aproach to achieve this I have to make this works, but for me is fine, that's why I'm here, because I don't see the error or what is wrong with this code, is not displaying errors, warning, nothing I tested the code directly in PHPMyAdmin and it works well, so this is the code...

$get_records = mysql_query('SELECT id_detail FROM my_table WHERE user_id = "'.$user_id.'" AND last_date BETWEEN "'.$date1.'" AND "'.$date2.'"') or die (mysql_error());

$array_details = array();

while ($details = mysql_fetch_array($get_records)) {

    $array_details[] = $details['id_detail'];

}  


$array_details = array_unique($array_details);


foreach ($array_details as $id_detail) {

    $delete_detail = mysql_query("DELETE FROM my_table WHERE user_id = '".$user_id."' AND id_detail = '".$id_detail."'") or die (mysql_error());
}

I got two records in the same table with the same ID detail (this works that way), so If I get the ID correcly with the query inside the foreach loop it should delete all the records with that id_detail and user_id doesn't matter if in the query I say between which dates get those IDs, it deletes only one, and that one is which has the date between that range

I have no idea why is not deleting both, some idea about what I'm missing?

NOTE: As I said before if I do that query directly in PHPMyAdmin it works fine and if I do from PHP it delete only one.

Hail Hydra
  • 472
  • 6
  • 19
  • Maybe it's getting only one id instead of two (in this script) – Aniruddha Chakraborty Sep 27 '16 at 17:00
  • Why would you use PHP as a middle-man for this? You're sidestepping MySQL's built in logic and increasing your queries by n+1. Why not just do a single `DELETE FROM {logic}` statement? Even if for some reason you do want to have PHP logic in between all of this stuff, you have an array of primary keys (presumed), so why not leverage `WHERE IN` to reduce queries? – Blake Sep 27 '16 at 17:01
  • if you always have 2 entries in the table, deleting by offset may solve it something like i=0; foreach($a as $b){ $my_query to del $b[0]; $i++;} – xYuri Sep 27 '16 at 17:05
  • @xYuri not always will be 2 – Hail Hydra Sep 27 '16 at 17:09
  • @Blake I know this is not best aproach but even with `WHERE IN` only deletes one – Hail Hydra Sep 27 '16 at 17:11
  • sorry, i meant its one or more, my solution will work if you applied it the right way, but i recommend what @Blake recommended – xYuri Sep 27 '16 at 17:21
  • WHERE IN might delete one row at a time, I'm not familiar with the execution plans in MySQL, but in SQL Server it typically converts this to a WHERE EXISTS execution which is set based. If EXISTS is available in your MySQL version, I'd recommend retrofitting Rebecca's answer below to use that. – vanlee1987 Sep 27 '16 at 17:27
  • 1
    And if you're not super deep into your application, you should really consider prepared statements using PDO or Mysqli to prevent sql injections. http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 – vanlee1987 Sep 27 '16 at 17:27
  • **WARNING**: If you're just learning PHP, please, do not use the [`mysql_query`](http://php.net/manual/en/function.mysql-query.php) interface. It’s so awful and dangerous that it was removed in PHP 7. A replacement like [PDO is not hard to learn](http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/) and a guide like [PHP The Right Way](http://www.phptherightway.com/) explains best practices. Your user parameters are **not** [properly escaped](http://bobby-tables.com/php) and there are [SQL injection bugs](http://bobby-tables.com/) that can be exploited. – tadman Sep 27 '16 at 17:36
  • to stay safe from `sql inection` 2 simple things to do 1) never use `mysql_*` functions, that means any function starts with `mysql_`, instead you can use `mysqli_*` functions. also always escape the values submitted by users by using, `mysqli_real_escape_string($connection_name, $string_to_escape);` – xYuri Sep 27 '16 at 20:02

1 Answers1

0

Does replacing that code with this query work for you?

$delete_detail = mysql_query("DELETE FROM my_table WHERE user_id = '".$user_id."' AND id_detail IN (SELECT id_detail FROM my_table WHERE user_id = '".$user_id."' AND last_date BETWEEN '".$date1."' AND '".$date2."')") or die (mysql_error());
xYuri
  • 369
  • 2
  • 17
Rebecca Close
  • 890
  • 7
  • 11