SQL Server stored procedure optimization

Above the question or comment text box, there is a toolbar. In that toolbar is the word CODE. The idea behind that feature is that we can read code (such as sql) in a non-proportional font AND we don't have to scroll half a mile downward to read the question. Please use it.

I have edited the question so you may now assess the difference.

Thank you Paul - I wasn't aware of this feature - sorry for the inconvenience

Seems to be you need a nightly job that calculates these metrics for yesterday and stores it into a table.
Then (instead of re-calculating this over multiple tables /joins and where clauses each time) just provide the requested results from that table.

I don't think it'll work with a temp table as the procedure can be called with different  from/to periods

Create indices on the appropriate tables for the expressions like

                 WHERE  v_Visits.visitTime BETWEEN @periodFrom AND @periodTo
                        AND v_Visits.userRole LIKE @uRole
                        AND v_Visits.channelName LIKE @channels

Select allOpen in new window

They require two indices each (visitTime, userRole, channelName) and (userRole, channelName, visitTime) depending on leading wildcard searches.

You may also run in a parameter sniffing issue, thus test it WITH RECOMPILE.

btw, what SQL Server version do you use? CAST(Visit_Comments.date AS DATE) has a code smell. Especially as, while being sargable in WHERE predicates, they can lead do more logical/physical reads and errors in cardinality estimation. Both things can effect a queries performance drastically.

I don't see there any reason for using an different isolation level. Especially READ UNCOMMITED can be nifty.

Thank you for your input - regarding the indexes - the v_Visits is a view - does it "inherit" the indexes from the underlying table(s) ?

And how can I make sure the procedure is run with "SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED" ?

How can I make sure that it runs with "SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED" ?

Why do you want to do that? Is creating locks when you run this sp?

can anyone give me som advice on how to optimize the procedure, as it runs quite slowly at the moment ?

It would be good to attach the query execution plan so we can advice on indexes.
Anyway I can see some similar SELECTs with difference only on the labels. Can't you suppress the repeating ones?

/****** Total ******/
SELECT	vGroup, vType, count(vDetail) AS vCount
FROM (
	SELECT 'OverallWeekDay' AS vGroup, cast(visitTime  as nvarchar(10)) AS vType,  visitTime  as vDate, v_Visits.id AS vDetail
	FROM v_Visits
	WHERE visitTime between @periodFrom and @periodTo AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType
UNION ALL
SELECT	vGroup, vType, count(vDetail) AS vCount
FROM (
	SELECT 'Overall' AS vGroup, 'Visits' AS vType, visitTime  as vDate, v_Visits.id AS vDetail
	FROM v_Visits
	WHERE visitTime  between @periodFrom and @periodTo AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType
UNION ALL
(...)
SELECT	vGroup, vType, count(vDetail) AS vCount
FROM (
	SELECT 'User likes' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
	FROM v_Visits 
		INNER JOIN Visit_Likes ON v_Visits.id = Visit_Likes.visitId
		INNER JOIN Users ON Visit_Likes.userId = Users.id
	WHERE cast (Visit_Likes.date as date) between @periodFrom and @periodTo AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType
UNION ALL
SELECT	vGroup, vType, count(vDetail) AS vCount
FROM (
	SELECT 'User liked' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
	FROM v_Visits 
		INNER JOIN Visit_Likes ON v_Visits.id = Visit_Likes.visitId
		INNER JOIN Users ON v_Visits.userId = Users.id
	WHERE cast (Visit_Likes.date as date) between @periodFrom and @periodTo AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType

/****** Brand ******/
UNION ALL
(...)

Select allOpen in new window

Why do you want to do that? Is creating locks when you run this sp?

If I run a copy with the stored procedure under another name, it only takes 2 sec to run - with out the "READ UNCOMMITTED" it takes 4 sec +

Can't you suppress the repeating ones?

I'm unfortunately not expert in SQL - can you give me a hint on how to do this ? Isn't the difference not only in the labels, but more importantly in the Groups ?

ok, I figured out how to fix the "read uncommitted"

ALTER PROCEDURE [dbo].[getPeriodStatsAlternative]
	@periodFrom nvarchar(8),
	@periodTo nvarchar(8),
	@uRole as nvarchar(10),
	@channels as nvarchar(10) = '%'

AS
BEGIN
	SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED
	BEGIN TRAN

Select allOpen in new window

ok, I figured out how to fix the "read uncommitted"

Do you know that can read information that aren't valid anymore, right?
You should check if it's acceptable for you to provide non accurate data.

Yes I know that it can result in dirty reads - but in this case it's much more important that no shared Locks are issued - so it doesn't prevent other transactions from modifying data read

Why on earth do you do this in the where clause?

cast (Visit_Likes.date as date) between @periodFrom and @periodTo

Just replace ist with:

Visit_Likes.date between @periodFrom and @periodTo


and I guearantee you it will be faster.

On top of that you can use (nolock) hint next to each table name in all of the FROM clauses. That will give you the isolation you want:

SELECT        'OverallWeekDay' AS vGroup, cast(visitTime  as nvarchar(10)) AS vType,  visitTime  as vDate, v_Visits.id AS vDetail
FROM            v_Visits (nolock)
WHERE        visitTime  between @periodFrom and @periodTo AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels

Why on earth do you do this in the where clause?

Visit_Likes.date is a datetime, and the procedure is fed with a date - but maybe theses a smart way to circumvent that :) ?

but maybe theses a smart way to circumvent that :) ?

It is if @periodFrom and @periodTo fills the dates from 00:00:00 to 23:59:59 so it will include all periods of the day.

Well, when   v_Visits is a view, then look at that first for optimizing by creating appropriate indices. But you may consider using an indexed view.

Have you tested WITH RECOMPILE?

I already wrote about the date problem... Why is a column named date of data type DATETIME??? Sad, so sad.

Visit_Likes.date is a datetime, and the procedure is fed with a date - but maybe theses a smart way to circumvent that :) ?

And what is to circumvent in that? :)

