Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2008 Forums
 Transact-SQL (2008)
 Can this be written without a cursor?

Author  Topic 

ferrethouse
Constraint Violating Yak Guru

352 Posts

Posted - 2011-11-17 : 17:07:46
I see quite a few functions in our database that do something similar. This one in particular is called 18 times per second so I'm guessing that getting rid of the cursor would greatly improve performance...


ALTER FUNCTION [dbo].[FuncGetSmartDepartmentRules]
(
-- Add the parameters for the function here
@DepartmentID INT
)
RETURNS VARCHAR(MAX)
AS
BEGIN
-- Declare the return variable here


DECLARE @Sql VARCHAR (MAX),@SqlFilter VARCHAR(MAX)
-- Declare cursor
DECLARE smart_deps CURSOR FAST_FORWARD FOR
SELECT SQLFilter
FROM SmartDepartments
WHERE DepartmentID = @DepartmentID

OPEN smart_deps

FETCH NEXT FROM smart_deps
INTO @SqlFilter
--first part of sql
SET @Sql = @SqlFilter

FETCH NEXT FROM smart_deps
INTO @SqlFilter

WHILE @@FETCH_STATUS = 0
BEGIN

SET @Sql = @Sql + ' OR ' + @SqlFilter

FETCH NEXT FROM smart_deps
INTO @SqlFilter
END

CLOSE smart_deps
DEALLOCATE smart_deps

-- Return the result of the function
RETURN @Sql

END

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2011-11-17 : 17:20:07
No need for a cursor here. A sinple select into a variable will/should work. I might use the FOR XML method incase SQL decides to change how selecting into a variable works. Here is an article that has several different methods (the first one should work just fine).
http://databases.aspfaq.com/general/how-do-i-concatenate-strings-from-a-column-into-a-single-row.html
Go to Top of Page

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-11-17 : 17:22:20
Ok.................

You may not be aware of this but It's pretty obvious that your database is WIDE OPEN potentially vulnerable to SQL INJECTION attack.

If anyone manages to get some malicious sql into [SQLFilter] in the SmartDepartments table then this is going to spit out a really compromised dynamic string back to some proc that'll just blindly execute it? or do you sanity check the results from the func??

Anyway, to answer your question (get rid of cursor)

You can concatenate strings using a vanilla SELECT statement. No need for the cursor

Example:

DECLARE @sql NVARCHAR(MAX)
SET @sql = N'
SELECT *
FROM someTable
WHERE
1 = 0'


DECLARE @foo TABLE (
[injectionAttack] NVARCHAR(MAX)
)

INSERT @foo ([injectionAttack])
SELECT '1 = 1'
UNION SELECT '1 = 1; DROP TABLE Sales; --'
UNION SELECT 'x = y'

-- Concatenate the string
SELECT @sql = sql + ' OR ' + [InjectionAttack]
FROM @foo

PRINT @sql



which would eliminate the need for a cursor but wouldn't help your vulnerability any

If you are going to do shit like this then you NEED TO CHECK ALL INPUTS. Regardless of if they are user generated or come from some database table. Just because the strings are in the db already doesn't mean they are safe.

EDIT -- too harsh
Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION
Go to Top of Page

sanjnep
Posting Yak Master

191 Posts

Posted - 2011-11-17 : 17:45:02
I think so...
Go to Top of Page

ferrethouse
Constraint Violating Yak Guru

352 Posts

Posted - 2011-11-17 : 17:51:44
The inputs are checked when put into table SmartDepartments
Go to Top of Page

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-11-17 : 17:59:06
cool. Do you ever check them again once they are in there?

At least you can remove that cursor now!

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION
Go to Top of Page

ferrethouse
Constraint Violating Yak Guru

352 Posts

Posted - 2011-11-17 : 18:01:17
quote:
Originally posted by Transact Charlie

cool. Do you ever check them again once they are in there?

At least you can remove that cursor now!

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION




Why would I need to check them again?
Go to Top of Page

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-11-17 : 18:19:51
bulk import / some other mechanism that bypasses your std insert checks.

basically just be really, really, really cautious. I always get nervous when people propose code like that.

If I have to do any kind of general purpose search (searching a dynamic number of columns from a table). I'd much rather implement a dynamic sql solution that doesn't run the risk of sql injection at all.

Gail posted a good article on 'catch all' queries that might be of interest to you.

http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION
Go to Top of Page

ferrethouse
Constraint Violating Yak Guru

352 Posts

Posted - 2011-11-17 : 18:30:14
quote:
Originally posted by Transact Charlie

bulk import / some other mechanism that bypasses your std insert checks.

basically just be really, really, really cautious. I always get nervous when people propose code like that.

If I have to do any kind of general purpose search (searching a dynamic number of columns from a table). I'd much rather implement a dynamic sql solution that doesn't run the risk of sql injection at all.

Gail posted a good article on 'catch all' queries that might be of interest to you.

http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION




I appreciate your feedback Charlie
Go to Top of Page

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-11-17 : 18:42:23
welcome

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION
Go to Top of Page
   

- Advertisement -