Need help optimizing TSQL

  • I have a stored proc that isn't horrible, it runs in a few seconds, it just has high executions and an improvement would be good overall.  It shows up as the worst performing query by CPU in the plan cache (mostly because of # of executions).

    I get a spool that I believe if eliminated it would run quite a bit faster.  I believe the spool is coming from this section:

    Bank = (Select Bank From Customer Where Customer=t.Customer)      (or at least from the 'OR' clause)

    I can't add the actual plan or sample data.  I think the query isn't that complex and someone can hopefully point me in the right direction on how I can write it better.

    Thank You

    [dbo].[Sel_Transf_Ext_Hist] (@Customer int, @Session int=0) AS
    Select Distinct(Transfer),DebitsAccount,CreditsAccount,Amount,Status,CreateDate,EffectiveDate,ModDate,ExternalAccountIs,Memo
    From dbo.Transfer_External t With(NOLOCK)
    Where (Status > 0 and Status < 301) AND (@Session = 0 or @session = SessionID)
    AND ((Customer = @Customer) OR (Customer IN (Select Customer From Customer c Where t.CustomerNumber=c.CustomerNumber
    AND Profile IS NOT NULL AND Bank = (Select Bank From Customer Where Customer=t.Customer))))
    Order By Transfer Desc
    GO

    sel_trans_hist

    sel_trans_hist_reads

    The Customer table has 800K rows, the Transfer_Ext table has 30K rows.

     

  • First comment - make sure you use the table aliases in your sub-queries.  This not only helps identify what you are comparing - but if you reference a column that exists in the outer query but does not exist in the sub-query you can get incorrect results.

    As for your issue - try removing that sub-query, it isn't needed.  You are already selecting from the customer - and in the where for that sub-query you are limiting the results to: Where t.CustomerNumber=c.CustomerNumber

    The Bank value cannot be anything but the Bank value from that Customer so further filtering the results isn't necessary.

     

    Jeffrey Williams
    “We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

    ― Charles R. Swindoll

    How to post questions to get better answers faster
    Managing Transaction Logs

  • I don't understand what you are trying to do or your data. But I think the part:

    Bank = (Select Bank From Customer Where Customer=t.Customer)

    is redundant.

    I think this is equivalent to your SQL:

    CREATE PROCEDURE [dbo].[Sel_Transf_Ext_Hist]
    (
    @Customer INT,
    @Session INT = 0
    )
    AS
    SELECT DISTINCT
    t.Transfer,
    t.DebitsAccount,
    t.CreditsAccount,
    t.Amount,
    t.STATUS,
    t.CreateDate,
    t.EffectiveDate,
    t.ModDate,
    t.ExternalAccountIs,
    t.Memo
    FROM dbo.Transfer_External t WITH(NOLOCK)
    WHERE STATUS > 0
    AND STATUS < 301
    AND @Session IN(0, t.SessionID)
    AND (t.Customer = @Customer
    OR EXISTS(SELECT *
    FROM Customer c
    WHERE c.CustomerNumber = t.CustomerNumber
    AND c.Customer = t.Customer
    AND c.Profile IS NOT NULL
    )
    )
    ORDER BY Transfer DESC;

     

  • Thanks guys.  I also thought the Bank filter may be unnecessary.  I tried removing it and got one more record than with it, I'll look into that.  It ran extremely fast without it.  I'll work with our developer on it.

    Also, I'm new at this place and didn't write the SP,  I agree with your comments about aliasing etc.

  • rothj wrote:

    Thanks guys.  I also thought the Bank filter may be unnecessary.  I tried removing it and got one more record than with it, I'll look into that.  It ran extremely fast without it.  I'll work with our developer on it.

    Also, I'm new at this place and didn't write the SP,  I agree with your comments about aliasing etc.

    Generically, ask why they are using DISTINCT and NOLOCK also.

    I've been at my position for 8 years now.   When I first came onboard, every query had distinct and nolock.

    My predecessor as the DBA  made the developers add nolock to every query. Distinct came from one of the developers, he thought you had to have that in every query!

     

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

  • Agreed, the NOLOCK was already on my radar.

  • I think this is the equivalent of your original statement.

    CREATE PROCEDURE [dbo].[Sel_Transf_Ext_Hist]
    (
    @Customer INT,
    @Session INT = 0
    )
    AS
    SELECT DISTINCT
    t.Transfer,
    t.DebitsAccount,
    t.CreditsAccount,
    t.Amount,
    t.STATUS,
    t.CreateDate,
    t.EffectiveDate,
    t.ModDate,
    t.ExternalAccountIs,
    t.Memo
    FROM dbo.Transfer_External t WITH(NOLOCK)
    WHERE STATUS > 0
    AND STATUS < 301
    AND @Session IN(0, t.SessionID)
    AND (t.Customer = @Customer
    OR EXISTS(SELECT *
    FROM Customer c
    WHERE c.CustomerNumber = t.CustomerNumber
    AND c.Profile IS NOT NULL
    AND EXISTS(SELECT *
    FROM Customer c2
    WHERE c2.Customer = t.Customer
    AND c2.Bank = c.Bank)
    )
    )
    ORDER BY t.Transfer DESC;

    Does it return the correct number of rows?

  • Yes!   Thank You very much!

  • What is the difference in using 'EXISTS' instead of 'IN'?

  • rothj wrote:

    What is the difference in using 'EXISTS' instead of 'IN'?

    The simple answer is that EXISTS is usually a better query plan. As I understand it, EXISTS stops when the first occurrence of the value is found, IN retrieves all of the records.

    The "rules of thumbs" here are:

    1. If there are no columns being returned in the dataset from a query, don't use a join, use exists
    2. Use IN when you have a "static" string, such as a list of numbers.

    As usual, it depends.  One of the bad habits here is if we tune a proc, and apply whatever technique(s) that makes it better,  that technique then becomes the rule with the developers. It's a regular conversation when I ask "Why did you do it that way" and get the answer "Because that's what you did when you fixed XYZ"

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

  • Jonathan AC Roberts wrote:

    I don't understand what you are trying to do or your data. But I think the part:

    Bank = (Select Bank From Customer Where Customer=t.Customer)

    is redundant.

    I'm not sure it is.  The columns being compared are different.

    The first comparison uses CustomerNumber and the second uses Customer.

    Sample data would be most helpful, but I think it's possible that the original comparisons are valid.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • rothj wrote:

    Agreed, the NOLOCK was already on my radar.

    Removing NOLOCK would only slow down the process, never speed it up.  I'm not saying you should or shouldn't use it here -- it depends on the details of each table usage -- but it won't help performance to remove NOLOCK.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • ScottPletcher wrote:

    Jonathan AC Roberts wrote:

    I don't understand what you are trying to do or your data. But I think the part:

    Bank = (Select Bank From Customer Where Customer=t.Customer)

    is redundant.

    I'm not sure it is.  The columns being compared are different.

    The first comparison uses CustomerNumber and the second uses Customer.

    Sample data would be most helpful, but I think it's possible that the original comparisons are valid.

    Good catch, you may be correct.  I looked for duplicates in CustomerNumber (Customer Table) that had multiple banks, there are a lot of them.  And the Customer is different.  These are bogus numbers but you get the idea:

    sel_trans_hist_cust

     

  • But when Jonathon added both columns the WHERE clause, it might have taken care of it.

    c.CustomerNumber = t.CustomerNumber

    AND c.Customer = t.Customer

     

    Jonathon, your second query brought the spool back.  But I tried numerous CustomerNumbers that had dups with your first query and couldn't get a different result from the original.

  • rothj wrote:

    But when Jonathon added both columns the WHERE clause, it might have taken care of it.

    c.CustomerNumber = t.CustomerNumber AND c.Customer = t.Customer

    Jonathon, your second query brought the spool back.  But I tried numerous CustomerNumbers that had dups with your first query and couldn't get a different result from the original.

    I don't think so.  That WHERE condition would have to be met by a single row, the other method could be (indeed seemed intended to allow) different rows to match each part.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

Viewing 15 posts - 1 through 15 (of 35 total)

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