Trying to avoid duplicate SQL entries

Question

I am trying to avoid adding a certain game's data to my table more than once so I am trying to make an if statement that would check if that game's ID is already in the table, however for some reason the if statement is always false. This is my code:

    $a = $_GET['id'];
    $colname = $_GET['colname'];
    $b = "SELECT count(*)
              FROM table
              WHERE gameid = ".$a;
    if($dup = mysqli_query($dbc, $b)){
      if(mysqli_num_rows($dup)==0){
        $insrt = "INSERT INTO table ($colname)
        VALUES ($a)";
        mysqli_query($dbc, $insrt);
      }
    }

The example code is vulnerable to SQL Injection, because it's incorporating a potentially unsafe value into the text of a SQL statement. Potentially unsafe values must be properly escaped, using e.g. mysqli_real_escape_string function. But in this example statement, escaping still wouldn't prevent SQL injection. An even better pattern is to use a prepared statements with bind placeholder. — spencer7593
– spencer7593, Commented Jun 2, 2015 at 0:08

jming · Accepted Answer · 2015-06-02 00:00:54Z

1

If I were you, instead of using logic within your program to avoid creating duplicate entries, I would simply tell MySQL that your ID column should be unique. Take a look at the info on column definitions in the MySQL Reference Manual, specifically the keywords UNIQUE or PRIMARY KEY.

If you specify that your ID column should be a unique index, then MySQL will prevent another entry with the same ID value from being added. That way, in your program, you can simply attempt to add the data, and the procedure will automatically fail if it is a duplicate. As an added bonus, this means you'll only have to do one query from your program instead of two.

answered Jun 2, 2015 at 0:00

jming

1108 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

spencer7593 Over a year ago

Yes, add a UNIQUE KEY key constraint if the rule is that the gameid must be unique in the table. But OP code could still do the same pattern, especially if he's got an AUTO_INCREMENT, attempting to insert a duplicate row will use up a value, and any BEFORE INSERT triggers are going to be fired. And OP code also works even if he can't add a unique constraint, and what he wants is just to "not add a row".

Uueerdo Over a year ago

I agree with @spencer7593. Yes, use UNIQUE KEY to enforce the constraint, but not checking beforehand and just letting the query fail is "poor form" in my personal opinion. Usually, it's a lot easier, and cleaner, to check beforehand than it is to determine that your error wasn't really an actual problem.

Uueerdo · Accepted Answer · 2015-06-01 23:50:48Z

1

A SELECT COUNT().... query, barring exceptional circumstances, is generally going to return at least one row (more if there is a GROUP BY clause that would indicate otherwise); you need to check that the field value is 0, not that there are no rows in the result.

answered Jun 1, 2015 at 23:50

Uueerdo

16k1 gold badge19 silver badges21 bronze badges

3 Comments

ghadams Over a year ago

but the gameid's are unique to each match and so if I have already recorded the match there should be one row in the table with that id, so it should return 1 right? And if I haven't recorded the game with that certain id then it should return a 0, because of my WHERE statement.

spencer7593 Over a year ago

@ghadams: executing a query returns a resultset. And mysqli_num_rows gives the number of rows in the resultset. It doesn't matter if the value returned by SELECT COUNT() is 3, 1, or 0. The resultset returned by the query actually does contain a row. So, even when COUNT() is zero, the number of rows in the result set is still 1. But you don't really need to check the value on the row, just change the query so that it doesn't return a row.

Uueerdo Over a year ago

@ghadams Correct. I am not very familiar with PHP, but it looks like you are checking the row count, not the calculated COUNT() value.

spencer7593 · Accepted Answer · 2015-06-01 23:58:38Z

0

Change your query to remove the aggregate, and just return a column, e.g.

 SELECT gameid 
   FROM table
  WHERE gameid = ?
  LIMIT 1

You don't need a count of rows, you just need to know whether a row is returned.

Or, you could add a HAVING clause to your query to not return a row when the COUNT is zero...

 SELECT COUNT(*)
   FROM table
  WHERE gameid = ?
 HAVING COUNT(*) > 0

There's no need for you to retrieve the value of the column from the row that's returned, just return an empty resultset, and test whether the resultset is empty, like your code is doing.

answered Jun 1, 2015 at 23:58

spencer7593

109k15 gold badges122 silver badges148 bronze badges

Collectives™ on Stack Overflow

Trying to avoid duplicate SQL entries

3 Answers 3

2 Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related