2

I am getting an exception:

Incorrect syntax near ','.

in following query: please help.

I have tried it in many ways but not it does not work. Even when I try to insert a single value I still get this exception.

cmd = new SqlCommand("insert into purchaseOrder_master(sup_id,po_date,required_date,tot_amt,uid,potime) values("+supid.Text +",'"+podate.Value.Date+"','"+reqdate.Value.Date+"',"+pocost.Text +","+uid.Text +",'"+potime.Value.TimeOfDay  +"')", con);
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Programmer
  • 63
  • 2
  • 8
  • 1
    A small tip: Create the SQL string separately, then you can use a debugger to see its value (or just print it out). It will then, hopefully, be pretty obvious what's wrong with the SQL statement in the string. Or create the query using proper parameters as suggested otherwise. – Some programmer dude Aug 29 '13 at 06:39
  • 1
    first have to say this is not good way to insert record. u better use cmd.parameterd.add to pass parameters. then what is the data type of po_date? – DevT Aug 29 '13 at 06:39
  • @DevT po_date is datetime – Programmer Aug 29 '13 at 08:01

7 Answers7

5

How often do we have to repeat that you should use parameterized queries?!

using (SqlCommand cmd = new SqlCommand("insert into table (column) values (@param)", conn))
{
  cmd.Parameters.AddWithValue("@param", value);
  cmd.ExecuteNonQuery();
}

The problems with your code are:

  1. Wide open to SQL injections
  2. DateTime values will not be handled correctly (probably the problem right now)
  3. Strings must be handled properly (quotes)
