April 12, 2013 at 12:56 pm
I have a stored procedure that builds a dynamic sql and executes it. It gives no error messages, but it doesn't return results, just a message that the command completed successfully. However, when I run the sp code manually in SSMS I get the results I'm looking for. Why would this be? The sp takes one nvarchar(max) parameter, @CLM. ANSI_NULLS, QUOTED_IDENTIFIER, and NOCOUNT are ON. In the sp there is one temp table #tmpAffPeople. Its columns are EmpLogin nvarchar(8), Matter nvarchar(15), NameandTitle nvarchar(max), TotalHours decimal(6,2), ClientMatter varchar(max). The text of the sp is as follows:
DECLARE @SQLString AS nvarchar(max)
SET @SQLString = ';WITH Emps (EmpLogin, AttorneyAndTitle)
AS
(SELECT EmpLogin,
CASE WHEN EmpTermDate IS NULL OR EmpTermDate = ' + '''' + '''' + ' THEN LastName + ' + '''' + ', ' + '''' + ' + FirstName + ' + ''''+ ' [' + '''' + ' + Title + ' + '''' + '] ' + '''' +
'ELSE LastName + ' + '''' + ', ' + '''' + ' + FirstName + ' + '''' + ' [' + '''' + ' + Title +' + '''' +'] (inactive) ' + '''' + ' END as FullName
FROM [LitigationHold].[dbo].[Employees]
where JobCode IN (' + '''' + '100' + '''' + ',' + '''' + '300' + '''' + ',' + '''' + '400' + '''' + ',' + '''' + '500' + '''' + ',' + '''' + '700' + '''' + ',' + '''' + '800' + '''' + ',' + '''' + '1801' + '''' + '))
INSERT INTO #tmpAffPeople
SELECT t.[EmpLogin],t.Matter, e.AttorneyAndTitle, SUM([Workhours]) AS TotalHours,
c.ClientName + ' + '''' + '/' + '''' + ' + em.MatterDesc
FROM TimeCard t
INNER JOIN Emps e
ON e.EmpLogin = t.EmpLogin
INNER JOIN EliteMatter em
ON t.Matter = em.MatterName
INNER JOIN EliteClient c
ON c.ClientNum = em.ClientNum
WHERE t.Matter IN (' + @CLM + ')
GROUP BY t.EmpLogin, c.ClientName, em.MatterDesc, t.Matter, e.AttorneyAndTitle
ORDER BY TotalHours'
After this runs, in the sp I execute the @SQLString by calling execute with @SQLString right after it between parentheses. I then drop the table. I've done this kind of thing before and never had a problem. What am I missing? I'm using SQL Server 2008R2.
April 12, 2013 at 1:03 pm
Please post the stored procedure and how you are invoking it as well.
April 12, 2013 at 1:07 pm
I tried to post the entire stored proc, but something in it was causing a problem with the forum. I suspect it's either the table creation or dropping or the invoking of the execute itself. If someone knows how I can get around this, I'll happily post the whole thing. As for how I'm invoking it, if I do
exec getAffectedEmployees "'099999-00005','099999-00004'"
from withing SSMS, I get no results. However, if I copy the actual code from the sp to a query window in SSMS, I get a result set.
April 12, 2013 at 1:10 pm
It's late on a Friday. Someone on another forum just pointed out to me that I wasn't doing a SELECT on my temp table after I built it. I'm going to go slam my head into a wall now. Happy weekend everyone!
April 12, 2013 at 1:18 pm
Melanie Peterson (4/12/2013)
It's late on a Friday. Someone on another forum just pointed out to me that I wasn't doing a SELECT on my temp table after I built it. I'm going to go slam my head into a wall now. Happy weekend everyone!
BE VERY CAREFUL. From what you posted and described you are receiving a parameter and putting that directly into a dynamic string and executing it. That is textbook sql injection vulnerability. If you want to pass in a delimited string like that it is FAR better and safer to parse that string instead of just sticking it inside a string and executing. I am guessing the reason you are using dynamic sql in the first place is because you want to pass a comma delimited list to your sproc and this was the only you could figure out how to make that work. It sounds like your proc needs a major overhaul.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 12, 2013 at 1:19 pm
Melanie Peterson (4/12/2013)
I tried to post the entire stored proc, but something in it was causing a problem with the forum. I suspect it's either the table creation or dropping or the invoking of the execute itself. If someone knows how I can get around this, I'll happily post the whole thing. As for how I'm invoking it, if I doexec getAffectedEmployees "'099999-00005','099999-00004'"
from withing SSMS, I get no results. However, if I copy the actual code from the sp to a query window in SSMS, I get a result set.
That's what I was thinking may be the problem. Glad you got it sorted.
April 12, 2013 at 1:25 pm
SSCrazy Eights - thanks for the warning. The input to this stored procedure, however, is done entirely through choices made in lists populated from the database. The user never has the opportunity to input anything. Furthermore, it's not an app on an outward facing website, just something internal to our rather small company and well within a firewall, DMZ, etc., so I'm not really concerned about SQL injection here. It's always a good warning though, and after what I've just said you still think it should be handled differently, feel free to say so! 🙂
April 12, 2013 at 1:32 pm
Can you try uploading your procedure in a text file (*.txt)? I had issues like this at a previous employer and that was the only way to provide code sometimes.
April 12, 2013 at 1:39 pm
Melanie Peterson (4/12/2013)
SSCrazy Eights - thanks for the warning. The input to this stored procedure, however, is done entirely through choices made in lists populated from the database. The user never has the opportunity to input anything. Furthermore, it's not an app on an outward facing website, just something internal to our rather small company and well within a firewall, DMZ, etc., so I'm not really concerned about SQL injection here. It's always a good warning though, and after what I've just said you still think it should be handled differently, feel free to say so! 🙂
You might be reasonably safe then at least until this has to be opened up to people at home. 🙂
The change is fairly easy actually. I assume that your variable @CLM is what contains your delimited list? This query will return the same thing but it is not vulnerable to injection and it doesn't need dynamic sql either. It does however use the DelimitedSplit8K function. You can find out more about that function and the code to generate it by following the link in my signature about splitting strings.
SELECT t.[EmpLogin],t.Matter, e.AttorneyAndTitle, SUM([Workhours]) AS TotalHours,
c.ClientName + '/' + em.MatterDesc --not quite sure what this was doing
FROM TimeCard t
INNER JOIN Emps e ON e.EmpLogin = t.EmpLogin
INNER JOIN EliteMatter em ON t.Matter = em.MatterName
INNER JOIN EliteClient c ON c.ClientNum = em.ClientNum
INNER JOIN dbo.DelimitedSplit8K(@CLM, ',') x on x.Item = t.Matter
GROUP BY t.EmpLogin, c.ClientName, em.MatterDesc, t.Matter, e.AttorneyAndTitle
ORDER BY TotalHours
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 12, 2013 at 1:54 pm
SSCrazy Eights - thanks very much!
Lynn - I'd love to upload the file, but I can't figure out how to do that either. I think it must be Friday. :blush:
April 12, 2013 at 4:13 pm
Look down to attachments while writing a post.
April 15, 2013 at 8:31 am
OK, it's Monday, brain is (more or less) refreshed and I'm posting the text of the sp, including the T-SQL that seemed to be causing the code to not post to this forum. I'm also including the SELECT statement that I needed to get the thing to work properly, and would have eliminated the need for this post in the first place. :blush:
April 15, 2013 at 8:47 am
Melanie Peterson (4/15/2013)
OK, it's Monday, brain is (more or less) refreshed and I'm posting the text of the sp, including the T-SQL that seemed to be causing the code to not post to this forum. I'm also including the SELECT statement that I needed to get the thing to work properly, and would have eliminated the need for this post in the first place. :blush:
I still say you should look into the DelimitedSplit8K function. It eliminates the need for dynamic sql and you have zero risk of sql injection. It will run just as fast but it is way simpler to read.
Having looked at your code you don't need the temp table either. Your entire proc becomes this simple.
CREATE PROCEDURE [dbo].[GetLitHoldAffectedEmployees]
@CLM as NVARCHAR(max)
AS
SET NOCOUNT ON
;WITH Emps (EmpLogin, AttorneyAndTitle) AS (
SELECT EmpLogin,
CASE WHEN EmpTermDate IS NULL OR EmpTermDate = '' THEN LastName + ', ' + FirstName + ' [' + Title + '] '
ELSE LastName + ', ' + FirstName + ' [' + Title +'] (inactive) ' END as FullName
FROM [LitigationHold].[dbo].[Employees]
where JobCode IN ('100','300','400','500','700','800','1801'))
SELECT t.[EmpLogin],t.Matter, e.AttorneyAndTitle, SUM([Workhours]) AS TotalHours,
c.ClientName + '/' + em.MatterDesc
FROM TimeCard t
INNER JOIN Emps e ON e.EmpLogin = t.EmpLogin
INNER JOIN EliteMatter em ON t.Matter = em.MatterName
INNER JOIN EliteClient c ON c.ClientNum = em.ClientNum
INNER JOIN dbo.DelimitedSplit8K(@CLM, ',') s on s.Item = t.Matter
GROUP BY t.EmpLogin, c.ClientName, em.MatterDesc, t.Matter, e.AttorneyAndTitle
ORDER BY TotalHours
Test is out but I think that will produce the exact same results.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
Viewing 13 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply