Query to compare execution performance metrics for the three most recent months

Question

I've been working on a semi-awkward query in that it uses a very high number of functions given its relatively small size and scope. I was hoping to get some feedback on any ways I could format or re-factor this better?

select Name ,
 avg(TimeProcessing / 1000 + TimeRendering / 1000 + TimeDataRetrieval / 1000) as 'Current Month' ,
 isnull(count(TimeProcessing), 0) as 'Sample' ,
 min(l2.[Avg Exec Previous Month]) as 'Previous Month' ,
 isnull(min(l2.[Sample Previous Month]), 0) as 'Sample' ,
 min(l3.[Avg Exec Two Months Ago]) as 'Two Months ago' ,
 isnull(min(l3.[Sample Two Months Ago]), 0) as 'Sample'
from marlin.report_execution_log l
 inner join marlin.report_catalog c on l.ReportID = c.ItemID
 left outer join ( select l2.ReportID ,
 avg(TimeProcessing / 1000 + TimeRendering
 / 1000 + TimeDataRetrieval / 1000) as 'Avg Exec Previous Month' ,
 count(TimeProcessing) as 'Sample Previous Month'
 from marlin.report_execution_log l2
 where TimeEnd between dateadd(MONTH, -2,
 getdate())
 and dateadd(MONTH, -1,
 getdate())
 group by l2.ReportID
 ) l2 on l.ReportID = l2.ReportID
 left outer join ( select l3.ReportID ,
 avg(TimeProcessing / 1000 + TimeRendering
 / 1000 + TimeDataRetrieval / 1000) as 'Avg Exec Two Months Ago' ,
 count(TimeProcessing) as 'Sample Two Months Ago'
 from marlin.report_execution_log l3
 where TimeEnd between dateadd(MONTH, -3,
 getdate())
 and dateadd(MONTH, -2,
 getdate())
 group by l3.ReportID
 ) l3 on l.ReportID = l3.ReportID
group by l.ReportID, Name
order by Name

• Clockwork-Muse: Because I sincerely hope that you aren't actually getting a 'Month' value (say, 1 for 'January') by using the AVG() function, you should probably rename Current Month to whatever the actual measure is, including units - perhaps Average Time Elapsed in Seconds or something. And you might consider changing the operations to (TimeProcessing + TimeRendering + TimeDataRetrieval) / 1000, which will guarantee that the same conversion is used for each field.
• Michael A: Not at all, avg is receiving the average execution times of the references columns (see the where clause to understand how it's identifying what month)
• Clockwork-Muse: See, that's exactly what I'm talking about - The result column name doesn't identify, at all what the data is. Some other interesting things I didn't really look at before: Are you sure your main SELECT AVG() is only getting the current month? Without a WHERE clause, it won't restrict it - not if the previous two months are also in the table. Also, be careful when using between - currently, anything equal to dateadd(MONTH, -2, getdate()) will be counted twice - once for l2, and once for l3 - I reccomend exclusive upper bounds.

Answer 1

You might want to use Common Table Expression or Should use meaningful table alias in Join.

You might also want to use indexes on your date column with <= and>= operator instead of between.

Surround your column names in [] instead of single quotes.

• Michael A: Can you elaborate more on what makes a meaningful table alias? I would have thought ReportID (matching over each table and being key fields) would be a great match here and a cte seems unnecessary?
• Nilesh Thakkar: l2 and l3 doesn't convey anything. Rather you should name it something like (e.g) AvgExecutionPreviousMonth.
• Michael A: Right. But that doesn't address my question about why we would use a CTE here?
• Nilesh Thakkar: If it's easy to read and understand the whole thing, you should not get your subquery as CTE.
• Clockwork-Muse: I second the CTE idea, as it will allow the exact same query to be used for the two subqueries (meaning that maintanence can't cause divergent results). Nil - maybe you could show an example of how you'd write the CTE?

Answer 2

I agree with Nil; it seems like this might be a good situation to use a Common Table Expression.

I haven't tested this out, but below is my attempt to rewrite this query using a CTE:

with ReportByMonth (ReportID, [Year], [Month], [Avg Exec], [Sample]) as
(
 select ReportID ,
 avg(TimeProcessing / 1000 + TimeRendering / 1000 + TimeDataRetrieval / 1000) as [Avg Exec] ,
 datepart(YEAR, TimeEnd) as [Year] ,
 datepart(MONTH, TimeEnd) as [Month] ,
 count(TimeProcessing) as [Sample]
 from Marlin.report_execution_log
 group by
 ReportID,
 datepart(YEAR, TimeEnd),
 datepart(MONTH, TimeEnd)
)
select Name ,
 CurrentMonthReport.[Avg Exec] as [Current Month Avg Exec] ,
 isnull(CurrentMonthReport.[Sample], 0) as [Current Month Sample] ,
 PreviousMonthReport.[Avg Exec] as [Previous Month Avg Exec] ,
 isnull(PreviousMonthReport.[Sample], 0) as [Previous Month Sample] ,
 TwoMonthAgoReport.[Avg Exec] as [Two Months Ago Avg Exec] ,
 isnull(TwoMonthAgoReport.[Sample], 0) as [Two Months Ago Sample]
from marlin.report_catalog c
inner join ReportByMonth CurrentMonthReport on
 CurrentMonthReport.ReportID = c.ItemID
 and CurrentMonthReport.Year = datepart(YEAR, getdate())
 and CurrentMonthReport.Month = datepart(MONTH, getdate())
left outer join ReportByMonth PreviousMonthReport on
 PreviousMonthReport.ReportID = c.ItemID
 and PreviousMonthReport.Year = datepart(YEAR, dateadd(MONTH, -1, getdate()))
 and PreviousMonthReport.Month = datepart(MONTH, dateadd(MONTH, -1, getdate()))
left outer join ReportByMonth TwoMonthAgoReport on
 TwoMonthAgoReport.ReportID = c.ItemID
 and TwoMonthAgoReport.Year = datepart(YEAR, dateadd(MONTH, -2, getdate()))
 and TwoMonthAgoReport.Month = datepart(MONTH, dateadd(MONTH, -2, getdate()))
order by
 Name
;

EDIT: I changed my query to try to match the behavior OP's original query. I changed the join for CurrentMonthReport from "left outer join" to "inner join". This means that if a particular report has not been run during this month, then it won't show up in the result.

I also included use of the isnull function and added an order by clause.

• Michael A: On initial testing this doesn't return any results. Just got in the door so I'll get some work done and will then take a look at this and update you further.