I am getting a "Incorrect syntax near '1'." error

Question

My C# code is pasted below:

SqlConnection con = new SqlConnection("Data Source=pawan-pc;Initial Catalog=App91;Integrated Security=True");
try
{
    SqlCommand cmd = new SqlCommand("sp_gettinglogsfromdevice", con);
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.AddWithValue("@tablename","DeviceLogs_"+comboBox_Attendance_SelMonth.Text+"_"+combobox_Attendance_SelectYear.Text);
    cmd.Parameters.AddWithValue("@fromduration",datetimepicker_att_From.Value.Date);
    cmd.Parameters.AddWithValue("@Toduration",datetimepicker_att_To.Value.Date);

    DataRowView drow = (DataRowView)combobox_att_events.SelectedValue;
    string l = drow.Row.ItemArray[0].ToString();


    SqlCommand cmd1 = new SqlCommand();
    cmd1.Connection = con;
    con.Open();


    cmd1.CommandText = "select WorkCode from WorkCodes where WorkCodeName='"+l+"'";
    SqlDataReader reader = cmd1.ExecuteReader();

    dk = new DataTable();
    dk.Load(reader);



    cmd.Parameters.AddWithValue("@WorkCode", dk.Rows[0][0]);

    SqlDataReader reader1 = cmd.ExecuteReader();
    DataTable dp =new DataTable();
    dp.Load(reader1);

    datagridview_att_attendance.DataSource=dp;

The referenced stored procedure is pasted below:

create procedure sp_gettinglogsfromdevice
(
    @tablename varchar(75),
    @fromduration datetime,
    @Toduration datetime,
    @WorkCode nvarchar(100)
)
As
Begin
Exec('select UserId,LogDate,WorkCode from '
    +@tablename+' where LogDate between '
    +@fromduration+' and '+@Toduration+' and WorkCode = '+@WorkCode);
End
Go

Iam actually taking the name of the table dynamically as in my database a table is created automatically with the name DeviceLogs_(month)_(year) for every month of logs that are created.Please help me with this error.I have seen numerous post but i cannot seem to wrap my head along this idea

Which SQL command is resulting in this error? What is the exact SQL code being executed in that command? — David, Aug 03 '17 at 12:54
You need quotes around your dates if you are going to execute it like that. But your code, as is, is open to SQL Injection. You should use `sp_executesql` instead. — mjwills, Aug 03 '17 at 12:55
@David looks like the `exec` line the concatenation of that datetime and the string would break the sql. — Cleptus, Aug 03 '17 at 12:56
1. Your code is vulnerable to SQL Injection attacks (Your stored procedure as well). 2. Don't use `sp_` as a prefix for stored procedre names. this prefix is used by microsoft. use `stp_` if you must use a prefix. 3. Don't share all the code. We are only interested in the relevant part. — Zohar Peled, Aug 03 '17 at 12:56
@mjwills to add, `QUOTENAME(@tablename)` would probably be a good idea as well. — interesting-name-here, Aug 03 '17 at 12:56
Possible duplicate of [EXEC sp\_executesql with multiple parameters](https://stackoverflow.com/questions/28481189/exec-sp-executesql-with-multiple-parameters) — mjwills, Aug 03 '17 at 12:56
I have posted the stored procedure above.The Execution breaks at this sentence stating the error — Pawan Bisht, Aug 03 '17 at 12:57
@ZoharPeled first time I see someone complaining about to much code :) — Rand Random, Aug 03 '17 at 12:57
@mjwills but the OP does not use `sp_executesql`... I know, he should ;-) — Cleptus, Aug 03 '17 at 12:58
Don't Worry its a windows form application.Its not on the net.Can you guys please tell me what's causing this error.I will amend my code to use the appropriate ways lator — Pawan Bisht, Aug 03 '17 at 13:00
Don't fall into the trap of "this is internal so I can code it sloppy because nobody will do anything malicious". This is a pattern I have seen too many times. First of all you are still prone to sql injection, just not as likely. Secondly, this program could be converted to a website at some point. Third, if you do write an externally facing application somebody may use this code as an example and now the lazy coding done here is suddenly exposed externally. Just use parameters...they are so simple to use. — Sean Lange, Aug 03 '17 at 13:34

mjwills · Accepted Answer · 2017-08-03T13:43:00.123

2

Exec('select UserId,LogDate,WorkCode from '
+@tablename+' where LogDate between '
+@fromduration+' and '+@Toduration+' and WorkCode = '+@WorkCode);

should be changed to:

declare @statement nvarchar(4000)

set @statement = N'select UserId,LogDate,WorkCode from '
    +@tablename+' where LogDate between @fromduration and @Toduration and WorkCode = @WorkCode'

EXEC sp_executesql  @statement,N'@fromduration datetime, @Toduration datetime, @WorkCode nvarchar(100)', @fromduration, @Toduration, @WorkCode;

To ensure that the dates and work code are passed correctly.

edited Aug 03 '17 at 13:43

answered Aug 03 '17 at 13:05

mjwills

23,389
6
40
63

Yes, but nobody has noticed the first error is before even attempting to execute the stored procedure. – Crowcoder Aug 03 '17 at 13:11
Well, OK I guess I can't be sure. It looked to me like OP got 1 confused with l but now maybe not. Apologies. – Crowcoder Aug 03 '17 at 13:15
The 1 is likely because the date starts with a 1 @Crowcoder . The query he is trying to execute is something like `select UserId,LogDate,WorkCode from TableName where LogDate between 1/1/2000` etc. – mjwills Aug 03 '17 at 13:16
Yes, the problem is with `'`, but your answer with `sp_executesql` is much better and can be faster. +1 – Rokuto Aug 03 '17 at 13:18
Must declare the scalar variable "@Toduration". – Pawan Bisht Aug 03 '17 at 13:31
Dude That worked,Thank you so much.I know i got lot to learn,Iam just beginning.Will stay in touch thanks – Pawan Bisht Aug 03 '17 at 13:55

I am getting a "Incorrect syntax near '1'." error

1 Answers1