2

I've been wondering how to check in a MySQL table if username and/or email are duplicates during registration. Here's what I've tried so far but to no avail:

$username = $_POST['username'];
$sq = $db->exec("SELECT * FROM `users` WHERE `username` = '$username'");
if ($sq->rowCount() > 0)
{
    $msg = "That username is already taken.";
    $error = true;
}

$email = $_POST['email'];
$sq = $db->exec("SELECT * FROM `users` WHERE `email` = '$email'");
if ($sq->rowCount > 0)
{
    $msg = "That email is already taken.";
    $error = true;
}

if (!error)
{
    //add to db
}

The error this gives is Call to a member function rowCount() on a non-object

Could you please help?

OMG Ponies
  • 325,700
  • 82
  • 523
  • 502
argoneus
  • 1,097
  • 2
  • 13
  • 24
  • 2
    What's the problem? Does it not work at all? Do something unexpected? Not pick up existing usernames and emails? – andrewsi Sep 15 '12 at 00:16
  • Oh, I knew I forgot something. Edited. – argoneus Sep 15 '12 at 00:18
  • **Your code is vulnerable to SQL injection.** You *really* should be using [prepared statements](http://stackoverflow.com/a/60496/623041), into which you pass your variables as parameters that do not get evaluated for SQL. If you don't know what I'm talking about, or how to fix it, read the story of [Bobby Tables](http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain). – eggyal Sep 15 '12 at 00:19
  • I don't know much about PHP, but in the first if you have parens in $sq->rowCount() and in the second if you just have $sq->rowCount. Could this be an issue? – ChaseMedallion Sep 15 '12 at 00:22

3 Answers3

4

PDO::exec() returns an integer, so your above code will die with a fatal error along the lines of call to member function rowCount() on a non-object. What you should do for this is SELECT COUNT(*) to get the number of rows.

You can also use exceptions to aid the error handling process.

Like this:

// Ensure PDO will throw exceptions on error
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

// Turn emulated prepares off
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, FALSE);

try {

  // Check if username is taken
  $stmt = $db->prepare("SELECT COUNT(*) FROM `users` WHERE `username` = :username");
  $stmt->execute(array('username' => $_POST['username']));
  if ($stmt->fetchColumn() > 0) {
    throw new Exception("That username is already taken.");
  }

  // Check if email is taken
  $stmt = $db->prepare("SELECT COUNT(*) FROM `users` WHERE `email` = :email");
  $stmt->execute(array('email' => $_POST['email']));
  if ($stmt->fetchColumn() > 0) {
    throw new Exception("That email is already taken.");
  }

  // Username and email are free

} catch (PDOException $e) {

  // A database error occured

} catch (Exception $e) {

  // Either the username of email is taken

}

If the data in the column should be unique, it should have a unique index on it to prevent duplicate inserts, and speed up queries that search based on the values of that column.

DaveRandom
  • 87,921
  • 11
  • 154
  • 174
  • 1
    Or even `SELECT EXISTS (SELECT * FROM ...)` so that MySQL knows to stop searching once it finds a single matching record, rather than continuing looking for others in order to find an overall count that is of no interest. – eggyal Sep 15 '12 at 00:25
  • @eggyal I was wondering about a `LIMIT 1` for that reason. Although to be fair if the column should be unique, it should have a unique index so that wouldn't really optimise it. – DaveRandom Sep 15 '12 at 00:26
  • I don't think `LIMIT 1` would make any difference, as it would only apply to the aggregated results (of which there will only ever be 1 record). But yes, a `UNIQUE` key would make my point redundant. – eggyal Sep 15 '12 at 00:27
  • @eggyal Ahh yes, `LIMIT` is applied after results have been collected so I suppose it will not affect performance. Updated answer to mention unique index and refactored to be a bit nicer. – DaveRandom Sep 15 '12 at 00:33
  • If I make the column value unique, it will throw a mysql error before even executing this code, no? – argoneus Sep 15 '12 at 00:37
  • @argoneus If you add a unique index it will throw an error if you try and insert a duplicate row - but only at the point that you actually try and execute the query. – DaveRandom Sep 15 '12 at 00:37
2

To avoid a potential race condition, you should simply set your username and email fields as unique indexes.

You database engine will give you a specific error code when you try to insert a duplicate row, for MySQL, it's 1062.

If the query fails, check if the error number is that of a duplicate row, and display the message based on that.

At the very least, your columns should be unique indexes to prevent duplicates at the database level.

discomatt
  • 195
  • 1
  • 4
0

Please clarify the type of $db, assuming it to be sqllite3.

Check the documentation for SQLLITE3::exec() at http://php.net/manual/en/sqlite3.exec.php .. as far as it says, it only valid for (UPDATE, INSERT, DELETE) and returns boolean. So, your code will not probably run.

If according to your tag, you're using MySQL. I believe you need to look at mysql/mysqli extension, mysql_query() & mysqli_query(), or $db -> query() if $db is of type mysqli.

Mahmoud Aladdin
  • 536
  • 1
  • 3
  • 13