can i remove loop with this and is there any other better

Question

Post reply

can i remove loop with this and is there any other better

rajemessage 14195

Hall of Fame

Points: 3299
More actions
November 30, 2016 at 3:33 am

#323792

hi,
1) should i remove loop with follwiong query.
2) and is there any other way to do it, with out using loop.
declare @STR varchar(max)=''
select @STR=@str+'exec [dbo].[usptest]' + ' '''+abc.name+'''; '
from abc
exec(@str )
yours sincerly

Viewing 12 posts - 1 through 11 (of 11 total)

You must be logged in to reply to this topic. Login to reply

Thom A SSC Guru Points: 99126 More actions · Answer 1

declare @STR varchar(max)=''

select @STR=@str+'exec [dbo].[usptest]' + ' '''+abc.name+'''; '

from abc

exec(@str)

There isn't a loop in the SQL you've given us here. Your code will simply concatenate your strings, and then EXECs your statement as Dynamic SQL.

A loop would defined as something like:

DECLARE @i INT = 1;

CREATE TABLE #LoopTable (i INT,

s INT);

WHILE @i <= 10 BEGIN

INSERT INTO #LoopTable

VALUES (@i, @i * @i);

SET @i = @i + 1;

END

SELECT *

FROM #LoopTable;

DROP TABLE #LoopTable;

Note the WHILE and END statements. Is your code part of a bigger process?

Thom~

Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk

rajemessage 14195 Hall of Fame Points: 3299 More actions · Answer 2

my question is ,

is my version is correct or is there any harm in it?

secondly removing loop with set base is good?

third is there any way to write, my version, with out using loop , in any other way?

John Mitchell-245523 SSC Guru Points: 148809 More actions · Answer 3

Yes, in most cases replacing loops with set-based code is a good thing. There's nothing wrong with your code as written. You can also write it something like this (precise syntax not checked):dec lare @STR varchar(max)

SET @STR = (

SELECT 'exec [dbo].[usptest] ''' +name + '''; '

FROM abc

FOR XML PATH ('')

)

exec(@str)

John

Grant Fritchey SSC Guru Points: 398690 More actions · Answer 4

I don't see any protection against SQL injection with this approach. I'd be a bit cautious about it. You don't want to meet Bobby Tables[/url].

"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt

Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning

rajemessage 14195 Hall of Fame Points: 3299 More actions · Answer 5

i think my version is fast , so should i use my version. or is there any thing wrong , so that i use yours.

John Mitchell-245523 SSC Guru Points: 148809 More actions · Answer 6

Grant Fritchey (11/30/2016)
I don't see any protection against SQL injection with this approach. I'd be a bit cautious about it. You don't want to meet Bobby Tables[/url].

There's no user input - that's the main protection. You'd need to be able to trust what's in your table - maybe the data type or constraints on the column make SQL injection impossible, or maybe the table is read-only. But yes, SQL injection needs to be foremost in one's mind when writing code such as this. I should have mentioned that.

John

Grant Fritchey SSC Guru Points: 398690 More actions · Answer 7

John Mitchell-245523 (11/30/2016)
Grant Fritchey (11/30/2016)
I don't see any protection against SQL injection with this approach. I'd be a bit cautious about it. You don't want to meet Bobby Tables[/url].
There's no user input - that's the main protection. You'd need to be able to trust what's in your table - maybe the data type or constraints on the column make SQL injection impossible, or maybe the table is read-only. But yes, SQL injection needs to be foremost in one's mind when writing code such as this. I should have mentioned that.
John

Is abc.name from a table or is it from some code? If it's a table, sure, I guess you leave it be. If it's code, I'd rather see the use of sp_executesql and an actual parameter with a data type to ensure we're not visiting the Tables family.

"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt

Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning

Sean Lange SSC Guru Points: 286573 More actions · Answer 8

From what is posted there is no need for dynamic sql at all. Why introduce the possibility of sql injection when you could simply skip the dyanmic sql entirely?

_______________________________________________________________

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/

John Mitchell-245523 SSC Guru Points: 148809 More actions · Answer 9

Grant Fritchey (11/30/2016)
Is abc.name from a table or is it from some code? If it's a table, sure, I guess you leave it be. If it's code, I'd rather see the use of sp_executesql and an actual parameter with a data type to ensure we're not visiting the Tables family.

According to the code, it's from a table called abc. You're right, though - this is easily parameterised and sp_executesql would be a better choice here.

Sean Lange (11/30/2016)
From what is posted there is no need for dynamic sql at all. Why introduce the possibility of sql injection when you could simply skip the dyanmic sql entirely?

Yes, you could loop through the table and execute the stored procedure with the parameters one at a time. If the time the stored procedure takes to execute is long compared to the extra time it takes to do the loop instead of building all the EXEC dbo.usptest statements in one go, that would probably get my vote, for the reason you mentioned.

rajemessage 14195 (11/30/2016)
i think my version is fast , so should i use my version. or is there any thing wrong , so that i use yours.

Only you can make that decision. Read all the comments that have been made (and any more that will be), do some performance testing with each query you're considering, and decide.

John

Sean Lange SSC Guru Points: 286573 More actions · Answer 10

John Mitchell-245523 (11/30/2016)

Sean Lange (11/30/2016)
From what is posted there is no need for dynamic sql at all. Why introduce the possibility of sql injection when you could simply skip the dyanmic sql entirely?
Yes, you could loop through the table and execute the stored procedure with the parameters one at a time. If the time the stored procedure takes to execute is long compared to the extra time it takes to do the loop instead of building all the EXEC dbo.usptest statements in one go, that would probably get my vote, for the reason you mentioned.

I totally missed that they are building up the string with multiple EXEC statement in it. Something still seems to be missing here though.

_______________________________________________________________

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/

Jeff Moden SSC Guru Points: 1003863 More actions · Answer 11

rajemessage 14195 (11/30/2016)
hi,
1) should i remove loop with follwiong query.
2) and is there any other way to do it, with out using loop.
declare @STR varchar(max)=''
select @STR=@str+'exec [dbo].[usptest]' + ' '''+abc.name+'''; '
from abc
exec(@str )
yours sincerly

So, in other words, you want to execute the stored procedure for every row in table ABC.

In that case, the way you've done it has no performance benefit over the use of a loop whatsoever. The disadvantage of the method you used may be in the lack of error handling and reporting for each execution of the stored procedure and the ability to continue for other rows should an exception take place.

The real problem here is the fact that you have a RBAR stored procedure (usptest) that will only process one row at a time. In other words, you're using a stored procedure like a scalar function. A lot of people do this because they're written a stored procedure to service a singleton request from the front end and then try to use that same stored procedure to process batches of data and it never works out for performance or resource usage.

Based on the name of the stored procedure, I believe that the best thing to do would be to rewrite the stored procedure as an iTVF (Inline Table Valued Function) and CROSS APPLY it with the SELECT from the abc table. As you write the code for the iTVF, remember that if the word BEGIN appears in the code, then you haven't written an iTVF and performance will suffer.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)