0

I am trying to update the store column by adding new item (string) to the end of the column, But what happens is the new item is added twice at the end of the column, This is the code:

$query = "SELECT * FROM users";
$result = $conn->query($query);

while($row = $result->fetch_assoc()){

$item = 'item_name';
$store = $row['store'];
$newstore = $store . '|' . $item;

echo 'newstore : ' . $newstore . '<br>'; // It looks normal : store|item
    
$sql = "UPDATE users SET store='" . $newstore . "' WHERE username='" . $row['username'] .  "'";
$conn->query($sql); 
}

In in the database I find: store|item|item

Dharman
  • 30,962
  • 25
  • 85
  • 135
alex555
  • 15
  • 6
  • 1
    **Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Dec 15 '20 at 22:18
  • Do you have users with duplicate usernames? Are you 100% sure the code isn't executed twice? – M. Eriksson Dec 15 '20 at 22:19
  • @Dharman I will only use the code for one time only (by me) – alex555 Dec 15 '20 at 23:02
  • @MagnusEriksson no, no users with duplicate usernames, and yes i'am sure that the code isn't executed twice – alex555 Dec 15 '20 at 23:04
  • Everything was fine, I think the problem was with the php, I used node js and everything worked just right – alex555 Dec 15 '20 at 23:05
  • Then if it's only going to be used one time and by you, **then you must use parameterized queries as I have said before**. There is literally no excuse why you would ever want to do it any other way. You should always use prepared statements and bind your data separately. – Dharman Dec 15 '20 at 23:05
  • @Dharman It meets my needs and I wrote the code a little bit faster (tests in the localhost) In production always use a filters and prepare stmt anyway thank you – alex555 Dec 16 '20 at 00:46

1 Answers1

0

Rather than reading the entire table and looping through it with PHP, run just a single UPDATE query to concatenate the extra data onto the column.:

$item = 'item_name';
$query = "UPDATE users SET store=concat(store,'|','$item')";
$result = $conn->query($query);

Note: this form is potentially open to SQL injection if you can't trust the value in $item. You'd do better to use a prepared query if that's the case.

  • I have to loop with php because I will add costume conditions later, do you notice any problem in the code? – alex555 Dec 15 '20 at 21:17
  • I don't see a problem with the code. Are you sure the database contains what you think it does? Have you run the query before? In any case, depending on the complexity of your conditions, you can build them into your UPDATE query. –  Dec 15 '20 at 21:28
  • It's weird, I tried your code and got the same error – alex555 Dec 15 '20 at 21:48
  • Let's assume it can't be trusted – Strawberry Dec 15 '20 at 23:01
  • @Strawberry Let's not make unwarranted assumptions. –  Dec 16 '20 at 00:30