Thorsten Dittmar
  • 55,956
  • 8
  • 91
  • 139
  • For ever. New users to SQL won't have heard you the other times. Frustrating, but `I've told a hundred other people this!` isn't really going to matter to the 101st user... :( – MatBailie Aug 29 '13 at 06:47
  • I know. The question is rather rhetoric. Just as the question *Why didn't you search SO before asking a question that has been asked 100 times?*. – Thorsten Dittmar Aug 29 '13 at 06:48
  • Because new users don't even know what to search for. :( – MatBailie Aug 29 '13 at 06:49
  • @MatBailie That is sort of a rediculous point, don't you think? – Thorsten Dittmar Aug 29 '13 at 06:53
  • @Thorsten Dittmar Thank you so much for your suggestions now onwards i will use parametrized queries and thanks for quick response. – Programmer Aug 29 '13 at 07:32
  • @MatBailie Thank you so much for understanding the problem of new users. – Programmer Aug 29 '13 at 07:43
  • @MatBailie I'd like to elaborate a bit why I think your point is not valid: It's not a matter of whether you know how to use SO or not. Every programmer should be able to use Google to search for solutions to his problems. Usually you would start with a very specific query, which then gets broader and broader until you find a result that helps you. The Google query *C# SqlCommand INSERT exception* brings up this as first result: http://stackoverflow.com/questions/12939501/insert-into-c-sharp-with-sqlcommand. So if you can't even Google properly...? – Thorsten Dittmar Aug 29 '13 at 08:59
2
cmd = new SqlCommand("insert into PurchaseOrder_master(sup_id) values(@val)", con);    
cmd.parameters.AddWithValue("@val", supid.Text );

please check data type of date as well. it seems you passing just string for this. if its date time then you need parse string to datetime. for that conversion use, 

 DateTime.Parse("stringValue");

if you want to save it without command parameters, then convert your date value and check as mentioned above.

DevT
  • 4,843
  • 16
  • 59
  • 92
0

Check out your actual SQL string.

Probably one of your values is an empty string.

That, and what Thorsten said. Use parametrized queries.

Bart Friederichs
  • 33,050
  • 15
  • 95
  • 195
0

Change 

"+uid.Text +"

to 

'"+uid.Text +"'

and 

"+supid.Text +"

to 

'"+supid.Text +"'

so that

values("+supid.Text +",'"+podate.Value.Date+"','"+reqdate.Value.Date+"',"+pocost.Text +","+uid.Text +",'"+potime.Value.TimeOfDay  +"')

reads

values('"+supid.Text +"','"+podate.Value.Date+"','"+reqdate.Value.Date+"',"+pocost.Text +",'"+uid.Text +"','"+potime.Value.TimeOfDay  +"')
Thorsten Dittmar
  • 55,956
  • 8
  • 91
  • 139
Giannis Paraskevopoulos
  • 18,261
  • 1
  • 49
  • 69
  • 4
    No, don't do that. Use parameterized queries. – Thorsten Dittmar Aug 29 '13 at 06:40
  • 1
    @ThorstenDittmar I don't argue about being right or wrong. But there are times you just cannot do otherwise. SO has expressed a specific issue, and this resolves it. He didn't ask if it is right or wrong... – Giannis Paraskevopoulos Aug 29 '13 at 06:42
  • 1
    And I tend to point people, who's code is showing deeper problems than just the one they want a solution for, to a better solution. Using parameterized queries here solves his problem as well as your solution (probably), but also resolves most other problems. If you ask me why your bubble sort implementation doesn't work, I will tell you, but I'll also tell you why bubble sort is not a good idea. – Thorsten Dittmar Aug 29 '13 at 06:51
  • 1
    @ThorstenDittmar The reason i haven't actually delete my post is because it really gives another point of view. For instance, it is good to show the asker what the problem is. From your point of view the problem is that he is not using parameterized queries. That doesn't explain though why he got his error... Keeping this post live will at least show him what else was wrong apart from a wrong approach. – Giannis Paraskevopoulos Aug 29 '13 at 06:57
  • @ThorstenDittmar agreed to what you have pointed out, but if you have downvoted for this, then i think it is unfair. Because, the question is not for code review. it is for solution and this does resolves the issue. obviously not the best way, but it does. – Ehsan Aug 29 '13 at 06:57
  • @NoOne Valid point, I'm taking back the downvote, as it does solve the issue (sorry, I had to edit the answer slightly so I could take back the downvote). – Thorsten Dittmar Aug 29 '13 at 07:01
  • @jyparask and All of you really helped me to learn something from all your answers and comments. Thank you again... – Programmer Aug 29 '13 at 07:55
0

i believe the problem is you are putting strings in the insert statement without "'" signs

try:

cmd = new SqlCommand("insert into purchaseOrder_master(sup_id,po_date,required_date,tot_amt,uid,potime) values("+supid.Text +",'"+podate.Value.Date+"','"+reqdate.Value.Date+"','"+pocost.Text +"','"+uid.Text +"','"+potime.Value.TimeOfDay  +"')", con);

as you can read here, when doing insert statement, every string needs to be surround with "'"

No Idea For Name
  • 11,411
  • 10
  • 42
  • 70
  • There's almost no proper way of inserting a `DateTime` into the database using concatenated strings. You need to convert it to US date format first, which is not always given. Also I suggest you don't encourage him, but suggest using parameterized queries, as this is the only sane way of dealing with database queries. – Thorsten Dittmar Aug 29 '13 at 06:43
0

Try debugging your code and check what exactly the query is formed.

Also i think the possible reason is

insert into purchaseOrder_master(sup_id,po_date,required_date,tot_amt,**uid**,potime

uid is a keyword. replace it with [uid] and check

Ronak Patel
  • 2,570
  • 17
  • 20
-1

You have a string.empty in yours variables. Suddenly you generate the following commande : insert into purchaseOrder_master(sup_id,po_date,required_date,tot_amt,uid,potime) values('value1','value2',,'value3'...

you can use string.format to format your command

tdelepine
  • 1,986
  • 1
  • 13
  • 19
  • 1
    i am not the downvoter. but i would recommend that you update your answer with some solution. it does highlight the issue. – Ehsan Aug 29 '13 at 06:59