-2

I'm using the following code to add a row of data into a MySQL database. I'm getting an odd error though. The error suggests I'm trying to add data into a column that doesn't exist called "Nathan", but that's the value of $fName. Is VALUES not where I put the data that I want inserted into the table? Heres my relevant code, thanks!

// Create connection
$conn = new mysqli($servername, $username, $password, $username);
// Check connection
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
} 
echo "Connected successfully";

//Column names are FIRSTNAME, LASTNAME, TEACHERNAM, MSIPROOM
$sql = "INSERT INTO PerA (FIRSTNAME, LASTNAME, TEACHERNAME, MSIPROOM) VALUES ($fName, $lName, $tName, $mRoom)";

if ($conn->query($sql) === TRUE) {
    echo "New record created successfully";
} else {
    echo "Error: " . $sql . "<br>" . $conn->error;
}
Nat
  • 890
  • 3
  • 11
  • 23
  • For a start, print out your query before executing it. That's basic debugging 101. And _show_ us the error, don't just describe it. – paxdiablo Feb 12 '15 at 22:24
  • Appologies, I meant to post the error but I somehow forgot... I started sql and php only a few days ago, so I'll try to remember to print the query, thank you! – Nat Feb 12 '15 at 22:33

4 Answers4

2

String literals need to enclosed in quotes; SQL standard is to use single quotes. (By default, MySQL also accepts double quotes, but for maximum portability, I recommend using single quotes.)

INSERT INTO mytable (mycolumn) VALUES ('myvalue')
                                       ^       ^ 

The error MySQL is returning indicates that it thinks you are supplying an identifier (a column name) rather than a literal value.

Note that putting values into SQL text makes your code vulnerable to SQL Injection. If you are going to do that, the values need to be appropriately escaped. (I think the function you'd want to use is mysqli_real_escape_string.)

But don't do that.

For new development, I strongly recommend you consider using prepared statements with bind placeholders. Then you won't need to "escape" the values...

e.g.

 $sth = $dbh->prepare("INSERT INTO mytable (mycolumn) VALUES ( :fname )";
 $sth->bindParam(':fname',$fname);
 $sth->execute();
spencer7593
  • 106,611
  • 15
  • 112
  • 140
  • Ah, I didn't think they would need quotes since in the other languages I've used, you don't need to surround a string var with quotes. Also thanks for the info about escaping, I'm going to try to get this working first and then I'll definetly try to take the appropriate security measures including yours. thanks! – Nat Feb 12 '15 at 22:36
  • 3
    @Nathan I'd skip straight to prepared statements. Aside from being more secure, they're honestly easier to use, because you don't have to worry about stuff like quotes. – Chris Hayes Feb 12 '15 at 22:40
  • Ok ill look into them! – Nat Feb 12 '15 at 22:45
  • Hear, Hear! @ChrisHayes gives **sound** advice. Learn to use **prepared statements** with **bind placeholders**. That is best practice; and they really are *easier*. Using prepared statements gives *much* better performance (in most RDBMS, and even in MySQL if you enable server side prepared statements). And your DBAs will postively laud you for reusing *static* SQL text, rather than sending a bloatload of one off statements. Escaping strings was the "next best" we could do with the PHP mysql interface, but that was eons ago. Both mysqli and PDO support prepared statements. – spencer7593 Feb 12 '15 at 22:46
1

You have to quote your values, like:

$sql = "INSERT INTO PerA 
               (FIRSTNAME, LASTNAME, TEACHERNAME, MSIPROOM) 
        VALUES ('$fName', '$lName', '$tName', '$mRoom')";

or, in case the values contain quotes themselves:

    $sql = sprintf(
        "INSERT INTO PerA (FIRSTNAME, LASTNAME, TEACHERNAME, MSIPROOM) 
         VALUES (%s, %s, %s, %s)",
         mysql_real_escape_string($fName),
         mysql_real_escape_string($lName),
         mysql_real_escape_string($tName),
         mysql_real_escape_string($mRoom)
   );
Blablaenzo
  • 1,027
  • 1
  • 11
  • 14
  • If we're not *certain* that the values being incorporated into the statement aren't *safe*, those values absolutely, positively *must* be escaped. OP shows he's using **mysqli** interface, so he would need to use `mysqli_real_escape_string` (not `mysql_real_escape_string`, and the SQL text would still need to include the single quotes. (The sprintf example in this answer doesn't include single quotes around the string literals... same problem OP is having.) But all of that aside, best practice dictates using **prepared statements** with **bind_placeholders**. – spencer7593 Feb 12 '15 at 23:12
0

Probably erroring due to things in your string values.

$sql = "INSERT INTO PerA (FIRSTNAME, LASTNAME, TEACHERNAME, MSIPROOM) VALUES ('".$fName."', '".$lName."', '".$tName."', '".$mRoom."')";

also you should get in to the habit of escaping when you insert in to a database, even if you're supplying the data, but that's a different question.

jdu
  • 581
  • 2
  • 3
  • Don't give advice that's only _half_ good :-) OP should be using parametrised queries, escaping data is an improvement but can introduce other problems. – paxdiablo Feb 12 '15 at 22:26
  • Yes, string literals appearing in SQL text do need to be enclosed in quotes. But that value of the variable being incorporated into the SQL text may also contain single quotes. The code shown in this answer is potentially vulnerable to SQL Injection. – spencer7593 Feb 12 '15 at 22:41
0

Try to put the values in quote

$sql = "INSERT INTO PerA (FIRSTNAME, LASTNAME, TEACHERNAME, MSIPROOM) VALUES ('$fName', '$lName', $tName, '$mRoom')";
Obi Ik
  • 159
  • 2
  • 9
  • Yes, string literals need to be enclosed in quotes. But that value being incorporated into the SQL text may also contain single quotes. The code shown in this answer is potentially vulnerable to **SQL Injection**. – spencer7593 Feb 12 '15 at 22:40