How to update mysql database using php foreach

Question

I am working on a project that when the user approves a task (using approve and decline radio button choices), all the database rows with the same task id must be updated. Each task has sub tasks, and I wish to update the sub tasks too, not only the main task. I am unsure how to implement the foreach condition. Here is my code:

foreach( $_POST['task_id'] as $id) {
  $ids = $id["pro_id"];
}

$update = "UPDATE tasks SET for_checking = 'no', approved = 'yes' WHERE task_id = $ids";

I know my coding is faulty, sorry for that. I hope someone can help me improve my code. Or someone tell me if there is any other way other than using foreach. Newbie student here. Thanks in advance!

You probably don't even need the `foreach` if all the values are the same. Use the `in()` operator in your query and implode all the values. You should parameterize your query. As is `$ids =` overwrites on every iteration, so that variable only ever has 1 value. — user3783243, Dec 17 '18 at 13:39
Query execution is missing... Also `$update` should probabply be inside the foreach. Also as stated above, use parametrized statements. — Eugene Anisiutkin, Dec 17 '18 at 13:41
@EugeneAnisiutkin But that would be agonizingly slow and inefficient. — Strawberry, Dec 17 '18 at 13:42
`foreach` on radios? This question's unclear. We need to see the HTML/form for this. `foreach` is usually used with checkboxes, *not* radios. Radios are "one" choice, not multiple. — Funk Forty Niner, Dec 17 '18 at 13:55
You're also overwriting the `$ids` variable on each iteration. — M. Eriksson, Dec 17 '18 at 13:56
@user3783243 Hello! Thanks for the response. I forgot to mention that there is a task list. So in one click of submit button, multiple tasks are updated. And each task has sub tasks. That's why I thought of using foreach. — Dizza, Dec 17 '18 at 14:01
@FunkFortyNiner Hi. I used radio button because approve and decline are the only choices. The user can choose one only. — Dizza, Dec 17 '18 at 14:02
@Dza you've been given answers, you'll have to take it up with them. I can't offer any more help with this, not till I know what it is we're dealing with here. I'll just stand on the side lines on this one and see what comes of it, sorry. — Funk Forty Niner, Dec 17 '18 at 14:04
Are you building a list of answers from each radio into one array? e.g. there's 2 radio options but there's 5 settings- are you sending an array of 5 with the selected radios or are you trying to send radios individually? — treyBake, Dec 17 '18 at 14:18

score 4 · Answer 1 · answered Dec 17 '18 at 13:43

4

What you should do is use prepared statements. When dealing with user input ($_POST, $_GET) - it's always best to use a prepared statements. Prepared statements protect you from SQL injection. You can use the mysqli library but I personally prefer PDO.

Here's some example code on how to set up PDO:

<?php
    $pdo = new \PDO('mysql:hostname=localhost;dbname=my_db', 'user', 'password');

    # we now have access to function in PDO! Happy days
    # we write your SQL outside the foreach loop
    $sql = 'update `table` set `field` = :value;';
    # prepare your statement
    $res = $pdo->prepare($sql);

    # loop through data
    foreach ($_POST['ids'] as $id)
    {
        $res->execute(array(':value' => $id));
    }

    # now we've done an SQL statement that's fast and safe!

answered Dec 17 '18 at 13:43

treyBake

6,440
6
26
57

4

Well, it's not fast, but it *is* safe. Note that, while fiddly, it is possible to construct a query that uses IN() with parameters. – Strawberry Dec 17 '18 at 13:46
I wouldn't say just `user input`, that's how 2nd level injections get executed. Any non-static value should be bound. (Maybe add https://stackoverflow.com/questions/53400701/dynamically-creating-or-conditions-by-passing-an-array-to-a-query-in-mysql-php/53400798#53400798 then the `foreach` isn't needed) – user3783243 Dec 17 '18 at 13:46
@Strawberry ahh I see, my PHP is better than MySQL - I forget about `IN`! – treyBake Dec 17 '18 at 13:52
@Strawberry does each element in the array needs it's own key in PDO binding? Or can I do :value and set it to implode($array, ' ')? – treyBake Dec 17 '18 at 13:53
@user3783243 what, other than user input is non-static? – treyBake Dec 17 '18 at 13:57
You should [read my comment](https://stackoverflow.com/questions/53816374/how-to-update-mysql-database-using-php-foreach#comment94481576_53816374) up there and look at their question again. – Funk Forty Niner Dec 17 '18 at 13:58
@FunkFortyNiner I feel like your comment tackles a bigger semantics problem.. though the main two things I got from the question was how to implement the foreach with the SQL statement whilst suggestion improvements.. if the data is coming to PHP as an array then my answer is still relevant. I agree that the data shouldn't be an array - but if it is then my answer addresses that... – treyBake Dec 17 '18 at 14:01
Unfortunately, my MySQL is much better than my PHP (although that's not saying much), so I always have to refer back to google - here's a reference https://websitebeaver.com/php-pdo-prepared-statements-to-prevent-sql-injection – Strawberry Dec 17 '18 at 14:01
@treyBake Database data – user3783243 Dec 17 '18 at 14:05
@user3783243 wasn't aware people could hijack retrieval functions .. though I use prepared for everything so I guess I never needed to? Idk haha – treyBake Dec 17 '18 at 14:07
@treyBake Well, you use prepared to store, that means injectable code could live in your db. If you then execute that code again later, unprepared, it could be executed. https://portswigger.net/kb/issues/00100210_sql-injection-second-order – user3783243 Dec 17 '18 at 14:13
The question can't be answered and the OP still hasn't updated their question to contain the HTML for this. I feel they're going about it the wrong way. So yeah, we're dealing with an unknown animal with "bigger semantics". I've asked them to update it but still nothing and they're not saying anything in any of the answers. Hence, no "green tick". – Funk Forty Niner Dec 17 '18 at 14:14
@FunkFortyNiner I've added a comment above (on the actual OP) that may get some much needed clarity :) – treyBake Dec 17 '18 at 14:19
`foreach` can't be used on radios, but checkboxes, unless they're using JS which *probably* could work. I originally voted to close as unclear earlier and feel my vote is valid. I think this one's a stalemate. Edit: You deleted the comment I was responding to, why? – Funk Forty Niner Dec 17 '18 at 14:19
@FunkFortyNiner ^^ the JS part is where I think it's a possibility that radio values are sent to an array - but I can see your point, lets just hope OP gets back soon :) – treyBake Dec 17 '18 at 14:20
@FunkFortyNiner because I felt it was a bit ragey/ranty :) – treyBake Dec 17 '18 at 14:20

How to update mysql database using php foreach

1 Answers1