 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
.NET Forum / ASP.NET / General / October 2007A Microsot Bug in Dynamic Comlumns?
Hello, -=PREHISTORY=- Occasionally, I wanted to rewrite my UPDATE code to prevent SQL injection. I generated the SQL query UPDATE tt SET @p1_name = @p1_val , @p2_name = @p2_val , ... along with the pairs of paramz: new SqlParameter(formalName, actualName); new SqlParameter(formalValue, actualValue); for every column. Now, user can send name-value pairs the corresponding columns will be updated safely. However, this fails if a value is longer than 4 chars: "System.Data.SqlClient.SqlException: String or binary data would be truncated. The statement has been terminated." Note, the code still works if the column names are not parametrized. As this is weired, so I have decided to trial in pure SQL. -=SQL=- Here is the code to execute: DECLARE @val varchar(100) SELECT @val ='vvvv' -- making val longer breaks the code exec sp_executesql N'update tt SET @name=@val' , N'@name varchar(4), @val varchar(100)' , 'col1', @val Making the value one char longer will end up in the suspiciously famous "Msg 8152, Level 16, State 14, Line 1 String or binary data would be truncated." THE INTERESTING thing is that the maximal allowed @val length is determined by the @name parameter definition! Try yourslf by changing the declaration varchar(4) -> varchar(5) and this will allow entering 5-symbol values. Another bug observed is possibly related to the this one. The table data is not affected even when no error is encountered. The value is taken only when column name is not parametrized, that is, 'UPDATE tt SET col1 = @val' is submitted. Am I missing something? > Hello, > [quoted text clipped - 45 lines] > column name is not parametrized, that is, 'UPDATE tt SET col1 = @val' is > submitted. that's not a bug, it works as designed. The right approach might be along the following lines: UPDATE tt SET column1 = COALESCE(@p1_val, column1), column2 = COALESCE(@p2_val, column2), (snip) Thank you. Could you please clarify on what is 'column1' in your example. I need it to be an input parameter variable, so it must be prefixed by '@' in sql. > Thank you. Could you please clarify on what is 'column1' in your example. I > need it to be an input parameter variable, so it must be prefixed by '@' in > sql. there is no need to provide column names as parameters whatsoever. suppose you have the following table and update procedure: create table data.tt(tt_PK int not null, col1 int null, col2 int null, col3 int null) go create procedure writers.update_tt @tt_PK int, @col1 int = null, @col2 int = null, @col3 int =null as update data.tt set col1 = coalesce(@col1, col1), col2 = coalesce(@col1, col2), col3 = coalesce(@col1, col3) where tt_pk = @tt_pk go to update just one column col2, run the following script exec writers.update_tt @tt_pk = 1, @col2 = 12 Now, having the update procedure, the need to parametrize names is eliminated! > DECLARE @val varchar(100) > SELECT @val ='vvvv' -- making val longer breaks the code [quoted text clipped - 5 lines] > > Am I missing something? Yes. You seem to believe that the above code would update the col1 column in the tt table, setting it's value to 'vvvv'. That is not the case. Actually, the above code puts the value 'vvvv' in the variable @name. Obviously, that will cause an error if you try to put a value longer than 4 characters into a varchar(4) variable. What you probably want is this code: DECLARE @sql nvarchar(4000), @val varchar(100), @name varchar(4) SET @val ='vvvv' SET @name ='col1' SET @sql=N'update tt SET '+QUOTENAME(@name)+'=@val' exec sp_executesql @sql, N'@val varchar(100)', @val  SignatureRazvan Socol SQL Server MVP > Hello, > [quoted text clipped - 49 lines] > > Am I missing something? Not a bug. The meaning of UPDATE tt SET @name=@val is to assign the value @val to the variable @name. It doesn't update any column (altough it will fire update triggers on the target table). Therefore you should expect a truncation error if @name is declared to be smaller than the length of @val. You cannot parameterise column names in this way. You could do: exec sp_executesql N'update tt SET '+QUOTENAME(@name)+N'='@val but that's not a good idea. Column names should generally be in static code. SignatureDavid Portas > You cannot parameterise column names in this way. You could do: > > exec sp_executesql N'update tt SET '+QUOTENAME(@name)+N'='@val My apologies. Of course you cannot do that. You need to assign it to a variable first because complex expressions aren't permitted as parameters. SignatureDavid Portas > but that's not a good idea. Column names should generally be in static > code. Any arguments? How do I update the only changed record fields otherwise? > > but that's not a good idea. Column names should generally be in static > > code. > > Any arguments? How do I update the only changed record fields otherwise? UPDATE tt SET column1 = COALESCE(@p1_val, column1), column2 = COALESCE(@p2_val, column2), Excuse me for ignoring you. I just wanted to ask the one who clames that column names must always be a static code (hardcoded?). The thigs passed as parameters are defenetely non-static, so I must know what is suggested way to update only specific cell(s). > Excuse me for ignoring you. I just wanted to ask the one who clames that > column names must always be a static code (hardcoded?). The thigs passed as > parameters are defenetely non-static, so I must know what is suggested way > to update only specific cell(s). Create separate UPDATEs for each case. You might want to combine that with the approach that Alex suggested if you have a large number of possible permutations. Don't forget to code for the NULL cases if you have nullable columns. >From another perspective, a large number of columns being updated individually may indicate an underlying design problem. Tables are not 2D arrays (no "cells" please!) and you won't get the most out of SQL if you try to use them that way. -- David Portas > Excuse me for ignoring you. I just wanted to ask the one who clames that > column names must always be a static code (hardcoded?). The thigs passed > as parameters are defenetely non-static, so I must know what is > suggested way to update only specific cell(s). In a well-designed database, each column represents a distinct attribute, why parameterisation does not make sense. In less well-designed database you can try: UPDATE tbl SET col1 = CASE @col WHEN 'col1' THEN @value ELSE col1 END, col1 = CASE @col WHEN 'col2' THEN @value ELSE col2 END, ... SignatureErland Sommarskog, SQL Server MVP, esquel@sommarskog.se Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx [I] > In a well-designed database, each column represents a distinct attribute, This means that the attributes can be updated independently of the others. [II] > why parameterisation does not make sense. This contradicts to [I] > In less well-designed database you can try: > > UPDATE tbl > SET col1 = CASE @col WHEN 'col1' THEN @value ELSE col1 END, > col1 = CASE @col WHEN 'col2' THEN @value ELSE col2 END, > ... This seems to be a NULL-values problem solving alternative to the COALESE. > [I] >> In a well-designed database, each column represents a distinct attribute, > > This means that the attributes can be updated independently of the others. Yes. And? The point is you know which column you will update, and you will write your update statement accordingly. > [II] >> why parameterisation does not make sense. > > This contradicts to [I] It appears to me that most time when people want to parameterise there UPDATE statements is when they have columns like sales_jan, sales_feb, etc. SignatureErland Sommarskog, SQL Server MVP, esquel@sommarskog.se Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx >> [I] >>> In a well-designed database, each column represents a distinct [quoted text clipped - 5 lines] > Yes. And? The point is you know which column you will update, and you > will write your update statement accordingly. The Independence of cells from each other within a record does mean that a "well-designed" table is a 2d array of "cells". The point that it is user who knows which of them needs an update and signals the DB accordingly. There is no need to transfer and update the whole row (or column), when only one cell is changed. >> [II] >>> why parameterisation does not make sense. [quoted text clipped - 4 lines] > there UPDATE statements is when they have columns like sales_jan, > sales_feb, etc. Do you mean that it is the uniformity. which is the criteria, or it is a number of columns? > The Independence of cells from each other within a record does mean that > a "well-designed" table is a 2d array of "cells". The point that it is > user who knows which of them needs an update and signals the DB > accordingly. There is no need to transfer and update the whole row (or > column), when only one cell is changed. It's not really a matrix, as there are very different behaviour for rows and columns. Particularly, a colunm has a distinct data type, an update statement with parameters for one column is not going to be like the update statement for a different column. Furthermore, the column names are given. There is a finite set of columns in the table you can update. Whereas the key values that identify the rows are typically drawn from a virtually unlimited set of values. SignatureErland Sommarskog, SQL Server MVP, esquel@sommarskog.se Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx [...] > it is user who knows which of them needs an update and signals the DB > accordingly. There is no need to transfer and update the whole row > (or column), when only one cell is changed. I agree with you here. The way that the user would signal the DB than only some columns need to be updated would be to write an UPDATE statement accordingly, for example: UPDATE tbl SET col1='x', col3=7 WHERE id=100 or (to make better use of cached plans): EXEC sp_executesql N'UPDATE tbl SET col1=@p1, col3=@p2 WHERE id=@p3', N'@p1 varchar(10), @p2 int, @p3 int', 'x', 7, 100 Why would you want to use a stored procedure for this ? SignatureRazvan Socol SQL Server MVP > Why would you want to use a stored procedure for this ? It is advised to enforce security (mod access to tables is forbidden) in addition to the speed. > > Why would you want to use a stored procedure for this ? > > It is advised to enforce security (mod access to tables is forbidden) > in addition to the speed. In my opinion, using a stored procedure which executes dynamic SQL will not be any better than executing the sp_executesql procedure directly, in both security and speed. You still need to allow the user direct access to the tables (unless you sign your procedure with a certificate) and there will be parametrised execution plans cached for sp_executesql anyway. I'd appreciate if Erland and the other MVP-s could post their opinion, too (supporting or opposing my opinion, with better arguments). SignatureRazvan Socol SQL Server MVP > In my opinion, using a stored procedure which executes dynamic SQL will > not be any better than executing the sp_executesql procedure directly, [quoted text clipped - 5 lines] > I'd appreciate if Erland and the other MVP-s could post their opinion, > too (supporting or opposing my opinion, with better arguments). There is really not much add. Generally, I tend to think that for a stored procedure with dynamic SQL to be worthwhile there need to be some level of complexity that is worth packaging. Code like 'UPDATE tbl SET ' + quotename(@colname) + ' = @value ' + WHERE somekey = @key' does not really meet that level. And it looks clunky, because you will need different procedure for different data types. Or use sql_variant. In practice, a GUI usually goes with a procedure that updates most columns in the table (those exposed in the GUI). If there are other function where only a single column needs to be updated, you would typically have a separate procedure for that. SignatureErland Sommarskog, SQL Server MVP, esquel@sommarskog.se Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx > In practice, a GUI usually goes with a procedure that updates most > columns in the table (those exposed in the GUI). If there are other > function where only a single column needs to be updated, you would > typically have a separate procedure for that. And if the GUI is smart enough to know which columns have been changed by the user (and wants to update only those columns), then it should build a dynamic UPDATE statement on the client side and send it to sp_executesql, along with parameters for the values of those columns. SignatureRazvan Socol SQL Server MVP >> In practice, a GUI usually goes with a procedure that updates most >> columns in the table (those exposed in the GUI). If there are other [quoted text clipped - 5 lines] > build a dynamic UPDATE statement on the client side and send it to > sp_executesql, along with parameters for the values of those columns. And this really brings us to the point, if the GUI has all the value of all those columns, then how much do you really gain by just updating the columns that have actually changed? You save some network bandwidth, but that's about it. OK, if there is a trigger on the table with IF UPDATED in it, there may be more to win. Still it's not obvious that this warrants the increased gain in complexity. Having a stored procedure which handles all the columns owned by this particular GUI appears to be a simple way to go. SignatureErland Sommarskog, SQL Server MVP, esquel@sommarskog.se Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx you can not parameterize a column name. it will be treated as a standard sql variable. its the variable (which is only 4 chars), getting the truncation error in the set, not the column, as no column is specified -- bruce (sqlwork.com) > Hello, > [quoted text clipped - 47 lines] > > Am I missing something? Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
Reply to this Message
|
 Sign In
 Join
 My Latest Posts My Monitored Threads My Blog My Photo Gallery My Profile My Homepage Start New Thread Enable EMail Alerts Rate this Thread
|
©2008 Advenet LLC Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.