When I try to insert a record with the ExecuteNonQuery command, I get the following error information. Any clues why? Thanks.
SSqlException was unhandled by user code
...
Message="Incorrect syntax near [output of one of my field names]."
...
[Item detail:] In order to evaluate an indexed property, the property must be qualified and the arguments must be explicitly supplied by the user.
My code:
Private objCmdAs SqlCommandPrivate strConnAsNew SqlConnection(ConfigurationManager.AppSettings("conn"))
...
objCmd =New SqlCommand("INSERT INTO tblUsers (UserID,FName,LName,PrimLang1,Ctry,Phone)" & _
"VALUES('" & strUser &"','" & strFName.Text &"','" & strLName.Text &"', '" & strLang.Text &"', '" & strCtry.Text &"', '" & strPhone.Text &"'" _
, strConn)
strConn.Open()
objCmd.ExecuteNonQuery()
hi muybn,
there's not closing bracket for values() i mean
objCmd =New SqlCommand("INSERT INTO tblUsers (UserID,FName,LName,PrimLang1,Ctry,Phone)" & _
"VALUES('" & strUser &"','" & strFName.Text &"','" & strLName.Text &"', '" & strLang.Text &"', '" & strCtry.Text &"', '" & strPhone.Text &"')" _ 'can u see please i added a bracket )
, strConn)
regards,
satish.
|||Don't concatenate UI-supplied data to SQL statements that will be executed. This is an insecure practice as it opens up your server to SQL injection attacks. Use parameters instead.|||Thanks, but doesn't the closing parenthesis bracket go after the reference to the connection string, in this case on the last line, strConn)?
|||Thanks, TMorton. Is this merely a security precaution or would it cause the error I'm experiencing?
I plan to incorporate parameters into my project before I take it live. Can you point me to a definitive tutorial source for forming parameters, or better yet, mock up some of the variables that I've supplied above into parameters? To be honest, I've looked at quite a few sites and they've all confused me with how to define the parameters after the SQL statement, where you set the parameters equal to the variables.
|||What Terri is recommending is
1) considered a best practice
2) offers protection from sql injection
3) avoids issues with getting your quotes correct when concatenating the sql. (have you considered what happens if a lastname is "O'Rourke")
Dim objCmdAs SqlCommandDim strConnAs New SqlConnection(ConfigurationManager.AppSettings("conn"))'... objCmd.Parameters.Add(New SqlParameter("@.p1", strUser)) objCmd.Parameters.Add(New SqlParameter("@.p2", strFName.Text)) objCmd.Parameters.Add(New SqlParameter("@.p3", strLName.Text)) objCmd.Parameters.Add(New SqlParameter("@.p4", strLang.Text)) objCmd.Parameters.Add(New SqlParameter("@.p5", strCtry.Text)) objCmd.Parameters.Add(New SqlParameter("@.p6", strPhone.Text)) objCmd =New SqlCommand("INSERT INTO tblUsers (UserID,FName,LName,PrimLang1,Ctry,Phone)" & _" VALUES(@.p1,@.p2,@.p3,@.p4,@.p5,@.p6)", strConn) strConn.Open() objCmd.ExecuteNonQuery()|||still good option is write stored procedures wherever necessary, they are better in performance as they are precompiled. rest what mike has given as example is good one.
and for earlier post values clause has its own brackets so you need to close where i mentioned earlier.
thanks,
satish
|||Thanks, now I see. Hopefully it will work now.|||
satish_nagdev:
still good option is write stored procedures wherever necessary, they are better in performance as they are precompiled.
Performance differences between dynamic sql and stored procs is one of those things that is widely disputed. Personally i'm quite fond of dynamic sql but will still use a sproc if i see a benefit. But, rather than just debate the issue, let's test it. Here I offer the results of a very simple performance test.
Since the execution plan for dynamic sql is also cached (as is the execution plan for a sproc), the dynamic sql actually turns out to be quite performant.
Note that on iteration 1, the dynamic sql suffered a little because i ran it first. If i had run the sproc first, the result would look more like this:
Both set of results were taken after running my test code a few times to try to be more consistent with how a system in motion might perform.
Of course test results mean nothing unless you know how the test was run. I ran the test on my development system where sql 2000 was also installed on the same box.
This is the test code. Please adapt it to your own real word test to see if dynamic sql can compete with your own sprocs.
The test sproc:
CREATE PROCEDURE GetUserActivity (@.userid integer)AS-- tblTransactionLog has 1 million+ rows of data-- the userid column is indexedSELECT *FROM tblTransactionLogWHERE userid = @.userId;GO
The page code:
Protected Sub Page_Load(ByVal senderAs Object,ByVal eAs System.EventArgs)Handles Me.LoadDim swAs StopwatchDim drAs SqlDataReaderDim connAs SqlConnectionDim cmdDynamicAs New SqlCommandDim cmdSprocAs New SqlCommandDim dynParamAs SqlParameterDim sprocParamAs SqlParameterDim tsDynamicAs TimeSpanDim tsSprocAs TimeSpan conn =New SqlConnection("Initial Catalog=webcommon;Integrated Security=True") dynParam =New SqlParameter("@.userid", SqlDbType.Int, 4) dynParam.Value = 4347 cmdDynamic.Parameters.Add(dynParam) sprocParam =New SqlParameter("@.userid", SqlDbType.Int, 4) sprocParam.Value = 4347 cmdSproc.Parameters.Add(sprocParam) conn.Open() Using conn'prepare for sproc cmdDynamic.CommandText ="GetUserActivity" cmdDynamic.CommandType = CommandType.StoredProcedure cmdDynamic.Connection = conn'prepare for dynsql cmdSproc.CommandText ="select * from tblTransactionLog where userid = @.userid;" cmdSproc.CommandType = CommandType.Text cmdSproc.Connection = conn Response.Write("<table border=""1""><tr><th>Iteration</th><th>DynSql</th><th>Sproc</th></tr>")For indexAs Integer = 1To 10'going first incurs a small performance penalty on the very first iteration sw = Stopwatch.StartNew dr = cmdSproc.ExecuteReader() tsSproc = sw.Elapsed dr.Close() sw = Stopwatch.StartNew dr = cmdDynamic.ExecuteReader() tsDynamic = sw.Elapsed dr.Close() Response.Write(String.Format("<tr><td>{0}</td><td>{1}</td><td>{2}</td></tr>", index, tsDynamic.TotalSeconds.ToString("n6"), tsSproc.TotalSeconds.ToString("n6")))Next Response.Write("</table>")End UsingEnd Sub|||
still good option is write stored procedures wherever necessary, they are better in performance as they are precompiled.
This is misguided advice. There are good reasons to use stored procedures, but performance is not one of them. There are places where *not* using stored procedures is a better option. This topic (stored procedures vs. inline SQL) is the subject of a lot of heated, well-reasoned discussion in the blogosphere.
|||How awesome that you would take the time to detail all this for me! Thanks. I will test it out soon. Right now, I have to go one step at a time understanding the underlying principles and solving some other errors that are showing up.|||Mike, I'm getting this error while trying to use your suggestion on parameters: "Object reference not set to an instance of an object." This comes with each line that begins with "objCmd.Parameters." These are merely strings, as far as I can see, so I don't know why it would be asking for object instances.|||my bad. when adapting your code i got it out of sequence...you need to create the command object before you add the parameters.
Dim objCmdAs SqlCommandDim strConnAs New SqlConnection(ConfigurationManager.AppSettings("conn")) objCmd =New SqlCommand("INSERT INTO tblUsers (UserID,FName,LName,PrimLang1,Ctry,Phone)" & _" VALUES(@.p1,@.p2,@.p3,@.p4,@.p5,@.p6)", strConn) objCmd.Parameters.Add(New SqlParameter("@.p1", strUser)) objCmd.Parameters.Add(New SqlParameter("@.p2", strFName.Text)) objCmd.Parameters.Add(New SqlParameter("@.p3", strLName.Text)) objCmd.Parameters.Add(New SqlParameter("@.p4", strLang.Text)) objCmd.Parameters.Add(New SqlParameter("@.p5", strCtry.Text)) objCmd.Parameters.Add(New SqlParameter("@.p6", strPhone.Text)) strConn.Open() objCmd.ExecuteNonQuery()|||Terri, Mike,
i wont argue on that. I agree with you guys upto a limit, but mike in my last project there were heaps of inline queries we found while re-writing the application, so using procedures added positively to scalability. so depends on from situation to situation.
mike you've done testing thats good, if you get time could you do it for simultaneous instances say 10 at a go?
thanks,
satish.
|||Inline queries should by managed in a DAL Component. My DAL is a seperate project which keeps things nice and tidy.
I actually have very few hand typed dynamic sql statements. My dynamic sql is about 99% generated on the fly.
Anyways, I put my test page through an ACT test script and here are the results. I used only 8 simultaneous connections to avoid a resultset with http errors .
Test 1 - sproc performance:
commented out the dynamic reader code inside the loop
test duration: 1 minute
Avg Requests per second: 587
Total requests completed: 35,232
Test 2 - dynamic sql performance:
commented out the sproc reader code inside the loop
test duration: 1 minute
Avg Requests per second: 601
Total requests completed: 36,082
My test setup is a little flawed since my test script was running on the same system that was under test. But, since we're just doing a head to head comparison and since both tests were subject to the same testing flaw, i'd have to conclude that dynamic sql (in this specific test case) outperformed a stored proc.
No comments:
Post a Comment