PHP Delete deletes every entry

PHP
1

Hi, I’ve bought the book “Database Driven Web Site Using PHP & MySql 4th edition” and I attempting for learning purposes create my own to-do list.

I have got it all working, I can add data, delete data and edit data, but I have noticed that once in a while for some reason if I delete a task item, it will delete them all.

This is my code for deleting a task


<?php
  if (isset($_GET['deletetask']))
  {
    $id = mysqli_real_escape_string($link, $_POST['id']);
    $sql = "DELETE FROM tasks WHERE id='$id'";
    if (!mysqli_query($link, $sql))
    {
      $error = 'Error Deleting Joke: ' . mysqli_error($link);
      exit();
    }
		header('Location: .');
		exit();
  }
?>

I cant see whats wrong with it can anyone else?

2

what happens when that particular function (designed to allow the deletion of multiple records at one time) goes into production? it’s not a good idea then, either

your query is either designed to delete multiple rows, in which case LIMIT 1 is silly, or it is designed to delete a single row (by specifying a value for a primary or unique key), in which case LIMIT 1 is silly

also, LIMIT is a mysql proprietary extension to SQL, so your code won’t port to another database system

:slight_smile:

3

A nice:

 die($sql);

will tell you what query it is passing to the database.

4

Other than when you’re developing something that’s specifically designed to allow the deletion of multiple records at one time, why would it not be a good idea?

I’ve read many many posts here by you and recognize your expertise on this subject, so I’m not trying to argue with you – I’m a piker compared to your knowledge and experience. But I’m genuinely curious – is there some reason it would be a bad idea to use LIMIT 1 in a delete query such as this? Or are you saying that it’s not needed (as opposed to “it’s a bad idea”)?

5

i disagree, it is not always a good idea

it should not be necessary to do this in cases where you are deleting based on the primary key, because there can only ever be one row per key anyway!!

6

Thankyou, I have added LIMIT 1 to my query so I should hopefully fix that issue.

This is my code, just so you can see whether I have added it correctly or not


<?php
  if (isset($_GET['deletetask']))
  {
    if(!ctype_digit($_POST['id'])) exit('Invalid ID Given');
    
    $id = mysqli_real_escape_string($link, $_POST['id']);
    $sql = "DELETE FROM tasks WHERE id='$id' LIMIT 1";
    if (!mysqli_query($link, $sql))
    {
      $error = 'Error Deleting Task: ' . mysqli_error($link);
      exit();
    }
		header('Location: .');
		exit();
  }
?>

Also the ID’s are auto incrementing yes, so I don’t believe it was that that was the issue

7

On the issue of deleting all of your records, there a couple of more things you can do.

First, in your delete query, include LIMIT 1 as part of the query. This is always a good idea. Then, no matter what, your query will only delete 1 record and not the whole gosh-darned table.

Also – have you checked to make sure that all of your id’s are unique? If you’re using an autoincrementing field, they certainly should be, but since you haven’t provided the table’s structure, that’s not something we can assume. If all your jokes have an id of 1, for example, then “DELETE FROM tasks WHERE id=‘1’” will of course delete all of them.

8

I dont understand either, also there not actually jokes, I must of just copied down jokes while reading the book and forgot to change it. What I have done is here if you ever fancy having a look
http://keironlowe.co.uk/experiments/2-Do-Lists/

Thanks for your help anyway, I have added that extra bit of code and I will look out for that error.

9

Code seems fine to me.
Are you sure $_POST[‘id’] is always a numeric id?

I don’t really see why that would delete all your jokes, but that’s the only thing I can see could go wrong with your code.

Try modifying it thusly:


<?php
  if (isset($_GET['deletetask']))
  {
    if (!ctype_digit($_POST['id'])) exit('Invalid id given!');
    
    $id = mysqli_real_escape_string($link, $_POST['id']);
    $sql = "DELETE FROM tasks WHERE id='$id'";
    if (!mysqli_query($link, $sql))
    {
      $error = 'Error Deleting Joke: ' . mysqli_error($link);
      exit();
    }
		header('Location: .');
		exit();
  }
?>

If you ever see the text “Invalid id given!”, then that’s your problem right there.

10

Hey, I’ve also noticed that when I edit one of the tasks it changes them all, I’ve had a go at editing it myself by adding

WHERE id='$id'

into the code by its not working.


<?php
	if (isset($_GET['edittask']))
	{
		$newtask = mysqli_real_escape_string($link, $_POST['editinput']);
		$sql = "UPDATE tasks WHERE id='$id' SET taskname=' \\" . $newtask . \\" ' . ";
		if (!mysqli_query($link, $sql))
		{
			$error = 'Error editing task: ' . mysqli_error($link);
			exit();
		}
		header('Location: .');
		exit();
	}
?>
11

Thankyou very much you two, that worked perfectly! :slight_smile:

12

That might be because it’s supposed to go the other way around:

“UPDATE tasks SET taskname=’ \” . $newtask . \" ’ . WHERE id=‘$id’"

UPDATE … SET … WHERE

:slight_smile:

13

UPDATE … WHERE … SET is invalid syntax

the WHERE clause comes last

please, always test your queries outside of php first

:slight_smile: