Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 
        
Home       Members    Calendar    Who's On


Add to briefcase ««123»»

Can You make this code Shorter??.. Expand / Collapse
Author
Message
Posted Friday, September 27, 2013 2:11 AM
SSC Journeyman

SSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC Journeyman

Group: General Forum Members
Last Login: Monday, October 21, 2013 3:10 AM
Points: 79, Visits: 191
this is it

  Post Attachments 
error.JPG (8 views, 132.69 KB)
Post #1499244
Posted Friday, September 27, 2013 2:14 AM


Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: 2 days ago @ 9:53 PM
Points: 3,438, Visits: 5,390
enriquezreyjoseph (9/27/2013)

SET @SqlQuery = ' SELECT * FROM TestMyView ' +
CASE
WHEN @sexID <> 0 OR @statusID <> 0 OR LEN(@firstname) > 0 OR LEN(@middlename) > 0 OR LEN(@lastname) > 0
THEN ' WHERE 1=1 '
ELSE ''

END +



Above can be changed to just:

SET @SqlQuery = ' SELECT * FROM TestMyView WHERE 1=1' + 


Then just looking at one of the CASE statements:

enriquezreyjoseph (9/27/2013)

CASE
WHEN @sexID <> 0
THEN ' AND sexID = ' + convert(varchar(20), @sexID)
ELSE ''
END +



You should be able to change this to:

        CASE 
WHEN @sexID <> 0
THEN ' AND sexID = @Sexid'
ELSE ''
END +


Thus allowing you to pass @SexID into sp_executesql and avoid the SQL injection issue GilaMonster (Gail Shaw) chastised you about in another thread.



My mantra: No loops! No CURSORs! No RBAR! Hoo-uh!

My thought question: Have you ever been told that your query runs too fast?

My advice:
INDEXing a poor-performing query is like putting sugar on cat food. Yeah, it probably tastes better but are you sure you want to eat it?
The path of least resistance can be a slippery slope. Take care that fixing your fixes of fixes doesn't snowball and end up costing you more than fixing the root cause would have in the first place.


Need to UNPIVOT? Why not CROSS APPLY VALUES instead?
Since random numbers are too important to be left to chance, let's generate some!
Learn to understand recursive CTEs by example.
Splitting strings based on patterns can be fast!
Post #1499247
Posted Friday, September 27, 2013 2:17 AM


Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: 2 days ago @ 9:53 PM
Points: 3,438, Visits: 5,390
enriquezreyjoseph (9/27/2013)
this is it


Never seen that error but it looks like it is complaining about the typing of one of the parameters to your SP.



My mantra: No loops! No CURSORs! No RBAR! Hoo-uh!

My thought question: Have you ever been told that your query runs too fast?

My advice:
INDEXing a poor-performing query is like putting sugar on cat food. Yeah, it probably tastes better but are you sure you want to eat it?
The path of least resistance can be a slippery slope. Take care that fixing your fixes of fixes doesn't snowball and end up costing you more than fixing the root cause would have in the first place.


Need to UNPIVOT? Why not CROSS APPLY VALUES instead?
Since random numbers are too important to be left to chance, let's generate some!
Learn to understand recursive CTEs by example.
Splitting strings based on patterns can be fast!
Post #1499248
Posted Friday, September 27, 2013 2:25 AM
SSC Journeyman

SSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC Journeyman

Group: General Forum Members
Last Login: Monday, October 21, 2013 3:10 AM
Points: 79, Visits: 191
dwain.c (9/27/2013)
enriquezreyjoseph (9/27/2013)

SET @SqlQuery = ' SELECT * FROM TestMyView ' +
CASE
WHEN @sexID <> 0 OR @statusID <> 0 OR LEN(@firstname) > 0 OR LEN(@middlename) > 0 OR LEN(@lastname) > 0
THEN ' WHERE 1=1 '
ELSE ''

END +



Above can be changed to just:

SET @SqlQuery = ' SELECT * FROM TestMyView WHERE 1=1' + 


Then just looking at one of the CASE statements:

enriquezreyjoseph (9/27/2013)

CASE
WHEN @sexID <> 0
THEN ' AND sexID = ' + convert(varchar(20), @sexID)
ELSE ''
END +



You should be able to change this to:

        CASE 
WHEN @sexID <> 0
THEN ' AND sexID = @Sexid'
ELSE ''
END +


Thus allowing you to pass @SexID into sp_executesql and avoid the SQL injection issue GilaMonster (Gail Shaw) chastised you about in another thread.


What should be the syntax in passing to sp_executesql dwain??.... :-(
Post #1499253
Posted Friday, September 27, 2013 2:29 AM


Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: 2 days ago @ 9:53 PM
Points: 3,438, Visits: 5,390
enriquezreyjoseph (9/27/2013)

EXEC sp_executesql @SqlQuery, N'@statusID=@statusID, @sexID=@sexID, @firstname=@firstname, @middlename=@middlename, @lastname=@lastname' -- one for each of your filters
,@sexID=@sexID
,@statusID=@statusID
,@firstname=@firstname
,@middlename=@middlename
,@lastname=@lastname

END



You already had it in your prior post.

You probably should read BOL's examples on this subject:
http://technet.microsoft.com/en-us/library/ms188001.aspx



My mantra: No loops! No CURSORs! No RBAR! Hoo-uh!

My thought question: Have you ever been told that your query runs too fast?

My advice:
INDEXing a poor-performing query is like putting sugar on cat food. Yeah, it probably tastes better but are you sure you want to eat it?
The path of least resistance can be a slippery slope. Take care that fixing your fixes of fixes doesn't snowball and end up costing you more than fixing the root cause would have in the first place.


Need to UNPIVOT? Why not CROSS APPLY VALUES instead?
Since random numbers are too important to be left to chance, let's generate some!
Learn to understand recursive CTEs by example.
Splitting strings based on patterns can be fast!
Post #1499254
Posted Friday, September 27, 2013 2:30 AM


SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Wednesday, December 17, 2014 1:30 AM
Points: 2,386, Visits: 7,622
Why not build up your WHERE clause like this?

ALTER PROCEDURE [dbo].[SearchBiography] @firstname NVARCHAR(50), @middlename NVARCHAR(50), @lastname NVARCHAR(50), @sexID NCHAR(1), @statusID NCHAR(1) AS
BEGIN;
SET NOCOUNT ON;

DECLARE @SqlQuery NVARCHAR(MAX);
DECLARE @WhereClause NVARCHAR(MAX);

SET @WhereClause = STUFF(CASE WHEN @sexID <> '0' THEN ' AND sexID = @sexID' ELSE '' END +
CASE WHEN @statusID <> '0' THEN ' AND statusID = @statusID' ELSE '' END +
CASE WHEN LEN(@firstname) > 0 THEN ' AND firstname LIKE ''%@firstname%''' ELSE '' END +
CASE WHEN LEN(@middlename) > 0 THEN ' AND middlename LIKE ''%@middlename%''' ELSE '' END +
CASE WHEN LEN(@lastname) > 0 THEN ' AND lastname LIKE ''%@lastname%''' ELSE '' END, 1, 4, '');

SET @WhereClause = CASE WHEN LEN(@WhereClause) > 1 THEN 'WHERE'+@WhereClause ELSE @WhereClause END;

SET @SqlQuery = ' SELECT * FROM TestMyView '+@WhereClause;

EXEC sp_executesql @SqlQuery, N'@statusID=@statusID, @sexID=@sexID, @firstname=@firstname, @middlename=@middlename, @lastname=@lastname'
, @sexID = @sexID, @statusID = @statusID, @firstname = @firstname, @middlename = @middlename, @lastname = @lastname
END;




--Edit--

ALTER PROCEDURE [dbo].[SearchBiography] @firstname NVARCHAR(50), @middlename NVARCHAR(50), @lastname NVARCHAR(50), @sexID NCHAR(1), @statusID NCHAR(1) AS
BEGIN;
SET NOCOUNT ON;

DECLARE @SqlQuery NVARCHAR(MAX);
DECLARE @WhereClause NVARCHAR(MAX);

SET @WhereClause = STUFF(CASE WHEN @sexID <> '0' THEN ' AND sexID = @sexID' ELSE '' END +
CASE WHEN @statusID <> '0' THEN ' AND statusID = @statusID' ELSE '' END +
CASE WHEN LEN(@firstname) > 0 THEN ' AND firstname LIKE '+CHAR(39)+'%'+CHAR(39)+@firstname+CHAR(39)+'%'+CHAR(39) ELSE '' END +
CASE WHEN LEN(@middlename) > 0 THEN ' AND middlename LIKE '+CHAR(39)+'%'+CHAR(39)+'%'+@middlename+'%'+CHAR(39)+'%'+CHAR(39) ELSE '' END +
CASE WHEN LEN(@lastname) > 0 THEN ' AND lastname LIKE '+CHAR(39)+'%'+CHAR(39)+'@lastname'+CHAR(39)+'%'+CHAR(39) ELSE '' END, 1, 4, '');

SET @WhereClause = CASE WHEN LEN(@WhereClause) > 1 THEN 'WHERE'+@WhereClause ELSE @WhereClause END;

SET @SqlQuery = ' SELECT * FROM TestMyView '+@WhereClause;

EXEC sp_executesql @SqlQuery, N'@statusID=@statusID, @sexID=@sexID, @firstname=@firstname, @middlename=@middlename, @lastname=@lastname'
, @sexID = @sexID, @statusID = @statusID, @firstname = @firstname, @middlename = @middlename, @lastname = @lastname
END;




Not a DBA, just trying to learn

For better, quicker answers on T-SQL questions, click on the following...
http://www.sqlservercentral.com/articles/Best+Practices/61537/

For better, quicker answers on SQL Server performance related questions, click on the following...
http://www.sqlservercentral.com/articles/SQLServerCentral/66909/



If you litter your database queries with nolock query hints, are you aware of the side effects?
Try reading a few of these links...

(*) Missing rows with nolock
(*) Allocation order scans with nolock
(*) Consistency issues with nolock
(*) Transient Corruption Errors in SQL Server error log caused by nolock
(*) Dirty reads, read errors, reading rows twice and missing rows with nolock


LinkedIn | Blog coming soon (for sufficiently large values of "soon" )!
Post #1499255
Posted Friday, September 27, 2013 2:35 AM


SSC Eights!

SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!

Group: General Forum Members
Last Login: 2 days ago @ 1:37 AM
Points: 997, Visits: 3,089
enriquezreyjoseph (9/27/2013)
this is it

That's because you have declared your statement as varchar. sp_executesql only accepts nvarchar.

Your
DECLARE @SqlQuery varchar(max)

should be
DECLARE @SqlQuery Nvarchar(max)





The SQL Guy @ blogspot

@SeanPearceSQL

About Me
Post #1499256
Posted Friday, September 27, 2013 2:47 AM
SSC Journeyman

SSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC Journeyman

Group: General Forum Members
Last Login: Monday, October 21, 2013 3:10 AM
Points: 79, Visits: 191
Thank you guys..

so, i should change varchar now to my whole table and to my front-end...tsk :-(..
Post #1499258
Posted Friday, September 27, 2013 2:51 AM


SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Wednesday, December 17, 2014 1:30 AM
Points: 2,386, Visits: 7,622
enriquezreyjoseph (9/27/2013)
Thank you guys..

so, i should change varchar now to my whole table and to my front-end...tsk :-(..


You're also vulnerable to SQL injection. Please look over the code I posted and see the difference between it and yours.



Not a DBA, just trying to learn

For better, quicker answers on T-SQL questions, click on the following...
http://www.sqlservercentral.com/articles/Best+Practices/61537/

For better, quicker answers on SQL Server performance related questions, click on the following...
http://www.sqlservercentral.com/articles/SQLServerCentral/66909/



If you litter your database queries with nolock query hints, are you aware of the side effects?
Try reading a few of these links...

(*) Missing rows with nolock
(*) Allocation order scans with nolock
(*) Consistency issues with nolock
(*) Transient Corruption Errors in SQL Server error log caused by nolock
(*) Dirty reads, read errors, reading rows twice and missing rows with nolock


LinkedIn | Blog coming soon (for sufficiently large values of "soon" )!
Post #1499260
Posted Friday, September 27, 2013 3:00 AM
SSC Journeyman

SSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC JourneymanSSC Journeyman

Group: General Forum Members
Last Login: Monday, October 21, 2013 3:10 AM
Points: 79, Visits: 191
Cadavre (9/27/2013)
enriquezreyjoseph (9/27/2013)
Thank you guys..

so, i should change varchar now to my whole table and to my front-end...tsk :-(..


You're also vulnerable to SQL injection. Please look over the code I posted and see the difference between it and yours.


Is ' Stuff ' a keyword???
Post #1499264
« Prev Topic | Next Topic »

Add to briefcase ««123»»

Permissions Expand / Collapse