0

I have a simple function below to check if the user is already registered on my site. If they are then it doesn't add them and if they aren't it adds them. However In my database for some weird reason a certain user is getting added multiple times with the exact same id. Is there something wrong with mysqli_num_rows?

  $check = mysqli_query($con, "SELECT id FROM users WHERE id='$id'");
    $row_cnt = mysqli_num_rows($check);
       if ($row_cnt == 0) { 
    $query = "INSERT INTO users (id,username) VALUES ('$id','$username')";
    mysqli_query($con, $query);
       } else {   // If Returned user . update the user record      
    $query = "UPDATE users SET username='$username' where id='$id'";
    mysqli_query($con, $query);
    }
user2570937
  • 802
  • 3
  • 17
  • 34
  • 2
    What is the value of `$check`? Show a `print_r` or `var_dump` of `$check` – AlexL Nov 14 '14 at 22:12
  • have you tried running this before the num_rows check? http://php.net/manual/en/mysqli.store-result.php – Kai Qing Nov 14 '14 at 22:14
  • @KaiQing: I've never used `mysqli_store_result`, what does it do? The docs for [`mysqli_num_rows()`](http://php.net/manual/en/mysqli-result.num-rows.php) don't show it being used? – gen_Eric Nov 14 '14 at 22:18
  • Also, do `var_dump($row_cnt);` – gen_Eric Nov 14 '14 at 22:19
  • 1
    @Rocket - I never had either until I had this exact same issue. I'm using mysqli in OOP, not procedural. Not sure if it matters. Taken from docs it "Transfers a result set from the last query" but some other answer on SO suggested it in my case and indeed it worked. I have no idea why it was necessary and why it didn't allow me to access num_rows without it. Maybe it's just one reason why people are always suggesting PDO over mysqli... I'll try to find the link – Kai Qing Nov 14 '14 at 22:21
  • @KaiQing: Guess it's worth a shot, maybe it'll fix whatever issue is going on here. – gen_Eric Nov 14 '14 at 22:22
  • 2
    **WARNING**: When using `mysqli` you should be using parameterized queries and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). – tadman Nov 14 '14 at 22:23
  • @rocket - Here's the answer I used. I was binding parameters... you should be too. I mean OP should be too... http://stackoverflow.com/questions/5016253/php-mysqli-num-rows-returning-0 – Kai Qing Nov 14 '14 at 22:25
  • @KaiQing: AH! I think `store_result` is needed for prepared queries. :) – gen_Eric Nov 14 '14 at 22:28
  • right, I'm not sure why his is failing, but maybe the answer here is not why, but that he should be preparing his query anyhow so it doesn't really matter. – Kai Qing Nov 14 '14 at 22:36
  • i'm getting all my data from facebook so theres no need to prepare any statements. It still doesn't explain why I'm getting duplicate rows with the same ID though. I understand I should be using them but this is for a project written before that and it is illogical to change it all. – user2570937 Nov 14 '14 at 22:55

1 Answers1

2

A few things here.

First of all, num_rows() notoriously doesn't always return the right value. If you want to know how many rows match some criterion, use this query then read the rowcount in the one-row resultset.

SELECT COUNT(*) AS rowcount FROM users WHERE id='$id'

Second, it looks like you're trying to do a conditional insert / update operation. In this case you will be best off using Use the INSERT ... ON DUPLICATE KEY UPDATE mySQL command. This gets around the race condition that is in the code in your question.

You want a query something like this:

INSERT INTO users (id,username) VALUES ('$id','$username')
ON DUPLICATE KEY UPDATE username='$username';

http://dev.mysql.com/doc/refman/5.6/en/insert-on-duplicate.html

This will either add the row you want or change the username in the existing row, all at once. If some other user tries to do the same thing at the same time, this will do it safely.

O. Jones
  • 103,626
  • 17
  • 118
  • 172