November 8, 2018 at 7:30 am
I was updating a stored procedure to change the reporting time to clients timezone. The stored procedure is comprised of a dynamic SQL, which contains the time @timeoffset parameter of smallint data type.
DECLARE @sql VARCHAR(MAX) = To change the time to the clients' time zone, I need to subtract the @timeoffset. Making it negative doesn't change the output. Trying to add the (-) before the conversion, would raises an error as subtraction operator is invalid for varchar. Writing it without the conversion raises an error
N'SELECT DISTINCT cl.ClientId,
CASE WHEN DATEADD(HOUR,'+CONVERT(CHAR(2),@timeoffset)+', x.changedate) >= '''+ CONVERT(varchar, @start_date) +'''
AND DATEADD(HOUR,'+CONVERT(CHAR(2),@timeoffset)+', x.changedate) < '''+ CONVERT(varchar, @end_date_plus1) +'''
THEN DATEADD(HOUR,'+CONVERT(CHAR(2),@timeoffset)+',x.changedate)'
*'Conversion failed when converting the nvarchar value to data type smallint.*[/code]
Can anyone assist me with this please? Thanks.
November 8, 2018 at 7:47 am
Just put the minus sign in the dynamic SQL, like this:... DATEADD(HOUR, -'+CONVERT(CHAR(2),@timeoffset)+ ...
I've got to ask, though - why do you need dynamic SQL for this at all? Can you not just pass @timeoffset as a parameter directly to the DATEADD function?
John
November 8, 2018 at 8:14 am
John, thanks.
About your Q: the dynamic SQL is needed as the changedateutc is retrived from clients' databases. The clients have specific domains.
November 8, 2018 at 8:43 am
keneangbu - Thursday, November 8, 2018 8:14 AMJohn, thanks.
About your Q: the dynamic SQL is needed as the changedateutc is retrived from clients' databases. The clients have specific domains.
That doesn't really seem to change the reason, if I'm honest. Otherwise, why not parametrise your Dynamic SQL? So instead of something like:
DECLARE @sql nvarchar(MAX);
SET @sql = 'SELECT ' + CONVERT(varchar(2),DATEPART(HOUR,@SomeVariable)) + ';';
EXEC (@SQL);
You would do something like:DECLARE @sql nvarchar(MAX);
SET @sql = 'SELECT DATEPART(HOUR,@dSomeVariable);';
EXEC sp_executesql @sql, N'@dSomeVariable date', @dSomeVariable = @SomeVariable;
But, like John said, there's nothing actually dynamic in those statements. Dynamic SQL is normally needed because the object(s) referenced change, not the value of a variable/parameter.
Thom~
Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk
November 8, 2018 at 8:54 am
keneangbu - Thursday, November 8, 2018 8:14 AMJohn, thanks.
About your Q: the dynamic SQL is needed as the changedateutc is retrived from clients' databases. The clients have specific domains.
Out of curiosity, how is the timeoffset determined.
November 8, 2018 at 9:00 am
Also, be careful with syntax like CONVERT(varchar, @start_date). I doubt it'll be a problem with this specific scenario, but you should really be declaring your lengths for your datatypes. Otherwise one day you might have a nasty surprise when your data gets truncated.
Thom~
Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk
November 8, 2018 at 9:08 am
Thom A - Thursday, November 8, 2018 8:43 AMkeneangbu - Thursday, November 8, 2018 8:14 AMJohn, thanks.
About your Q: the dynamic SQL is needed as the changedateutc is retrived from clients' databases. The clients have specific domains.That doesn't really seem to change the reason, if I'm honest. Otherwise, why not parametrise your Dynamic SQL? So instead of something like:
DECLARE @sql nvarchar(MAX);
SET @sql = 'SELECT ' + CONVERT(varchar(2),DATEPART(HOUR,@SomeVariable)) + ';';
EXEC (@SQL);
You would do something like:DECLARE @sql nvarchar(MAX);
SET @sql = 'SELECT DATEPART(HOUR,@dSomeVariable);';
EXEC sp_executesql @sql, N'@dSomeVariable date', @dSomeVariable = @SomeVariable;But, like John said, there's nothing actually dynamic in those statements. Dynamic SQL is normally needed because the object(s) referenced change, not the value of a variable/parameter.
Thom, I've posted just part of the SELECT statement with an intention of keeping it simple; otherwise, it is referring to the clients' domain. It retrieves the data from the client database accordingly based on the environment: FROM linkedserver.'+@cp_db_name+'.dbo.table AS cl
and filter it by the client domain as WHERE o.clientdomain = '''+@client_domain +'''
November 8, 2018 at 9:09 am
Lynn Pettis - Thursday, November 8, 2018 8:54 AMkeneangbu - Thursday, November 8, 2018 8:14 AMJohn, thanks.
About your Q: the dynamic SQL is needed as the changedateutc is retrived from clients' databases. The clients have specific domains.Out of curiosity, how is the timeoffset determined.
To determine the timeoffset, there is a timezone table and the calculation is done on top while declaration of @timeoffset.
November 8, 2018 at 9:25 am
keneangbu - Thursday, November 8, 2018 9:08 AMThom, I've posted just part of the SELECT statement with an intention of keeping it simple; otherwise, it is referring to the clients' domain. It retrieves the data from the client database accordingly based on the environment:FROM linkedserver.'+@cp_db_name+'.dbo.table AS cl
and filter it by the client domain asWHERE o.clientdomain = '''+@client_domain +'''
Both of these are going to be open to injection, so I would avoid both of these.
For the first part, you should be using QUOTENAME, especially if the value of the database name is coming from user input: FROM linkedserver.'+QUOTENAME(@cp_db_name)+'.dbo.table AS cl
For the latter, why not use a paramterisation there? For example: WHERE o.clientdomain = @ClientDomain
And then declare @ClientDomain in your call to sp_executesql and pass the value of @client_domain to it (like I showed above).
You need to get out of the habit of concatenating raw strings when creating dynamic SQL. Doing so leaves you wide open to having your code injected, and could well cause you a lot of problems down the line.
Thom~
Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk
November 8, 2018 at 9:27 am
Okay, just trying to help you out with the dynamic SQL side of things as I have had to write a lot of dynamic SQL where I work because of the nature of the database and the requirements of the queries.
First, I don't use EXEC (@SQL) to run dynamic SQL queries. I use EXEC sys.executesql @SQLCmd [, @SQLParms, @ParmInDSQL1 = @ParmPassedIn1 [, @ParmInDSQL2 = @ParmPassedIn2 ...]].
Second, although I didn't at first, I validate values for table names, column names, server names against system tables and build those parts as concatenation of strings where variables aren't allowed in normal SQL statement. I also use QUOTENAME() around the pieces of table names, column names, and server names being concatenated to help prevent SQL injection (or in my case having special characters in those names because of people who shouldn't be messing with the database doing dumb things with names). Where I can use variables, I pass those into the dynamic SQL using the appropriate data types instead of converting them to string values.
Third, I have even gotten to the point that I use "replacement values" in the dynamic SQL so that all I have to do is use REPLACE to insert the converted values where I would normally use concatenation. This allows me to see the code without all the +'s and converts intermixed.
From what I have seen of your snippets, I believe the dynamic SQL can be rewritten to be easier to read and understand.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply