why wont this query delete details from all tables?

Question

I am trying to delete records from tables matching users ID while i delete the user. but somehow it deletes records only from the cv table.

what i am trying is

if($_GET['deluser'] !='1'){

         $qr = "delete from members where member_id IN(".$_GET['deluser'].")";
         $qr = "delete from company where caller_id IN(".$_GET['deluser'].")";
         $qr = "delete from cv where agent_id IN(".$_GET['deluser'].")";
         $st = $db->prepare($qr);
         $st->execute();        

    header('Location: users.php?action=DELETED');
    exit;

what could i be doing wrong?

Because you execute only one query. All above are overwritten without execution. — Akina, Feb 17 '20 at 10:21
There's no point in using prepare() if you don't use prepared statements. — Rotimi, Feb 17 '20 at 10:30
@RogerRhode Check the official PHP documentation regarding that topic. It also helps you secure your query against SQL injections. If you paste the value from your GET parameter "deluser" without validation into your query string it's highly insecure and should not be used for production code. https://www.php.net/manual/de/mysqli.quickstart.prepared-statements.php — daPhantom, Feb 17 '20 at 10:45

score 5 · Accepted Answer · edited Feb 17 '20 at 20:40

5

In your case you overwrite the value in $qr every time so you need to execute it, everyone of them separately,

you need also to fix the SQL injection problem so you can fix it by using bind your data in the execute method or by using bindParam

first, you need to add ? with the same number of input you want to pass

you can check how it work here in this answer

$in  = str_repeat('?,', count(explode(',', $_GET['deluser'])) - 1) . '?';

$qr = "delete from members where member_id IN($in)";
$st = $db->prepare($qr);
$st->execute(explode(',', $_GET['deluser']));   

$qr = "delete from company where caller_id IN($in)";
$st = $db->prepare($qr);
$st->execute(explode(',', $_GET['deluser']));   

$qr = "delete from cv where agent_id IN($in)";
$st = $db->prepare($qr);
$st->execute(explode(',', $_GET['deluser']));

You can read more about BindParam and Execute in the docs

edited Feb 17 '20 at 20:40

Dharman

30,962
25
85
135

answered Feb 17 '20 at 10:24

Joseph

5,644
3
18
44

Thanks mate for your help. – Roger Rhode Feb 17 '20 at 10:27
Hi yes have heard and I understand i need to Bind the variables instead of using GET right? – Roger Rhode Feb 17 '20 at 11:30
Once you fix the SQL injection, ping me and I will remove downvote. – Dharman Feb 17 '20 at 11:44
@Dharman check the answer again i update it – Joseph Feb 17 '20 at 13:45
it won't do. you need a separate placeholder for the every user id – Your Common Sense Feb 17 '20 at 13:58
but how can this be fixed he didn't mention how many `userid` he want to add – Joseph Feb 17 '20 at 14:01
@YourCommonSense if you could fix it please edit it – Joseph Feb 17 '20 at 14:02
2

[here you go](https://stackoverflow.com/a/14767651/285587) – Your Common Sense Feb 17 '20 at 14:39
1

Here's a lesson for you for the future: don't answer badly written questions with missing details. Now you are in a pickle, because the proper solution is not so easy and the solution the OP accepted is extremely bad. This answer can still be saved and improved. – Dharman Feb 17 '20 at 14:42
@YourCommonSense i read it it's really awesome answer thanks for booth of you, please check my answer again i think it is correct now – Joseph Feb 17 '20 at 14:45
@Dharman is it correct now ? – Joseph Feb 17 '20 at 14:48
Not yet. `$_GET['deluser']` is a string. I assume you need to explode it into an array first. – Dharman Feb 17 '20 at 15:10
You can't say that it is a string without the form, but in general because he use `in` operator in his mysql query so he sure from that before – Joseph Feb 17 '20 at 15:37
1

All get parameters are a string. I think it is something like this: `1,2,3,4`, so they would need to split it up into an array. – Dharman Feb 17 '20 at 16:05
@Dharman i think that the answer now is completely complete – Joseph Feb 17 '20 at 20:37
1

Yes, it's quite good. Surprisingly you already have 5 upvotes on it. I think you now see the benefit in improving the answer. – Dharman Feb 17 '20 at 20:41
@Dharman really i learn a lot from you about improve the answer thanks a lot :) – Joseph Feb 17 '20 at 20:43

why wont this query delete details from all tables?

1 Answers1