I am trying to limit network traffic by only sending fields that have actually changed to an update stored procedure. I am obviously not doing this correctly as I am rather new to this. This is a small version of my sproc that I am using to test the workings. I am getting 'Error converting data type nvarchar to int'. If you can help me get this easy query working, I would very much like your help to use the EXEC (sql) where sql is an sp_executesql statement because some of my update fields are much larger than 4000 nvarchar. The following statement was working when I was using the second version. If I am trying to update an integer foreign key field, shouldn't I try to use the parameters to send an integer value for that field, rather than an nvarchar? As you can see, I really need to get some not so basic info on this. I have done hours of research on the net and can't find some of these simple background principals. I also question whether I am saving enough network traffic to validate the cost in processing for building this query and executing it. Any help you can provide will be greatly appreciated.
Non working
CREATE PROCEDURE tspJobs_Updatetest
(
@.JobID int=0,
@.AddressID int=0,
@.AddressChg int=0,
@.CustomerID int=0,
@.CustomerChg int=0,
@.ContactID int=0,
@.ContactChg int=0,
@.PlanID int=0,
@.PlanChg int=0,
)
AS
DECLARE @.updateClause nvarchar(1000);
DECLARE @.whereClause nvarchar(500);
DECLARE @.sqlStatement nvarchar(4000);
DECLARE @.paramDefinition nvarchar(1000);
DECLARE @.valuesDefinition nvarchar(1500);
--Set Where Clause and parameter statement
IF @.JobID > 0
BEGIN
SET @.whereClause = ' WHERE job_id = @.update_job_id '
SET @.paramDefinition = N'@.update_job_id int '
SET @.valuesDefinition = N'@.update_job_id=@.JobID '
--Build Update Statement
SET @.updateClause = null
IF @.AddressChg > 0
BEGIN
SET @.paramDefinition = @.paramDefinition + N', @.update_address_id int'
SET @.valuesDefinition = @.valuesDefinition + N', @.update_address_id = @.AddressID'
IF @.updateClause IS NULL
SET @.updateClause = N'address_id = @.update_address_id'
ELSE
SET @.updateClause = @.updateClause + N', address_id = @.update_address_id'
END
IF @.CustomerChg > 0
BEGIN
SET @.paramDefinition = @.paramDefinition + N', @.update_customer_id int'
SET @.valuesDefinition = @.valuesDefinition + N', @.update_customer_id = @.CustomerID'
IF @.updateClause IS NULL
SET @.updateClause = N'customer_id = @.update_customer_id'
ELSE
SET @.updateClause = @.updateClause + N', customer_id = @.update_customer_id'
END
IF @.ContactChg > 0
BEGIN
SET @.paramDefinition = @.paramDefinition + N', @.update_contact_id int'
SET @.valuesDefinition = @.valuesDefinition + N', @.update_contact_id = @.ContactID'
IF @.updateClause IS NULL
BEGIN
IF @.ContactID = 0
SET @.updateClause = N'contact_id = NULL'
ELSE
SET @.updateClause = N'contact_id = @.update_contact_id'
END
ELSE
BEGIN
IF @.ContactID = 0
SET @.updateClause = @.updateClause + N', contact_id = NULL'
ELSE
SET @.updateClause = @.updateClause + N', contact_id = @.update_contact_id'
END
END
IF @.PlanChg > 0
BEGIN
SET @.paramDefinition = @.paramDefinition + N', @.update_plan_id int'
SET @.valuesDefinition = @.valuesDefinition + N', @.update_plan_id = @.PlanID'
IF @.updateClause IS NULL
BEGIN
IF @.PlanID = 0
SET @.updateClause = N'plan_id = NULL'
ELSE
SET @.updateClause = N'plan_id = @.update_plan_id'
END
ELSE
BEGIN
IF @.PlanID = 0
SET @.updateClause = @.updateClause + N', plan_id = NULL'
ELSE
SET @.updateClause = @.updateClause + N', plan_id = @.update_plan_id'
END
END
-- Complete SQL statement
IF NOT @.updateClause IS NULL
BEGIN
SET @.sqlStatement = 'UPDATE [dbo].[Jobs] SET ' + @.updateClause + @.whereClause
EXEC sp_executesql @.sqlStatement,@.paramDefinition, @.valuesDefinition
END
END
GO
Working, but using nvarchar for integer foreign key field updates
CREATE PROCEDURE tspJobs_Update
(
@.JobID int=0,
@.AddressID int=0,
@.AddressChg int=0,
@.CustomerID int=0,
@.CustomerChg int=0,
@.ContactID int=0,
@.ContactChg int=0,
@.PlanID int=0,
@.PlanChg int=0
)
AS
DECLARE @.updateClause varchar(3000);
DECLARE @.whereClause varchar(1000);
DECLARE @.sqlStatement nvarchar(4000);
DECLARE @.paramDefinition nvarchar(500)
--Set Where Clause and parameter statement
IF @.JobID > 0
BEGIN
SET @.whereClause = ' WHERE job_id = @.update_job_id'
SET @.paramDefinition = N'@.update_job_id int'
--Build Update Statement
SET @.updateClause = null
IF @.AddressChg > 0
BEGIN
DECLARE @.update_address_id varchar(15)
SET @.update_address_id = @.AddressID
IF @.updateClause IS NULL
SET @.updateClause = 'address_id = ' + @.update_address_id
ELSE
SET @.updateClause = @.updateClause + ', address_id = ' + @.update_address_id
END
IF @.CustomerChg > 0
BEGIN
DECLARE @.update_customer_id varchar(15)
SET @.update_customer_id = @.CustomerID
IF @.updateClause IS NULL
SET @.updateClause = 'customer_id = ' + @.update_customer_id
ELSE
SET @.updateClause = @.updateClause + ', customer_id = ' + @.update_customer_id
END
IF @.ContactChg > 0
BEGIN
DECLARE @.update_contact_id varchar(15)
SET @.update_contact_id = @.ContactID
IF @.updateClause IS NULL
BEGIN
IF @.update_contact_id IS NULL
SET @.updateClause = 'contact_id = NULL'
ELSE
SET @.updateClause = 'contact_id = ' + @.update_contact_id
END
ELSE
BEGIN
IF @.update_contact_id IS NULL
SET @.updateClause = @.updateClause + ', contact_id = NULL'
ELSE
SET @.updateClause = @.updateClause + ', contact_id = ' + @.update_contact_id
END
END
IF @.PlanChg > 0
BEGIN
DECLARE @.update_plan_id varchar(15)
SET @.update_plan_id = @.PlanID
IF @.updateClause IS NULL
BEGIN
IF @.update_plan_id IS NULL
SET @.updateClause = 'plan_id = NULL'
ELSE
SET @.updateClause = 'plan_id = ' + @.update_plan_id
END
ELSE
BEGIN
IF @.update_plan_id IS NULL
SET @.updateClause = @.updateClause + ', plan_id = NULL'
ELSE
SET @.updateClause = @.updateClause + ', plan_id = ' + @.update_plan_id
END
END-- Complete SQL statement
IF @.updateClause <> null
BEGIN
SET @.sqlStatement = 'UPDATE [dbo].[Jobs] SET ' + @.updateClause + @.whereClause
EXEC sp_executesql @.sqlStatement,@.paramDefinition, @.update_job_id=@.JobID
END
END
GO
I know that this is a huge post, but I have pretty much exhausted my resources.
Thanks,
I didn't analyze the logic of the SP. But you are making the SP more complicated than necessary. Here are the reasons:
1. You are using dynamic SQL which can hurt performance
2. Your dynamic SQL code doesn't protect against SQL injection. So you can compromise your database/server if you haave more SPs like this that take string parameters
3. Since the UPDATE statement is executed dynamically you need to grant UPDATE permissions to all callers of the SP. This increases the attack surface of the database and grants more permissions to users than necessary. And it kind of defeats the purpose of having a SP. You might as well form the UPDATE statement on the client-side and execute it directly. You will get the same or probably better performance
4. Debugging code using dynamic SQL can be hard and difficult to maintain also as you have encountered
5. Lastly, you don't really gain that much by updating only the necessary columns. SQL Server has to do lot of work to locate the row to update and after you have located the row for update it doesn't matter in most cases if you update one or all of the columns. The only case where it matters is if you have lot of variable length columns that can overflow or increase the row size which will require more work. Otherwise you might as well update the entire row. You don't save anything in terms of performance or efficiency.
If you are not passing the old values (i.e., columns that were not changed) then you can use alternate approach below which doesn't require dynamic SQL:
UPDATE dbo.Jobs
SET address_id = CASE WHEN AddressChg > 0 THEN @.AddressID ELSE address_id END
, ...
WHERE job_id = @.update_job_id
The main point to note is that keeping transactions/calls light-weight gives the best performance and improves overall utilization of the server.
|||Thank you very much for your reply. I agree that the dynamic SQL use opened us up to some danger, although this is a windows form application. I would love to get rid of it, however, I don't understand how the statement that you have shown works. The case I understand of course, it's the 'ELSE address_id END' that I don't understand. I don't have this value (address_id) from my application, only the 'THEN AddressID'. Is this address_id coming from the table update mechanism? If so, this is definitely the answer to most of my problems.
|||The address_id in the ELSE clause comes from the row you are updating. It refers to the column in the table. So it is a way to use the current value of address_id if there is no change. SQL is a set-based language so statements like UPDATE has logically a before and after image for each row & the operations happen in one shot. This allows you to swap values in two columns using an UPDATE statement without any intermediate storage:
update t
set a = b, b = a
The database engine handles all the dirty work for you.
|||It works beautifully. Thank you so much for this elegant, simple solution.
No comments:
Post a Comment