What Have I Done Wrong?

Question

What Have I Done Wrong ?

$addedby=$_SESSION['id'];

$text="Subject: I want to add a New Phone Name :$name   Address :$address   Phone :$phone                 Category :$category   Email :$email   Website :$website    Details :$details";
$sql = "INSERT INTO messages SET  text='$text' , from='$addedby'";
mysql_query($sql) or die('Website Under Maintanance '.mysql_error());

The Id is integer and the from is varchar(30).

Please Tell Me what have I done wrong ?

The Error Is :

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'from='1'

As far as I can tell, the thing you've done wrong is that people can't easily figure out what your question actually ***is***. — Michael Dautermann, Nov 19 '12 at 09:49
Apart from that, the SQL query is syntactically invalid, which would be also apparent from the error message you are getting. Reading [the manual](http://dev.mysql.com/doc/refman/5.5/en/insert.html) is kind of a prerequisite for working with anything. — Jon, Nov 19 '12 at 09:50
`from` is a reserved keyword and needs escaping. As @Lawrence Cherone points out in another comment `INSERT into ... SET` is in fact valid in MySQL — Martin Smith, Nov 19 '12 at 10:00

score 3 · Answer 1 · answered Nov 19 '12 at 09:50

You should try

$sql = "INSERT INTO messages (`text`,`from`) values ('$text' , '$addedby')";

And please note, you should always clean your manual inputs before assembling an SQL query out of them, because SQL injection is very nasty, and makes it very easy to do anything with your DB...

score 3 · Answer 2 · edited May 23 '17 at 11:43

3

Here is the correct INSERT syntax:

INSERT INTO messages(`text`, `from`) VALUES('$text', '$addedby');

Note that: Your code this way, is vulnerable for SQL Injection use PDO Or prepared statments instead. See the following post for more information:

Best way to prevent SQL injection?

edited May 23 '17 at 11:43

Community

1
1

answered Nov 19 '12 at 09:50

Mahmoud Gamal

78,257
17
139
164

Kishor · Answer 3 · 2012-11-19T10:05:54.023

1

$sql = "INSERT INTO messages SET text='$text' , from='$addedby'";

Should be

$sql = "INSERT INTO messages (`text`,`from`) VALUES('$text','$addedby')";

And please start using mysqli or PDO instead of Mysql. Its being deprecated.

You should seriously go through some examples and stuff about SQL.

edited Nov 19 '12 at 10:05

answered Nov 19 '12 at 09:51

Kishor

1,513
2
15
25

1

`INSERT into * SET` is a mysql extension [Prev Answer](http://stackoverflow.com/questions/861722/mysql-insert-into-table-values-vs-insert-into-table-set) http://dev.mysql.com/doc/refman/5.5/en/insert.html – Lawrence Cherone Nov 19 '12 at 09:55
Pointed taken, edited the answer so as to not provide wrong infos! :) – Kishor Nov 19 '12 at 10:05

score 1 · Answer 4 · answered Nov 19 '12 at 09:53

What have you done wrong:

You didn't provide enough information to answer the question.
You don't use PDO instead of mysql_query.
Your SQL is (probably) vulnerable to SQL injection.
You use or die construct instead of Exceptions.
You show the user MySQL error message.
You mix syntax of INSERT and UPDATE query.

The query that you want is (provided that $text and $addedby are propertly escaped to prevent SQL injection)

INSERT INTO messages (`text`, `from`) VALUES ('$text', '$addedby')

What Have I Done Wrong?

4 Answers4