You can safely and with no implications compare a datetime to a date, interval or not, it will work just fine because the variables will be converted implicitly to datetimes, which are dates with 00:00:00.000 time component! You never convert the table column in the WHERE or JOIN clauses because it will prevent from using any possible indexes but it is completely safe and with no impact to convert the variables.

If your datetimes from the column have to include the values for the end of the interval date, they will be for the start date, then you will have to add a day to it and subtract 3 milliseconds but you will have to cast it to datetime first:


Visit_Likes.date between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime)))

Have you tested WITH RECOMPILE?

Yes, no effect as far as I can measure. Yes, pretty sad about the naming

I have now removed all of the cast'ing in the where clauses, also in the view, and have tried it with the "WITH RECOMPILE", and with "READ UNCOMMITTED", and it does repsond a bit better

The first select now returns, as an example "Jun  1 2017" from the cast in:
cast(visitTime  as nvarchar(11)) AS vType

how can I modify this so it returns a a normal "short date" - ie 01-06-2017 - if change that cast for "as date" it throws an error  when running the procedure ?

CREATE PROCEDURE [dbo].[getPeriodStatsAlternative]
	@periodFrom nvarchar(8),
	@periodTo nvarchar(8),
	@uRole as nvarchar(10),
	@channels as nvarchar(10) = '%'
