Error: Duplicate entry '' for key 4

[elnasiru96]

pls im having an "(Error: Duplicate entry '' for key 4)" on testing registration of new members on my website through thier email addresses. any helps on how to avoid this annoying sting
? thanx

 

[AngusThermopyle]

Test for the presence of the value before you try to insert.

 

[misson]

Show us the code (in particular, a minimal test case consisting of PHP and SQL) and DB structure. Otherwise, we can only guess as to cause.

So you don't increase load on the MySQL server, rather than issuing additional queries you can suppress or disable error messages and test the result of running the insertion query. If the query fails due to a duplicate key, inform the user that that particular e-mail address (or whatever duplicate datum) is already registered, perhaps including a link to your password reset system.

 

[elnasiru96]

here's a brief on the script that's generating the error.

##User isn't registering, check verify code and change activation code to null, status to activated on success

$queryString = $_SERVER['QUERY_STRING'];

$query = "SELECT * FROM users";

$result = mysqli_query($link,$query) or die(mysqli_error($link));

while($row = mysqli_fetch_array($result)){

if ($queryString == $row["activationkey"]){

echo "Congratulations!" . $row["username"] . " is now the proud new owner of a carrygo.com account.";

$sql="UPDATE users SET activationkey = '', status='activated' WHERE (id = $row[id])";

if (!mysqli_query($link,$sql))

 

[chankarwing]

since each users will be unique. you should use auto increment.

 

[misson]

Please use

, [html] or [code] tags (as appropriate) to separate and formate code.

[quote="elnasiru96, post: 725249"]here's a brief on the script that's generating the error.[/QUOTE]
What about the table structure? Is column [FONT="Courier New"]activationkey[/FONT] unique? Is it in the primary index? Is it allowed to be [FONT="Courier New"]Null[/FONT]?

[quote="elnasiru96, post: 725249"][php]
##User isn't registering, check verify code and change activation code to null, status to activated on success

$queryString = $_SERVER['QUERY_STRING'];
$query = "SELECT * FROM users";

[/QUOTE]
Don't use SELECT *; select only the columns you need. The statement also needs a WHERE clause. Otherwise, you're fetching every entry in the `users` table, which is a terrible waste of server resources.

$userRegisteredQuery = mysqli_prepare('SELECT COUNT(*) FROM users WHERE activationkey = ?');
$userRegisteredQuery->bind_param('s', $_SERVER['QUERY_STRING']);

$result = mysqli_query($link,$query) or die(mysqli_error($link));

Don't use die when outputting HTML. Don't output MySQL error messages to non-administrators; it reveals too much information.



The code can be simplified to use a single query. I also recommend switching to PDO, which is simpler to use and more powerful than mysqli.

<?php
$activate = $db->prepare("UPDATE users 
        SET activationkey = NULL, status='activated' 
        WHERE activationkey = ?");
if ($activate->execute(array($_SERVER['QUERY_STRING']))) {
    if ($activate->rowCount()) {
        echo "Congratulations! You are now the proud new owner of a carrygo.com account.";
        // log in user?
        ...
    } else { // no affected rows. 
        /* Most likely an invalid activation code, or account is already activated.
           Include link to password reset & possibly a way to check if they're registered.
           The password reset mechanism may also function with the latter purpose.
         */
        ?><p>I couldn't find that activation code. You might have already activated 
        your account. If you've forgotten your password [...]
        </p><?php
    }
} else { // execution failed
  ?><p>There was an internal error trying to activate your account. It's been logged, 
  and we'll log into it. Please give us some time to fix it and try again later.</p><?php
  $error = $activate->errorInfo();
  error_log("Activation for '$_SERVER[QUERY_STRING]' failed: $error[2] ($error[0]/$error[1]).")
  ...
}

 

[elnasiru96]

here's the table structure

CREATE TABLE IF NOT EXISTS `users` (
`id` int(11) NOT NULL auto_increment,
`status` varchar(20) NOT NULL,
`username` varchar(20) NOT NULL,
`password` varchar(20) NOT NULL,
`email` varchar(20) NOT NULL,
`activationkey` varchar(100) NOT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `username` (`username`),
UNIQUE KEY `email` (`email`),
UNIQUE KEY `activationkey` (`activationkey`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=9 ;

 

[descalzo]

UNIQUE KEY `activationkey` (`activationkey`)

There is your problem. KEY, ok. UNIQUE, not ok.

When you activate the user, you set the activationkey to an empty string.
The second time you activate a user, the field is not unique.

1. Change the KEY so it is not UNIQUE
2. Don't null out the KEY
3. Put something else 'unique' in as a replacement (convert the ID to a string maybe).

 

[misson]

I'm going to have to disagree with Descalzo about the activation key being unique in the current design. Notionally, the activation key is unique to an account, since it's sufficient to identify an unactivated account; it's a candidate key. If the activation key and some other piece of identifying information (e.g. e-mail, username) were required, the activation key wouldn't be a candidate key. Moreover, the DB should enforce uniqueness. Declaring the column in a unique index is thus appropriate.

Removing the UNIQUE attribute from the activation key index isn't necessary to resolve the issue. Storing some other unique data (such as Descalzo suggests) would work, but someone could potentially re-activate their account using the new data (not that this would be problematic, though it is odd). Allowing the column to store NULL values would also work, since NULL doesn't equal itself using the standard comparison operators in MySQL. Nulling the activation key for activated accounts also lets you change the column type to a constant width CHAR type (since it's a fair assumption that activation keys are all of a length), which offers some performance advantages.

A third (and better) approach is to separate out the activation data. One reason it's better is that storing the activation key in the user table is wasteful.

One particular approach is to keep a table of pending user accounts. When an account is activated, move the entry from the `pending` table to the `users` table.

INSERT INTO users SELECT username, password, email, status FROM pending WHERE activationkey = ?;
-- if the above succeeds
DELETE FROM pending WHERE activationkey = ?;

Alternatively, the pending table could store the activation key and the ID of the user to be activated, with the rest of the user data in the `users` table. One downside to this is that corruption of the pending table will essentially authorize pending users.

Depending on how the authentication and authorization system is designed, each option will have other benefits and detriments. With the first option, the default behavior is that unactivated users aren't given access; with the latter, unactivated users can log in without special consideration.

 

[sith]

I agree with descalzo you should remove UNIQUE KEY `activationkey` (`activationkey`)

here's the table structure
CREATE TABLE IF NOT EXISTS `users` (
`id` int(11) NOT NULL auto_increment,
`status` varchar(20) NOT NULL,
`username` varchar(20) NOT NULL,
`password` varchar(20) NOT NULL,
`email` varchar(20) NOT NULL,
`activationkey` varchar(100) NOT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `username` (`username`),
UNIQUE KEY `email` (`email`),
UNIQUE KEY `activationkey` (`activationkey`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=9 ;

Just create new table, so it will be like this :

CREATE TABLE IF NOT EXISTS `users` (
`id` int(11) NOT NULL auto_increment,
`status` varchar(20) NOT NULL,
`username` varchar(20) NOT NULL,
`password` varchar(20) NOT NULL,
`email` varchar(20) NOT NULL,
`activationkey` varchar(100) NOT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `username` (`username`),
UNIQUE KEY `email` (`email`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1;

Have a nice day.