Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following SQL query which takes ~3 seconds to run, maybe ~4. This populates a consultants table with their expected payments for the month.

Unfortunately, the page only loads when this query is finished. Is there any way to make it quicker?

Is there anything you notice that I could do better or any best practices I could follow?

DECLARE @lastMonthDate VARCHAR(10)
DECLARE @currentMonthDate VARCHAR(10)
DECLARE @Year VARCHAR(10)

SET @lastMonthDate = CONVERT(VARCHAR(4), DATEPART(YEAR, DATEADD(MONTH, -1, GETDATE())))+'-'+CONVERT(VARCHAR(2), DATEPART(MONTH, DATEADD(MONTH, -1, GETDATE())))+'-21'
SET @currentMonthDate = CONVERT(VARCHAR(4), DATEPART(YEAR, GETDATE()))+'-'+CONVERT(VARCHAR(2), DATEPART(MONTH, GETDATE()))+'-20'
SET @Year = DATEPART(YEAR, GETDATE())

SELECT Comp_Name, comp_idnr, oppo_date17_1sent,datename(month, oppo_date17_1sent), budg_tempDODate1, CONVERT(DECIMAL(10,2), budg_do1_total) AS budg_do1_total, budg_paymentMethodM_R,  v_ClientTransactionSummary.HyphenStatus, v_ClientTransactionSummary.TransactionDate FROM Company

JOIN Opportunity on Company.Comp_CompanyId = Opportunity.Oppo_PrimaryCompanyId
JOIN Budget on  Opportunity.Oppo_OpportunityId = Budget.budg_OpportunityId
JOIN Users on company.Comp_PrimaryUserId = User_UserId
Join Channel on User_PrimaryChannelId = Channel.Chan_ChannelId
JOIN [Server2].PayTrader.dbo.v_ClientTransactionSummary on Company.comp_idnr COLLATE SQL_Latin1_General_CP1_CI_AS = v_ClientTransactionSummary.IDNumber COLLATE SQL_Latin1_General_CP1_CI_AS AND budg_do1_total = v_ClientTransactionSummary.ExpectedAmount AND CONVERT(Date, budg_tempDODate1, 105) = CONVERT(Date, v_ClientTransactionSummary.TransactionDate, 105)
WHERE CONVERT(DATE, budg_tempDODate1) BETWEEN CONVERT(DATE, @lastMonthDate) AND CONVERT(DATE, @currentMonthDate) AND User_FirstName = @Name AND User_LastName = @Lastname AND Oppo_deleted IS NULL
Order by budg_tempDODate1 ASC
share|improve this question
3  
SQL Server generally does a pretty good job of optimizing the query. If you have access to SQL Server Management Studio, put the query in and click Display Estimated Execution Plan button on the tool bar, this might offer some more insight into what is running slow. –  DFord May 15 at 13:43
1  
Yes this is definitely the right place ;) Welcome to Code Review! –  Vogel612 May 15 at 13:44
    
if you do create another follow up question please post the link here so we can follow along, thanks. and I will move my answer there for that part of the code as well –  Malachi May 15 at 14:32
1  
You should read up on "Sargable" where clauses: en.wikipedia.org/wiki/Sargable –  Dibstar May 15 at 15:37
    
Appended code removed. We discourage it as it could complicate the review process (and only the original code needs to be there). You may ask a new question if you'd like further review. –  Jamal May 15 at 16:29

1 Answer 1

up vote 6 down vote accepted

you are doing a lot of CONVERTing in this query.

Instead of converting the Date variables inside the WHERE clause you should do that before you even start the query, otherwise you are running this convert for every row in the SELECT because the WHERE decides if a row should be kept or discarded from the returned table.


Are the date fields not already in DateTime format?

get rid of all the Converts that are not needed.


also you should format your code with some indentation so that it is easier to read, for you and for others who will maintain the code you write.

share|improve this answer
    
Thanks for the advice Malachi, this is great - I have edited my post to show you what I have done, if you could provide me with some more guidance, that would be great. I Converted (Lastmonth) and (CurrentMonth) before the where, this shaved off 2 seconds. Unfortunately, those other columns were created by some idiot in VARCHAR. How would I convert those before the where? –  Mike May 15 at 14:19

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.