WITH RECOMPILE
AS
BEGIN
	SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED
	BEGIN TRAN

		/****** Total ******/
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'OverallWeekDay' AS vGroup, cast(visitTime  as nvarchar(11)) AS vType,  visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM            v_VisitsAlternative
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Overall' AS vGroup, 'Visits' AS vType, visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM            v_VisitsAlternative
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Overall' AS vGroup, 'Pictures' AS vType, visitTime  as vDate, Visit_Pictures.id AS vDetail
		FROM             v_VisitsAlternative INNER JOIN Visit_Pictures ON v_VisitsAlternative.id = Visit_Pictures.visitId
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Overall' AS vGroup, 'Comments' AS vType, cast (Visit_Comments.date as date) as vDate, Visit_Comments.id AS vDetail
		FROM             v_VisitsAlternative INNER JOIN Visit_Comments ON v_VisitsAlternative.id = Visit_Comments.visitId
		WHERE        Visit_Comments.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Overall' AS vGroup, 'Likes' AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM             v_VisitsAlternative INNER JOIN Visit_Likes ON v_VisitsAlternative.id = Visit_Likes.visitId
		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		/****** Region ******/
		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType, visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM             (v_VisitsAlternative INNER JOIN Customers ON v_VisitsAlternative.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Region comments' AS vGroup, Districts.name AS vType, cast (Visit_Comments.date as date) as vDate, Visit_Comments.id AS vDetail
		FROM             Visit_Comments INNER JOIN ((v_VisitsAlternative INNER JOIN Customers ON v_VisitsAlternative.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id) ON Visit_Comments.visitId = v_VisitsAlternative.id
		WHERE        Visit_Comments.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Region likes' AS vGroup, Districts.name AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM             Districts INNER JOIN
								 Users INNER JOIN
								 v_VisitsAlternative INNER JOIN
								 Visit_Likes ON v_VisitsAlternative.id = Visit_Likes.visitId ON Users.id = Visit_Likes.userId ON Districts.id = Users.districtId

		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'Region liked' AS vGroup, Districts.name AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM             Visit_Likes INNER JOIN ((v_VisitsAlternative INNER JOIN Customers ON v_VisitsAlternative.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id) ON Visit_Likes.visitId = v_VisitsAlternative.id
		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType


		/****** Bruger ******/
		UNION ALL
		SELECT vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'User visits' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM              v_VisitsAlternative INNER JOIN Users ON v_VisitsAlternative.userId = Users.id
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'User comments' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, cast (Visit_Comments.date as date) as vDate, Visit_Comments.id AS vDetail
		FROM              v_VisitsAlternative INNER JOIN (Visit_Comments INNER JOIN Users ON Visit_Comments.userId = Users.id) ON v_VisitsAlternative.id = Visit_Comments.visitId
		WHERE        Visit_Comments.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'User likes' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM              (v_VisitsAlternative INNER JOIN Visit_Likes ON v_VisitsAlternative.id = Visit_Likes.visitId) INNER JOIN Users ON Visit_Likes.userId = Users.id
		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'User liked' AS vGroup, Users.firstName + ' ' + Users.lastName AS vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM              (v_VisitsAlternative INNER JOIN Visit_Likes ON v_VisitsAlternative.id = Visit_Likes.visitId) INNER JOIN Users ON v_VisitsAlternative.userId = Users.id
		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		/****** Brand ******/
		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType,visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM            v_VisitsAlternative INNER JOIN Visits_r_Products ON v_VisitsAlternative.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM(
		SELECT        'Brand comments' AS VGroup, Products.brandName as vType, cast (Visit_Comments.date as date) as vDate, Visit_Comments.id AS vDetail
		FROM		   v_VisitsAlternative INNER JOIN
								 Products INNER JOIN
								 Visits_r_Products ON Products.id = Visits_r_Products.productId ON v_VisitsAlternative.id = Visits_r_Products.visitId INNER JOIN
								 Visit_Comments ON v_VisitsAlternative.id = Visit_Comments.visitId
		WHERE        Visit_Comments.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole  AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		UNION ALL
		SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM(
		SELECT        'Brand likes' AS VGroup, Products.brandName as vType, cast (Visit_Likes.date as date) as vDate, Visit_Likes.visitId AS vDetail
		FROM           v_VisitsAlternative INNER JOIN
								 Visit_Likes ON v_VisitsAlternative.id = Visit_Likes.visitId INNER JOIN
								 Products INNER JOIN
								 Visits_r_Products ON Products.id = Visits_r_Products.productId ON v_VisitsAlternative.id = Visits_r_Products.visitId
		WHERE        Visit_Likes.date between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole  AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		COMMIT TRAN
END

Select allOpen in new window

You have to use CONVERT not just CAST when you want to get adate in a certain format. Use this:

convert(varchar(10), visitTime, 105)

That 105 will determine the DD-MM-YYYY format. Here is the list of formats:

https://docs.microsoft.com/en-us/sql/t-sql/functions/cast-and-convert-transact-sql

Actually that convert is useless because in your upper query you don't return any date, you only return:

vGroup, vType, count(vDetail) AS vCount

In which case all the other columns from the inside queries are useless! In the inner queries you should only return the columns that are relevant in the outer final query.

Another issue is that for a query that doesn't do insert/updates/deletes you don't need transaction so remove the BEGIN TRAN and COMMIT TRAN lines.

Well it doesn't appear useless at all - it now returns the correct formatted records form the first Select :)

remove the BEGIN TRAN and COMMIT TRAN lines.

- the READ UNCOMMITTED will still be effective if those are removed ?

Let's get the first query:

SELECT	vGroup, vType, count(vDetail) AS vCount
		FROM (
		SELECT        'OverallWeekDay' AS vGroup, cast(visitTime  as nvarchar(11)) AS vType,  visitTime  as vDate, v_VisitsAlternative.id AS vDetail
		FROM            v_VisitsAlternative
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

Select allOpen in new window

In this query the   visitTime  as vDate and  v_VisitsAlternative.id AS vDetail are completely useless. If you remove them you will get the exactly same results for this first query:

		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'OverallWeekDay' AS vGroup, cast(visitTime  as nvarchar(11)) AS vType
		FROM            v_VisitsAlternative
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

Select allOpen in new window

I will repeat, you don't need the transaction for a select. The 2 issues are not related, the BEGIN...COMMIT TRAN with the SET ISOLATION LEVEL...

When you count use * instead of column name! If you use the column name the NULL values will not be counted, if there are NULL values in that column!

Why are the date variables of type nvarchar(8)? Change that to date type and you forgot to convert the @periodTo to include all times for that date:

... between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime)))

btw,you don't need the sub queries at all, but this has no performance impact besides a small compilation time plus..

e,g,

SELECT  vGroup ,
        vType ,
        COUNT(vDetail) AS vCount
FROM    ( SELECT    'OverallWeekDay' AS vGroup ,
                    CAST(visitTime AS NVARCHAR(10)) AS vType ,
                    visitTime AS vDate ,
                    v_Visits.id AS vDetail
          FROM      v_Visits
          WHERE     visitTime BETWEEN @periodFrom AND @periodTo
                    AND v_Visits.userRole LIKE @uRole
                    AND v_Visits.channelName LIKE @channels
        ) resSub
GROUP BY vGroup ,
        vType

-- is equivalent to
SELECT  'OverallWeekDay' AS vGroup ,
        CAST(visitTime AS NVARCHAR(10)) AS vType ,
        COUNT(vDetail) AS vCount
FROM    v_Visits
WHERE   visitTime BETWEEN @periodFrom AND @periodTo
        AND v_Visits.userRole LIKE @uRole
        AND v_Visits.channelName LIKE @channels
GROUP BY CAST(visitTime AS NVARCHAR(10))

Select allOpen in new window

In this query the   visitTime  as vDate and  v_VisitsAlternative.id AS vDetail are completely useless

Thank you - yes I have removed "visitTime  as vDate" now - the vDetail is needed (it will throw an error otherwise)

Why are the date variables of type nvarchar

if I use Date instead of nvarchar(10) it will throw an error

No, it is not needed and it will not throw any error if you use

count(*) AS vCount

instead of count(vDetail) AS vCount

You missed one of my post.

,you don't need the sub queries at all,

thnaks - I'll look into that, and see how much I can mess it up :)

if I use Date instead of nvarchar(10) it will throw an error

What error? You must be doing something wrong! You should avoid passing dates as varchars in a stored procedure.

No, it is not needed and it will not throw any error if you use

sorry about that - but it throws an error if it looks like this - I'm probably missing out some more .. :) ? The error shows when calling the procedure

The rest of the data for the vDetail column for the rest of the Unions are all varchar's - it's only dates in the first select :)

		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'OverallWeekDay' AS vGroup, convert(date,visitTime,105) AS vType,  v_VisitsAlternative.id AS vDetail
		FROM            v_VisitsAlternative
		WHERE        visitTime  between @periodFrom and @periodTo AND v_VisitsAlternative.userRole like @uRole AND v_VisitsAlternative.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

Select allOpen in new window

You will have to remove all those "vDetail" columns from all your inner queries and the select of the outer queries you will have to replace everywhere with:

SELECT      vGroup, vType, count(*) AS vCount

Zberteoc

Wow ! - thank you - it Works fine, but doesn't appear faster ? The total vCount is now 59936, compared to previously 57911 - what Nulls are getting counted in, in your version ?

and didn't you say that cast in  the where was a no go :) ? But thats probably where the difference lies in the vCount, as it includes another day

In the first inner query you actually need:

 convert(varchar(10), visitTime, 105) AS vType

to get the DD-MM-YYYY format.

Yes, in your version you missed rows form the @periodTo date, which are now included in "my" version. Also, the count(*) will include ALL the rows from each inner query regardless if the vDetail values are NULL or not.

The CAST is only an issue if it is applied to a COLUMN in the where clause and not to a variable! In this case has no impact to performance. Beside that you HAVE to use it because you want to include all the values for that date otherwise the rows with a time > 3 miliseconds after midnight will NOT be included, which would be wrong!

Thank you for all your input - but I've just done some Measurements comparing this latest iteration to the first - and the first took 3.2 sec, and the latest takes 4.3 ... ?

That is nothing and if you run it again will probably be faster. Regardless, not including the rows for the @periodTo date would be wrong so you HAVE to turn a date like

YYYY-MM-DD
to
YYYY-MM-DD 23:59:59.997

and that is what the CAST does!

Also make sure you have the right indexes on your tables. But that is another story...

Thanks - It does return pretty consistent test times, but it's certainly cleaned up a lot now :) Thank you very much

It is no use if it takes 0 time but with wrong results, is it? :) Anyway, I am sure that it could be improved even more by tweaking the code a bit and also some indexing but it should be fine for now.