SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 30
  1. #1
    SitePoint Member Godz06's Avatar
    Join Date
    Nov 2013
    Posts
    12
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)

    checking if username is already taken

    It's still letting me register the same username over and over again, what Am i doing wrong here? thanks!

    PHP Code:
    <?php
        
    function register(){
            if(isset(
    $_POST['username'], $_POST['password'])){
                require 
    'core/connect.php';

                
    $query dbConnect()->prepare("INSERT INTO users (username, password) VALUES (:username, :password)");

                
    $query->bindValue(':username'$_POST['username']);
                
    $query->bindValue(':password'$_POST['password']);

                if(
    $_POST['username'] == 1){
                    echo 
    'Username already taken.';
                }
                if(empty(
    $_POST['username']) || empty($_POST['password'])){
                    echo 
    'Please fill out all fields.';
                } else {
                if(
    $query->execute()){
                    
    header("Location: home.php");
                }
                }
            }
        }
    ?>

  2. #2
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,868
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    How is the table defined? Do you have username defined as unique in the table?
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  3. #3
    SitePoint Member Godz06's Avatar
    Join Date
    Nov 2013
    Posts
    12
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    `id` int(11) NOT NULL AUTO_INCREMENT,
    `username` varchar(32) NOT NULL,
    `password` varchar(32) NOT NULL,
    PRIMARY KEY (`id`)

  4. #4
    SitePoint Enthusiast
    Join Date
    Jun 2011
    Location
    Singapore
    Posts
    29
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Code:
    ALTER IGNORE TABLE mytbl ADD UNIQUE (columnName);

  5. #5
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,868
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    What is the id field for? If the username is supposed to be unique then that should be the primary key. The id field is only needed to allow multiple entries with the same username.

    Code:
    `username` varchar(32) NOT NULL,
     `password` varchar(32) NOT NULL,
     PRIMARY KEY (`username`)

    Also why is the password field a varchar rather than a char field - whether the password is one character long or a million characters long the hash you convert it to before storing it in the database will always be the same length.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  6. #6
    Programming Team silver trophybronze trophy
    Mittineague's Avatar
    Join Date
    Jul 2005
    Location
    West Springfield, Massachusetts
    Posts
    17,250
    Mentioned
    196 Post(s)
    Tagged
    2 Thread(s)
    I'm not sure
    PHP Code:
                if($_POST['username'] == 1){
                    echo 
    'Username already taken.';
                } 
    is what you want. Shouldn't that be something like numrows returned from the query?

    If you need an 'id' for use in JOIN queries etc. you could still use it, but felgall is correct, it shouldn't be the primary key in this table.

  7. #7
    SitePoint Member Godz06's Avatar
    Join Date
    Nov 2013
    Posts
    12
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    So something like this?

    PHP Code:
    $stmt $conn->prepare("SELECT COUNT(*) AS count FROM `user` WHERE username=?");
    $stmt->execute(array($username));
    while (
    $row $stmt->fetch(PDO::FETCH_ASSOC)) {
      
    $username_count $row["count"];
    }
    if (
    $username_count 0) {
      echo 
    "That username is already taken";


  8. #8
    SitePoint Enthusiast
    Join Date
    Jun 2011
    Location
    Singapore
    Posts
    29
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Seriously, just make the username column unique in the database and save yourself the hassle. If you use phpMyAdmin it should just be a checkbox (I think).

  9. #9
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    698
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by felgall View Post
    What is the id field for? If the username is supposed to be unique then that should be the primary key. The id field is only needed to allow multiple entries with the same username.
    Well no. Lot's of times you need to link other tables to the user table. You might also want to give users the ability to change their username. Keep the autoinc id.

  10. #10
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Jordan Windebank View Post
    Seriously, just make the username column unique in the database and save yourself the hassle. If you use phpMyAdmin it should just be a checkbox (I think).
    This is a good practice but it's not a real solution. Adding a duplicated key will result into an error. I know, you can use ON DUPLICATE KEY UPDATE/IGNORE but, again, this is not a good practice. You must check for existing records first, before the INSERT.

    So, as @Mittineague said, you have the wrong IF.
    You must check if you have any user with that `username`
    You just need results from
    Code:
    SELECT `iduser` FROM `users` WHERE `username` = :username
    And, check if $record['iduser'] is empty before the INSERT. If it's not empty return some error.

    If the column username is a varchar and unique this should be an instant query.
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel

  11. #11
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    698
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by vectorialpx View Post
    This is a good practice but it's not a real solution. Adding a duplicated key will result into an error. I know, you can use ON DUPLICATE KEY UPDATE/IGNORE but, again, this is not a good practice. You must check for existing records first, before the INSERT.

    So, as @Mittineague said, you have the wrong IF.
    You must check if you have any user with that `username`
    You just need results from
    Code:
    SELECT `iduser` FROM `users` WHERE `username` = :username
    And, check if $record['iduser'] is empty before the INSERT. If it's not empty return some error.

    If the column username is a varchar and unique this should be an instant query.
    You solution is not 100% complete in a web application environment. Consider the case where two users with the same username push the create user button at pretty much the same time. It's possible that the user 1 request might check for an existing user per your code, find there is none and then attempt to create one. Meanwhile, the second request has slipped in, also checked for an existing user and is then able to create a new user in between the time that the first request checks the username and inserts a new one.

    You could try locking tables but that has it's own set of problems. The only reasonable approach is to create a unique index and then check for errors(or catch the exception) after the insert.

    Prechecking the username is still a good idea but it's not a replacement for dealing with unique constraints at the database level and dealing with database errors.

  12. #12
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    I know what you're saying and I didn't deny the unique key idea.
    It's just not an elegant solution, to add an error into your MySql logs for each failed username.
    This is not a professional way of doing this (my opinion).

    The unique key exception should trigger only in case a user will send the request exactly in the small gap between MySQL select check and making the good and valid insert (where MySql locks the table for writing). This gap should take something like 0.001 seconds or less. And I'm not saying it's impossible to hit a request into this gap but, like I said, it's a good practice to check first.

    As example, all big companies will make an Ajax request that checks the username before you hit the submit.
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel

  13. #13
    Programming Team silver trophybronze trophy
    Mittineague's Avatar
    Join Date
    Jul 2005
    Location
    West Springfield, Massachusetts
    Posts
    17,250
    Mentioned
    196 Post(s)
    Tagged
    2 Thread(s)
    I guess the question is if @Godz06 ; is "stuck" with the database architecture?

    If not, I think it could be better designed, if yes, then will need do the best as can be done with what's there.

  14. #14
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,868
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    It is NOT good practice to check if a value already exists before inserting it.

    Best practice is to try to insert it and to capture and do appropriate processing of the error if it fails (which might be to ignore the error).

    Apart from the potential timing problems when two people try to insert simultaneously, it is also far more efficient to do one database call than two.

    With username being only 32 characters long it is perfectly appropriate in 99% of cases to use that as a key in other tables associated with the user as well so the only reason for including an id would be if you want people to be able to change their username on a regular basis. If users will only rarely change their username then you can use the username as the primary key and handle the change as a rare exception condition - the way almost all forum software does.

    If you definitely want a numeric primary key then make the username a number - I have seen a number of sites that allocate the next available number as the username as each person signs up.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  15. #15
    From space with love silver trophy
    SpacePhoenix's Avatar
    Join Date
    May 2007
    Location
    Poole, UK
    Posts
    5,072
    Mentioned
    103 Post(s)
    Tagged
    0 Thread(s)
    You could use transactions. The first query would check to see if the chosen name exists, the second query would insert a new row if the username was not found by the first query
    Community Team Advisor
    Forum Guidelines: Posting FAQ Signatures FAQ Self Promotion FAQ
    Help the Mods: What's Fluff? Report Fluff/Spam to a Moderator

  16. #16
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    It is NOT good practice to check if a value already exists before inserting it.
    I agree, but this should be an exception, because you have to show some message to the user. You are talking about avoiding duplicates, as a general matter but this is not just it. You will need to add the username AND the email as unique key. Maybe there is another unique key to consider (let's say personal code or other stuff). How will you do it? What will you tell to the user?

    edit:
    If users will only rarely change their username then you can use the username as the primary key
    I disagree with this. First of all, varchar will be way slower (size matters). Second, the username should be changeable, by business logic. I wouldn't recommend to "truncate" this option with the table design because the "client" can change its mind very easy. Just a personal opinion.

    In fact, all my comments are "what I would do" but, every programmer (db designer) has own choices and preferences it does not mean my way is the best
    Last edited by vectorialpx; Jan 6, 2014 at 16:49. Reason: added new comment
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel

  17. #17
    SQL Consultant gold trophysilver trophybronze trophy
    r937's Avatar
    Join Date
    Jul 2002
    Location
    Toronto, Canada
    Posts
    39,338
    Mentioned
    63 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by ahundiak View Post
    The only reasonable approach is to create a unique index and then check for errors(or catch the exception) after the insert.
    totally agree

    Quote Originally Posted by ahundiak View Post
    Lot's of times you need to link other tables to the user table.
    normally a natural key, when one exists, is better than a surrogate key, but not if there are many other tables linking to it -- which is exactly what happens with a user table

    so yeah, keep the id

    Quote Originally Posted by vectorialpx View Post
    It's just not an elegant solution, to add an error into your MySql logs for each failed username.
    This is not a professional way of doing this (my opinion).
    well, you know what they say, everyone's entitled to an opinion, even if it's wrong

    Quote Originally Posted by SpacePhoenix View Post
    You could use transactions.
    bah

    you've heard of solving the wrong problem? this is applying the wrong solution

    Quote Originally Posted by vectorialpx View Post
    You will need to add the username AND the email as unique key.
    wha?

    no, the username being defined as unique in the database is really all that's required

    trapping the "duplicate key" database error message and giving the user a friendly message is NOT THAT HARD, guys

    rudy.ca | @rudydotca
    Buy my SitePoint book: Simply SQL
    "giving out my real stuffs"

  18. #18
    SitePoint Enthusiast
    Join Date
    Jun 2011
    Location
    Singapore
    Posts
    29
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You should be setting unique on both your username and email fields anyway, that's just good database design. To handle your registration I'd consider something like this.

    PHP Code:
    $isError false;
    $errorMessage '';

    $query $conn->prepare("SELECT COUNT(*) AS count FROM `user` WHERE username=:username");
    $query->bindValue(':username'$_POST['username']);
    $query->execute();

    while (
    $row $query->fetch(PDO::FETCH_ASSOC)) {
      
    $user_count $row["count"];
    }
    if (
    $user_count 0) {
      
    $isError true;
      
    $errorMessage $username.' already taken, please choose another.';
    }

    $query $conn->prepare("SELECT COUNT(*) AS count FROM `user` WHERE email=:email");
    $query->bindValue(':email'$_POST['username']);
    $query->execute();

    while (
    $row $query->fetch(PDO::FETCH_ASSOC)) {
      
    $email_count $row["count"];
    }
    if (
    $email_count 0) {
      
    $isError true;
      
    $errorMessage $email.' already registered, please login.';
    }

    if(empty(
    $_POST['username']) || empty($_POST['password']) || empty($_POST['email'])){
      
    $isError true;
      
    $errorMessage 'Please fill in all fields.';
    }

    if(
    isError) {
      echo 
    $errorMessage;
    } else {
      
    $query dbConnect()->prepare("INSERT INTO users (username, email, password) VALUES (:username, :email, :password)");

      
    $query->bindValue(':username'$_POST['username']);
      
    $query->bindValue(':email'$_POST['email']);
      
    $query->bindValue(':password'$_POST['password']); // hash this!

      
    $query->execute();
      echo 
    'Registration successful.';

    Could be much cleaner but am typing this on my iPad without an editor so will have to do for now.

  19. #19
    SQL Consultant gold trophysilver trophybronze trophy
    r937's Avatar
    Join Date
    Jul 2002
    Location
    Toronto, Canada
    Posts
    39,338
    Mentioned
    63 Post(s)
    Tagged
    3 Thread(s)
    i don't wanna hammer the point, but using a SELECT before deciding to do the INSERT opens you up to race conditions

    really, the only sane solution is to let the database control uniqueness... and in that case, doing the SELECT before the INSERT is silly
    rudy.ca | @rudydotca
    Buy my SitePoint book: Simply SQL
    "giving out my real stuffs"

  20. #20
    SitePoint Enthusiast
    Join Date
    Jun 2011
    Location
    Singapore
    Posts
    29
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by r937 View Post
    i don't wanna hammer the point, but using a SELECT before deciding to do the INSERT opens you up to race conditions

    really, the only sane solution is to let the database control uniqueness... and in that case, doing the SELECT before the INSERT is silly
    I appreciate this, however if you have two unique fields and try to insert, how can you capture which constraint failed to inform the user? I'm not overly familiar with MySQL's error notification but assume this would throw a 1062 error. Reading the documentation this will provide error message 'Duplicate entry '%s' for key %d'. This should be fairly easy to capture and handle accordingly.

    Godz06, take a look at PDOStatement::errorInfo in the PHP manual.

  21. #21
    From space with love silver trophy
    SpacePhoenix's Avatar
    Join Date
    May 2007
    Location
    Poole, UK
    Posts
    5,072
    Mentioned
    103 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by r937 View Post
    i don't wanna hammer the point, but using a SELECT before deciding to do the INSERT opens you up to race conditions

    really, the only sane solution is to let the database control uniqueness... and in that case, doing the SELECT before the INSERT is silly
    How would doing a SELECT then an INSERT using transactions open it up to "race conditions"?

    From the PHP manual (http://www.php.net/manual/en/pdo.transactions.php)

    You're not limited to making updates in a transaction; you can also issue complex queries to extract data, and possibly use that information to build up more updates and queries; while the transaction is active, you are guaranteed that no one else can make changes while you are in the middle of your work. For further reading on transactions, refer to the documentation provided by your database server.
    Community Team Advisor
    Forum Guidelines: Posting FAQ Signatures FAQ Self Promotion FAQ
    Help the Mods: What's Fluff? Report Fluff/Spam to a Moderator

  22. #22
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by r937 View Post
    You will need to add the username AND the email as unique key.
    wha?
    So, you will have different users with the same email?

    Quote Originally Posted by r937 View Post
    trapping the "duplicate key" database error message and giving the user a friendly message is NOT THAT HARD, guys
    Can you explain, in case you also need (you will need!) the email to be unique?
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel

  23. #23
    SitePoint Addict bronze trophy vectorialpx's Avatar
    Join Date
    Dec 2012
    Location
    Bucharest
    Posts
    247
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    There may be another idea

    Code:
    -- DROP PROCEDURE AddUsername;
    
    DELIMITER |
    CREATE PROCEDURE AddUsername(
    	IN the_username VARCHAR(150), the_email VARCHAR(250),
    	OUT didIt BIGINT(20)
    )
    BEGIN
    DECLARE greenLight INT(1);
    SELECT COUNT(`iduser`) INTO greenLight FROM `users` WHERE `username` = the_username OR `email` = the_email;
    SET didIt = 0;
    IF greenLight = 0 THEN
    INSERT INTO `users` ( `username`, `email` ) VALUES ( the_username, the_email );
    SET didIt = LAST_INSERT_ID();
    END IF;
    END |
    DELIMITER ;
    
    CALL AddUsername( "george", "m@m.com", @wedidit );
    SELECT @wedidit;
    Tested on
    Code:
    CREATE TABLE IF NOT EXISTS `users` (
      `iduser` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
      `username` varchar(150) NOT NULL,
      `email` varchar(250) NOT NULL,
      `name` varchar(150) NOT NULL,
      PRIMARY KEY (`iduser`),
      UNIQUE KEY `uniq_username` (`username`)
    --  , UNIQUE KEY `uniq_email` (`email`) -- may not be unique
    ) ENGINE=InnoDB  DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;
    Be nice to nerds. Chances are you'll end up working for one - Bill Gates
    > photos | admin panel

  24. #24
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,868
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by vectorialpx View Post
    Code:
    -- DROP PROCEDURE AddUsername;
    
    DELIMITER |
    CREATE PROCEDURE AddUsername(
    	IN the_username VARCHAR(150), the_email VARCHAR(250),
    	OUT didIt BIGINT(20)
    )
    BEGIN
    DECLARE greenLight INT(1);
    SELECT COUNT(`iduser`) INTO greenLight FROM `users` WHERE `username` = the_username OR `email` = the_email;
    SET didIt = 0;
    IF greenLight = 0 THEN
    INSERT INTO `users` ( `username`, `email` ) VALUES ( the_username, the_email );
    SET didIt = LAST_INSERT_ID();
    END IF;
    END |
    DELIMITER ;
    
    CALL AddUsername( "george", "m@m.com", @wedidit );
    SELECT @wedidit;
    So now set that up so that several copies of that code run simultaneously so that at least two of them run the SELECT before the first one runs the INSERT.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  25. #25
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    It is NOT good practice to check if a value already exists before inserting it.

    Best practice is to try to insert it and to capture and do appropriate processing of the error if it fails (which might be to ignore the error).

    Apart from the potential timing problems when two people try to insert simultaneously, it is also far more efficient to do one database call than two.
    Quote Originally Posted by r937 View Post
    i don't wanna hammer the point, but using a SELECT before deciding to do the INSERT opens you up to race conditions

    really, the only sane solution is to let the database control uniqueness... and in that case, doing the SELECT before the INSERT is silly
    Let's not try to be so paranoid and watertight where not really required. What are the chances of two different people entering the same username and pressing the submit button within the same fraction of a second? The likelihood is so tiny than worrying about it makes little sense and in most cases this will never happen. And even if it happens (however I think it's more likely to get hit by a meteor falling from the sky) then no big deal - the user will see an error message and will have to try again.

    Doing the INSERT straight away is the best method to let the db handle the uniqueness perfectly 100% of the time but in this particular case it's not really a requirement. And there's also a problem mentioned by someone else here - if you add another unique column to the table for some reason then you can't really tell which constraint has been violated - with SELECT there's no problem with that. In this particular case I don't see a problem with either of these methods - in practice they both will work the same (and the performance difference will be negligeable).

    Quote Originally Posted by felgall View Post
    With username being only 32 characters long it is perfectly appropriate in 99% of cases to use that as a key in other tables associated with the user as well so the only reason for including an id would be if you want people to be able to change their username on a regular basis. If users will only rarely change their username then you can use the username as the primary key and handle the change as a rare exception condition - the way almost all forum software does.
    I'd say keeping the username as PK is a bad idea and changing usernames is the least problem (which is handled automatically by ON UPDATE CASCADE). In most cases the user's PK is used across many other tables as a FK so you need to keep all those columns exactly the same type. Imagine one day you want to change the maximum length of the username? Or even character set collation? MySQL will not allow you to simply ALTER TABLE because there will be a mismatch between types with all the corresponding foreign keys. You also can't ALTER TABLE the foreign keys for the same reason. In such a case you need to DROP all these foreign keys, change all the column types and add the foreign keys back - maintenance nightmare!


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •