I found an ugly query, can’t think about how to improve it…..

Home Forums Other Microsoft Servers and SaaS SQL Server 2005 / 2008 / 2008 R2 / 2012 / 2016 I found an ugly query, can’t think about how to improve it…..

This topic contains 3 replies, has 3 voices, and was last updated by Avatar Egyptian _Hacker71 4 years, 1 month ago.

Viewing 4 posts - 1 through 4 (of 4 total)
  • Author
    Posts
  • Avatar
    QuattroDave
    Member
    #165722

    Afternoon all,

    The other day I was looking at our SQL 2008 R2 server trying to find a glitch, I found and fixed the glitch but I came across a query that really doesn’t look good at all. In the scenario its in it actually works OK but the potential for it not working is rather high… I’ve been pouring over it for a few days now but I cannot seem to find a cleaner way to do it… Can some one with a fresh pair of eyes have a look see what they think please…

    The JobGroup can be a number of different things but the CASE Statement (as far as I’m aware) is there to process the folowing like so:

    If JobGroup = ‘T Primary’ then it needs to return ‘T Primary’ & ‘T Primary & Secondary’
    If JobGroup = ‘T Secondary’ then it needs to return ‘T Secondary’ & ‘T Primary & Secondary’
    If JobGroup = ‘T Primary & Secondary’ then it needs to return ‘T Primary’ & ‘T Secondary’ & ‘T Primary & Secondary’

    I’ve just re-read that a few times, I hope it makes sense….

    Many thanks

    Dave

    /**Declarations for bench testing **/
    Declare @Wanted_JobGroup VarChar(30)
    SET @Wanted_JobGroup = ‘T Primary & Secondary’
    Declare @StartDate Date
    Set @StartDate = ‘2015-09-04’
    Declare @EndDate Date
    Set @EndDate = ‘2015-09-04’
    Declare @S_ID Int
    Set @S_ID = 1021

    /** Main Query **/

    SELECT T.T_TID,
    (T.T_Title + ‘ ‘ + T.T_FName + ‘ ‘ + T.T_LName) AS Name,
    T.T_Address1, T.T_Address2, T.T_Address3, T.T_Address4,
    T.T_Postcode, T.T_Phone_LL, T.T_Phone_Mobile,
    T.T_Email, T.T_JobGroup, A.*,
    T_Ratings.TR_Rating,
    Geolocation.Longitude,
    Geolocation.Latitude
    FROM dbo.T
    FULL OUTER JOIN dbo.Geolocation
    ON Geolocation.Postcode = REPLACE (T.T_Postcode, RIGHT(T.T_Postcode, 3), ”)
    FULL OUTER JOIN S
    ON S.S_SID = @S_ID
    FULL OUTER JOIN dbo.T_Ratings
    ON T_Ratings.TR_TID = T.T_TID
    AND T_Ratings.TR_SID = @S_ID
    OUTER APPLY
    (SELECT TOP 1 A_Colour
    FROM Availability
    WHERE A_TID = T_TID
    AND A_AvailDate >= @StartDate
    AND A_AvailDate < = @EndDate GROUP BY A_Colour ORDER BY COUNT(*) Desc)A WHERE T.T_Status = 'Live' AND T.T_JobGroup LIKE /** Its this case statement below I take exception to **/ CASE WHEN @Wanted_JobGroup = 'T Primary' THEN '%T%' + '%Primary%' WHEN @Wanted_JobGroup = 'T Secondary' THEN '%T%' + '%Secondary%' WHEN @Wanted_JobGroup = 'T Primary & Secondary' THEN '%T%' + ('%ary%') ELSE @Wanted_JobGroup END AND (T_Ratings.TR_Rating IS Null OR T_Ratings.TR_Rating NOT LIKE 'X%') [/CODE][CODE]
    /**Declarations for bench testing **/
    Declare @Wanted_JobGroup VarChar(30)
    SET @Wanted_JobGroup = ‘T Primary & Secondary’
    Declare @StartDate Date
    Set @StartDate = ‘2015-09-04’
    Declare @EndDate Date
    Set @EndDate = ‘2015-09-04’
    Declare @S_ID Int
    Set @S_ID = 1021

    /** Main Query **/

    SELECT T.T_TID,
    (T.T_Title + ‘ ‘ + T.T_FName + ‘ ‘ + T.T_LName) AS Name,
    T.T_Address1, T.T_Address2, T.T_Address3, T.T_Address4,
    T.T_Postcode, T.T_Phone_LL, T.T_Phone_Mobile,
    T.T_Email, T.T_JobGroup, A.*,
    T_Ratings.TR_Rating,
    Geolocation.Longitude,
    Geolocation.Latitude
    FROM dbo.T
    FULL OUTER JOIN dbo.Geolocation
    ON Geolocation.Postcode = REPLACE (T.T_Postcode, RIGHT(T.T_Postcode, 3), ”)
    FULL OUTER JOIN S
    ON S.S_SID = @S_ID
    FULL OUTER JOIN dbo.T_Ratings
    ON T_Ratings.TR_TID = T.T_TID
    AND T_Ratings.TR_SID = @S_ID
    OUTER APPLY
    (SELECT TOP 1 A_Colour
    FROM Availability
    WHERE A_TID = T_TID
    AND A_AvailDate >= @StartDate
    AND A_AvailDate < = @EndDate
    GROUP BY A_Colour
    ORDER BY COUNT(*) Desc)A
    WHERE
    T.T_Status = ‘Live’
    AND T.T_JobGroup LIKE

    /** Its this case statement below I take exception to **/
    CASE
    WHEN @Wanted_JobGroup = ‘T Primary’ THEN
    ‘%T%’ + ‘%Primary%’
    WHEN @Wanted_JobGroup = ‘T Secondary’ THEN
    ‘%T%’ + ‘%Secondary%’
    WHEN @Wanted_JobGroup = ‘T Primary & Secondary’ THEN
    ‘%T%’ + (‘%ary%’)
    ELSE
    @Wanted_JobGroup
    END

    AND (T_Ratings.TR_Rating IS Null
    OR T_Ratings.TR_Rating NOT LIKE ‘X%’)
    [/CODE]

    Avatar
    Ossian
    Moderator
    #190864

    tbh it looks OK – complex, but effective
    You could simplify it by dropping the %T% part IF all job groups start T Something

    #384258

    Thanks for your reply, don’t get me wrong it works fine and to date it has worked with out a hitch, I’m just a bit wary as its not very explicit and could lead to errors… No only 3 job groups begin T Something… But if someone adds another one it may well qualify, I mean ‘%T%’ + ‘%ary%’ isn’t very explicit is it… :-/

    Thanks

    Dave

    Avatar
    Ossian
    Moderator
    #190865

    Hi Dave
    It does rely on the current group names
    The %T% catches those groups which include a T – note that to get only groups which START with a T, T% would be a better call
    The %ary% would catch both “primary” and “secondary”

    The risk is you have another group such as “X tertiary” which would be caught because it contains a T and “ary”

Viewing 4 posts - 1 through 4 (of 4 total)

You must be logged in to reply to this topic.