mnwuzor Posted October 1, 2015 Share Posted October 1, 2015 (edited) Hello Friends, Please I solicit your kind review. I wrote the code below to help me check for duplicate record entry so my users will not enter same record twice in a year buts the code is neither posting the record not checking if there is any duplicate entries. Here is how the system work. The application on its own allows multiple entries but once in a year. Every year members of a church gather for a conference and the system is used to capture their details. So for every participants, the system will capture their detail for that year once. If a participants details is entered twice the system should check the RegNo and Year fields to be sure the record has not been posted for that year. But its not working Please I need help on this. if(isset($_POST['submit'])) { { $con = mysql_connect('localhost', 'root', '', 'rmwworg_dbms') or die('Error connecting to MySQL server'); $Camp = $_POST['Camp']; $Hostel = $_POST['Hostel']; $Validate = $_POST['Validate']; $Event_Title = $_POST['Event_Title']; $Regno = $_POST['regno']; $Title = $_POST['Title']; $Surname = $_POST['surname']; $OtherNames = $_POST['OtherNames']; $Address = $_POST['Address']; $City = $_POST['City']; $Age = $_POST['Age']; $State = $_POST['State']; $Country = $_POST['Country']; $Assembly= $_POST['Assembly']; $Sex = $_POST['Sex']; $MaritalStatus = $_POST['MaritalStatus']; $Phone = $_POST['Phone']; $Email = $_POST['Email']; $photo2 = $_POST['photo2']; $datetime = $_POST['datetime']; $Year = $_POST['Year']; $validatedby = $_POST['validatdby']; $category = $_POST['category']; $result = mysql_query("SELECT * FROM validatedparticipants WHERE RegNo = '$regno' AND Year = year(now())"); //check for duplicates $num_rows = mysql_num_rows($result); //number of rows where duplicates exist if($num_rows==0) { //if there are no duplicates...insert { $sql="INSERT INTO validatedparticipants (Camp, Hostel, Validate, Event_Title, RegNo, Title, Surname, OtherNames, Address, City, Age, State, Country, Assembly, Sex, MaritalStatus, Phone, Email, photo2, datetime, Year, validatdby, category) VALUES ('$Camp', '$Hostel', '$Validate', '$Event_Title', '$regno', '$Title', '$surname', '$OtherNames', '$Address', '$City', '$Age', '$State', '$Country', '$Assembly', '$Sex', '$MaritalStatus', '$Phone', '$Email', '$photo2', '$datetime','$Year','$validatedby','$category')"; $result1 = mysql_query($con, $sql) or die('Error querying database.'); mysql_close($con); } } else { echo"The Participant added"; } } header("valpartcard.php?RegNo=" . $row_rsValidateParticipant['RegNo'] . ""); } Edited October 1, 2015 by QuickOldCar Quote Link to comment Share on other sites More sharing options...
scootstah Posted October 1, 2015 Share Posted October 1, 2015 You have many issues with this code. 1. You should not be using the mysql_* library. It is deprecated and will soon be removed entirely from PHP. You should be using PDO. 2. You need to be escaping input that is used in queries, otherwise you are open to SQL injection attacks. You should wrap all input that gets used in a query in mysql_real_escape_string(). Or, if you're using PDO you would use a prepared statement. 3. You have some weird thing going on with using extra braces inside conditionals. This is unnecessary and adds clutter to your code. 4. The header() call at the bottom is supposed to be a redirect I assume? You do not have the correct syntax there - a redirect header needs to start with Location: . Also, you should put an exit() after a header redirect to prevent the rest of the script from executing. I've revised your code to make it more readable and fix some of the things I pointed out above. if (isset($_POST['submit'])) { $con = mysql_connect('localhost', 'root', '', 'rmwworg_dbms') or die('Error connecting to MySQL server'); $Camp = mysql_real_escape_string($_POST['Camp']); $Hostel = mysql_real_escape_string($_POST['Hostel']); $Validate = mysql_real_escape_string($_POST['Validate']); $Event_Title = mysql_real_escape_string($_POST['Event_Title']); $Regno = mysql_real_escape_string($_POST['regno']); $Title = mysql_real_escape_string($_POST['Title']); $Surname = mysql_real_escape_string($_POST['surname']); $OtherNames = mysql_real_escape_string($_POST['OtherNames']); $Address = mysql_real_escape_string($_POST['Address']); $City = mysql_real_escape_string($_POST['City']); $Age = mysql_real_escape_string($_POST['Age']); $State = mysql_real_escape_string($_POST['State']); $Country = mysql_real_escape_string($_POST['Country']); $Assembly= mysql_real_escape_string($_POST['Assembly']); $Sex = mysql_real_escape_string($_POST['Sex']); $MaritalStatus = mysql_real_escape_string($_POST['MaritalStatus']); $Phone = mysql_real_escape_string($_POST['Phone']); $Email = mysql_real_escape_string($_POST['Email']); $photo2 = mysql_real_escape_string($_POST['photo2']); $datetime = mysql_real_escape_string($_POST['datetime']); $Year = mysql_real_escape_string($_POST['Year']); $validatedby = mysql_real_escape_string($_POST['validatdby']); $category = mysql_real_escape_string($_POST['category']); $result = mysql_query("SELECT * FROM validatedparticipants WHERE RegNo = '$regno' AND Year = year(now())"); //check for duplicates $num_rows = mysql_num_rows($result); //number of rows where duplicates exist if ($num_rows==0) { //if there are no duplicates...insert $sql="INSERT INTO validatedparticipants (Camp, Hostel, Validate, Event_Title, RegNo, Title, Surname, OtherNames, Address, City, Age, State, Country, Assembly, Sex, MaritalStatus, Phone, Email, photo2, datetime, Year, validatdby, category) VALUES ('$Camp', '$Hostel', '$Validate', '$Event_Title', '$regno', '$Title', '$surname', '$OtherNames', '$Address', '$City', '$Age', '$State', '$Country', '$Assembly', '$Sex', '$MaritalStatus', '$Phone', '$Email', '$photo2', '$datetime','$Year','$validatedby','$category')"; $result1 = mysql_query($con, $sql) or die('Error querying database.'); } else { echo"The Participant added"; } header("Location: valpartcard.php?RegNo=" . $row_rsValidateParticipant['RegNo']); exit; }Note that there is no variable called $row_rsValidateParticipant defined in the code you gave. Either you defined that elsewhere or you're trying to use an undefined variable. So what is not working exactly? You're not getting any results from this query when you think you should be? SELECT * FROM validatedparticipants WHERE RegNo = '$regno' AND Year = year(now())What type is the "Year" column in your database? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 1, 2015 Share Posted October 1, 2015 Actually, this whole approach of checking for duplicates is unnecessarily fragile and cumbersome. If you want the pairs (regno, year) to be unique, simply add a UNIQUE constraint to your table: ALTER TABLE validatedparticipants ADD CONSTRAINT UNIQUE (regno, year); It might even be appropriate to use this as the primary key. Now the database system will actually enforce unique combination. Your application no longer has to worry about duplicates. It can simply try to insert the row, and if that fails due to a constraint violation, MySQL will trigger an error which can be handled the application. I'm not familiar with the old mysql_* functions, but if you follow scootstah's advice and switch to PDO, the code will look like this: Setting up the database connection: <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; // MySQL error codes const MYSQL_ER_DUP_UNIQUE = '23000'; // violation of UNIQUE constraint $dSN = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHARSET; // set up the database connection $databaseConnection = new PDO($dSN, DB_USER, DB_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // activate prepared statements PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // activate exceptions PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this isn't critical) ]); Then you try to insert the row and handle possible errors: <?php require_once __DIR__.'/database.php'; $participantRegistrationStmt = $databaseConnection->prepare(' INSERT INTO validatedparticipants SET camp = :camp, hostel = :hostel, ... '); // Try to insert the row; if the participant is already registered, this will trigger an exception try { $participantRegistrationStmt->execute([ 'camp' => $_POST['camp'], 'hostel' => $_POST['hostel'], // ... ]); } catch (PDOException $participantRegistrationError) { // If the exception was caused by a constraint violation, tell the user about the duplicate name; otherwise propagate the exception. if ($participantRegistrationError->errorCode() === MYSQL_ER_DUP_UNIQUE) { echo "You're already registered."; } else { throw $participantRegistrationError; } } This will work reliably under all circumstances, and you only need a single query. Sure, you could try to emulate this behaviour in the application by first checking if the row already exists, but this has a lot of problems: There's a certain gap between your two queries. If a duplicate request arrives within this time frame, your application will accept it, because it doesn't “see” the previous row yet. In the end, you'll have the same row twice despite your check. Applications have bugs (as you already saw), so your check may simply fail to work and leave you with tons of garbage data. If the data is edited manually (be it by you, your coworker, you client, whatever), there are no checks whatsoever. So there's a fairly big risk that duplicate data will be introduced through human error. Two queries instead of one are unnecesary bloat. Quote Link to comment Share on other sites More sharing options...
mnwuzor Posted October 1, 2015 Author Share Posted October 1, 2015 Dear scootstah Thank you so much. I am really grateful. I will test code and get back to you. The form wasnt posting the records to MySQL database. The Year field type is a "year". The Year field is used to filter all validated participants. Validated participants are those that were marked present at the conference. So when the system administrators wants to analyze the attendance, they will use analyze based on the very years event. Now when a participant tries to register twice, the system will check if he has registered earlier. The criteria's for checking are his RegNo and Year of event. You have many issues with this code.1. You should not be using the mysql_* library. It is deprecated and will soon be removed entirely from PHP. You should be using PDO.2. You need to be escaping input that is used in queries, otherwise you are open to SQL injection attacks. You should wrap all input that gets used in a query in mysql_real_escape_string(). Or, if you're using PDO you would use a prepared statement.3. You have some weird thing going on with using extra braces inside conditionals. This is unnecessary and adds clutter to your code.4. The header() call at the bottom is supposed to be a redirect I assume? You do not have the correct syntax there - a redirect header needs to start with Location:. Also, you should put an exit() after a header redirect to prevent the rest of the script from executing.I've revised your code to make it more readable and fix some of the things I pointed out above. if (isset($_POST['submit'])) { $con = mysql_connect('localhost', 'root', '', 'rmwworg_dbms') or die('Error connecting to MySQL server'); $Camp = mysql_real_escape_string($_POST['Camp']); $Hostel = mysql_real_escape_string($_POST['Hostel']); $Validate = mysql_real_escape_string($_POST['Validate']); $Event_Title = mysql_real_escape_string($_POST['Event_Title']); $Regno = mysql_real_escape_string($_POST['regno']); $Title = mysql_real_escape_string($_POST['Title']); $Surname = mysql_real_escape_string($_POST['surname']); $OtherNames = mysql_real_escape_string($_POST['OtherNames']); $Address = mysql_real_escape_string($_POST['Address']); $City = mysql_real_escape_string($_POST['City']); $Age = mysql_real_escape_string($_POST['Age']); $State = mysql_real_escape_string($_POST['State']); $Country = mysql_real_escape_string($_POST['Country']); $Assembly= mysql_real_escape_string($_POST['Assembly']); $Sex = mysql_real_escape_string($_POST['Sex']); $MaritalStatus = mysql_real_escape_string($_POST['MaritalStatus']); $Phone = mysql_real_escape_string($_POST['Phone']); $Email = mysql_real_escape_string($_POST['Email']); $photo2 = mysql_real_escape_string($_POST['photo2']); $datetime = mysql_real_escape_string($_POST['datetime']); $Year = mysql_real_escape_string($_POST['Year']); $validatedby = mysql_real_escape_string($_POST['validatdby']); $category = mysql_real_escape_string($_POST['category']); $result = mysql_query("SELECT * FROM validatedparticipants WHERE RegNo = '$regno' AND Year = year(now())"); //check for duplicates $num_rows = mysql_num_rows($result); //number of rows where duplicates exist if ($num_rows==0) { //if there are no duplicates...insert $sql="INSERT INTO validatedparticipants (Camp, Hostel, Validate, Event_Title, RegNo, Title, Surname, OtherNames, Address, City, Age, State, Country, Assembly, Sex, MaritalStatus, Phone, Email, photo2, datetime, Year, validatdby, category) VALUES ('$Camp', '$Hostel', '$Validate', '$Event_Title', '$regno', '$Title', '$surname', '$OtherNames', '$Address', '$City', '$Age', '$State', '$Country', '$Assembly', '$Sex', '$MaritalStatus', '$Phone', '$Email', '$photo2', '$datetime','$Year','$validatedby','$category')"; $result1 = mysql_query($con, $sql) or die('Error querying database.'); } else { echo"The Participant added"; } header("Location: valpartcard.php?RegNo=" . $row_rsValidateParticipant['RegNo']); exit; }Note that there is no variable called $row_rsValidateParticipant defined in the code you gave. Either you defined that elsewhere or you're trying to use an undefined variable.So what is not working exactly? You're not getting any results from this query when you think you should be? SELECT * FROM validatedparticipants WHERE RegNo = '$regno' AND Year = year(now())What type is the "Year" column in your database? Quote Link to comment Share on other sites More sharing options...
mnwuzor Posted October 1, 2015 Author Share Posted October 1, 2015 Dear Jacques1, Thank you so much for your contributions. Regarding using UNIQUE RegNo and Year, the system is build to accept same set of people every year. So if I use UNIQUE feature it will hinder participants from registering in the future. Thats why I am using the RegNo and Year fields to check for duplicate entries per year. Thank you very much for the clean code you wrote our there for me. I have to go back to my study table to update myself on this. Thank you so much Actually, this whole approach of checking for duplicates is unnecessarily fragile and cumbersome. If you want the pairs (regno, year) to be unique, simply add a UNIQUE constraint to your table: ALTER TABLE validatedparticipants ADD CONSTRAINT UNIQUE (regno, year); It might even be appropriate to use this as the primary key. Now the database system will actually enforce unique combination. Your application no longer has to worry about duplicates. It can simply try to insert the row, and if that fails due to a constraint violation, MySQL will trigger an error which can be handled the application. I'm not familiar with the old mysql_* functions, but if you follow scootstah's advice and switch to PDO, the code will look like this: Setting up the database connection: <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; // MySQL error codes const MYSQL_ER_DUP_UNIQUE = '23000'; // violation of UNIQUE constraint $dSN = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHARSET; // set up the database connection $databaseConnection = new PDO($dSN, DB_USER, DB_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // activate prepared statements PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // activate exceptions PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this isn't critical) ]); Then you try to insert the row and handle possible errors: <?php require_once __DIR__.'/database.php'; $participantRegistrationStmt = $databaseConnection->prepare(' INSERT INTO validatedparticipants SET camp = :camp, hostel = :hostel, ... '); // Try to insert the row; if the participant is already registered, this will trigger an exception try { $participantRegistrationStmt->execute([ 'camp' => $_POST['camp'], 'hostel' => $_POST['hostel'], // ... ]); } catch (PDOException $participantRegistrationError) { // If the exception was caused by a constraint violation, tell the user about the duplicate name; otherwise propagate the exception. if ($participantRegistrationError->errorCode() === MYSQL_ER_DUP_UNIQUE) { echo "You're already registered."; } else { throw $participantRegistrationError; } } This will work reliably under all circumstances, and you only need a single query. Sure, you could try to emulate this behaviour in the application by first checking if the row already exists, but this has a lot of problems: There's a certain gap between your two queries. If a duplicate request arrives within this time frame, your application will accept it, because it doesn't “see” the previous row yet. In the end, you'll have the same row twice despite your check. Applications have bugs (as you already saw), so your check may simply fail to work and leave you with tons of garbage data. If the data is edited manually (be it by you, your coworker, you client, whatever), there are no checks whatsoever. So there's a fairly big risk that duplicate data will be introduced through human error. Two queries instead of one are unnecesary bloat. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 1, 2015 Share Posted October 1, 2015 You might have misunderstood my suggestion regarding the UNIQUE constraint: I'm not talking about two individual constraints for the two columns (that of course wouldn't work), I'm talking about a single constraint which covers both columns. So this constraint makes sure that the combination of regno and year is unique. That's the same check you currently do in your code, but without the problems of your approach. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.