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

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
getdate())
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
getdate())
getdate())
group by  l3.ReportID
) l3 on l.ReportID = l3.ReportID
group by l.ReportID, Name
order by Name

• 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. – Clockwork-Muse Jan 3 '12 at 18:05
• 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) – Michael A Jan 3 '12 at 22:17
• 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. – Clockwork-Muse Jan 4 '12 at 16:36

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.

• 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? – Michael A Jan 3 '12 at 5:42
• l2 and l3 doesn't convey anything. Rather you should name it something like (e.g) AvgExecutionPreviousMonth. – Nilesh Thakkar Jan 3 '12 at 8:37
• Right. But that doesn't address my question about why we would use a CTE here? – Michael A Jan 3 '12 at 22:18
• If it's easy to read and understand the whole thing, you should not get your subquery as CTE. – Nilesh Thakkar Jan 4 '12 at 5:14
• 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? – Clockwork-Muse Jan 4 '12 at 16:30

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.

• 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. – Michael A Jan 4 '12 at 22:01