Search code examples
sql-server-2008performance-testing

Optimizing SQL- swapping out functions for JOINS and confusion when variables exist


I just started a new job in Dev Ops at a publishing company. My first task is to optimize a huge SQL query that is composed of functions. Functions are really slow. The guy that created the query was smart but didn't know SQL and used functions when he could have used JOINs instead. I am having trouble with converting the functions that have variables. For example, this is one of the functions. Here it is within the query and next is the associated function stored elsewhere.

dbo.rpt_get_isbn(b.bookkey, 21) AS 
f_upc

Then the function...

ALTER FUNCTION [dbo].[rpt_get_isbn](
        @i_bookkey  INT,
        @i_isbn_type    INT)

/*  Returns the identifier such as EAN, 
ISBN, with or without dashes


   PARAMETER @i_isbn_type
        10 = ISBN10
        13 = ISBN 13
        16 = EAN
        17 = EAN (no dashes)
        18 = GTIN
        19 = GTIN (no dashes)
        20 = LCCN
        21 = UPC
*/
    RETURNS VARCHAR(50)

AS
BEGIN
        DECLARE @RETURN VARCHAR(50)
        DECLARE @v_desc VARCHAR(50)

    IF @i_isbn_type = 10
        BEGIN
            SELECT @v_desc = isbn10
            FROM isbn
            WHERE bookkey = @i_bookkey
        END

    ELSE IF @i_isbn_type = 13
        BEGIN
            SELECT @v_desc = isbn
            FROM isbn
            WHERE bookkey = @i_bookkey
        END

    ELSE IF @i_isbn_type = 16
        BEGIN
           SELECT @v_desc = ean
           FROM isbn
            WHERE bookkey = @i_bookkey
        END
    ELSE IF @i_isbn_type = 17
        BEGIN
            SELECT @v_desc = ean13
            FROM isbn
            WHERE bookkey = @i_bookkey
        END
    ELSE IF @i_isbn_type = 18
        BEGIN
            SELECT @v_desc = gtin
            FROM isbn
            WHERE bookkey = @i_bookkey
        END
    ELSE IF @i_isbn_type = 19
        BEGIN
            SELECT @v_desc = gtin14
            FROM isbn
            WHERE bookkey = @i_bookkey
        END
    ELSE IF @i_isbn_type = 20
        BEGIN
            SELECT @v_desc = lccn
            FROM isbn
            WHERE bookkey = @i_bookkey
        END
    ELSE IF @i_isbn_type = 21
        BEGIN
            SELECT @v_desc = upc
            FROM isbn
            WHERE bookkey = @i_bookkey
        END
    IF LEN(@v_desc) > 0
        BEGIN
            SELECT @RETURN = 
LTRIM(RTRIM(@v_desc))
        END
    ELSE
        BEGIN
            SELECT @RETURN = ''
        END

  RETURN @RETURN
END

So this function can return various different results based on the variable given as the second parameter. If that wasn't there, this would be easy. I would simply convert it with a solution like this one to retrieve the book's cover..

LTRIM(RTRIM(bo.ean13)) AS p_coverimagepath

and the JOIN needed...

LEFT JOIN Isbn bo WITH (NOLOCK) ON bo.bookkey = b.bookkey

But again, now I'm dealing with parameters and a function that uses If/elses to derive an answer. So do I need to add this if/else logic to my main query? I can't think of a ways that will yield as simple an answer. I look forward to figuring this out. Please let me know if I forgot any crucial elements to understand what I'm doing. Thanks!

UPDATE Here's where the code is going

,pss8.dbo.xml_StripIllegalChars(dbo.rpt_get_series_volume(b.bookkey)) AS 
p_seriesvol
    ,CASE
        WHEN dbo.rpt_get_isbn(b.bookkey, 17) = ''
            THEN (
                    SELECT ipg_id
                    FROM tmmdb.ipg_extra.dbo.vw_Pss8IsbnOrUpc bo
                    WHERE bo.bookkey = b.bookkey
                    )
        ELSE dbo.rpt_get_isbn(b.bookkey, 17)
        END AS p_coverimagepath
    ,CASE
        WHEN dbo.rpt_get_isbn(b.bookkey, 17) = ''
            THEN (
                    SELECT ipg_id
                    FROM tmmdb.ipg_extra.dbo.vw_Pss8IsbnOrUpc bo
                    WHERE bo.bookkey = b.bookkey
                    )
        ELSE dbo.rpt_get_isbn(b.bookkey, 17)
        END AS TSP_p_coverimagepath
    ,pss8.dbo.xml_StripIllegalChars(replace(dbo.rpt_get_title(b.bookkey, 
'T'), '&', '&')) AS p_title /*  30OCT14 */
    
,pss8.dbo.xml_StripIllegalChars(replace(dbo.rpt_get_sub_title(b.bookkey), 
'&', '&')) AS p_subtitle /* 20OCT14 */

Solution

  • Not a complete answer, but an example structure to help you dig your way out of this. I have made many assumptions because your question requires more detail. But hopefully you can appreciate this is way simpler.

    SELECT 
    CASE 
        WHEN ISNULL(LTRIM(RTRIM(bo.ean13)),'') = '' THEN T2.ipg_id 
        ELSE LTRIM(RTRIM(bo.ean13))
    END as p_coverimagepath
    FROM MainTable b
    LEFT JOIN Isbn bo 
    ON bo.bookkey = b.bookkey
    LEFT JOIN tmmdb.ipg_extra.dbo.vw_Pss8IsbnOrUpc T2
    ON T2.bookkey=b.bookkey
    

    In your example code, two columns do exactly the same thing so I haven't repeated this.

    A few things noted:

    -Don't sprinkle NOLOCK in your code thinking it's a performance boost

    -If you're using an outer join, you need to accommodate in case a NULL is returned

    -It's easier to join once to tmmdb.ipg_extra.dbo.vw_Pss8IsbnOrUpc and use columns out of it rather than repeat the code over and over.