SQL Server query - where to put Order By

right after line 18

Thanks mike - now I have the right order  :)

SWEATY
FEET
WILL
GIVE
HORRIBLE
ODORS

SELECT (with TOP, DISTINCT)
FROM (with JOINs)
WHERE
GROUP BY
HAVING
ORDER BY

Good luck getting that our of your head.

For TOP 5, notice that there are no ORDER BY clauses, which means it's going to pick the first five in the table.  If you want the 'first 5 x', then you have to include an 'ORDER BY x' in the query.

/****** Region ******/		
SELECT	vGroup, vType, count(*) AS vCount
FROM (
	SELECT TOP 5 'Region visits' AS vGroup, Districts.name AS vType
	FROM  v_Visits 
		JOIN Customers ON v_Visits.customerId = Customers.id) 
		JOIN Districts ON Customers.districtId = Districts.id
	WHERE  visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType

/****** Brand ******/
UNION ALL
SELECT	vGroup, vType, count(*) AS vCount
FROM (
	SELECT  TOP 5  'Brand visits' AS VGroup, Products.brandName as vType
	FROM v_Visits 
		INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId 
		INNER JOIN Products ON Visits_r_Products.productId = Products.id
	WHERE visitTime between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
	) resSub
GROUP BY  vGroup, vType

Select allOpen in new window

Noticed that I cleaned up your code using the tab character to make it much easier to read.

Also as an aside I recommend against using BETWEEN, especially if you're doing the 'knock three milliseconds off the date' trick. Check out Beware of Between for why.

Instead, say if @periodFrom = '20170101' and @periodTo is 1/5, use '20170106' instead and do this..

WHERE visitTime >= @periodFrom  AND visitTime < @periodTo

Select allOpen in new window

Good luck getting that our of your head

That'll be a tough one :) Actually I know I just accepted a solution, but it's actually not giving me the wanted result - shoudl I then open another question, or do one do that ?

In that case I'd unaccept the answer (or Request Attention and ask if you can't do that yourself), then continue to work the problem with all experts that provide meaningful comments, then re-grade when you're good.

Thanks Stefan - I'll see if I can make that code Work in my SP

A TOP per GROUP (BY) requires ROW_NUMBER() OVER ( PARTITION BY groupByColumns ORDER BY topOrderColumns ASC ). E.g.

Hi Stefan - I'm struggling a bit to make your SQL Work - what needs to be put in front of your whole WITH statement - sorry for the maybe very Noob question :) ?

For consideration by all:

At ID: 42188621, Bojerne, in response to my solution, writes: Thanks mike - now I have the right order  :)
An then he gives me assist points.
(EE notif on 2017-06-22 05:54 AM reads "Bojerne has accepted your comment as an assisted solution to: SQL Server query - where to put Order By")

My solution was the first comment and he seemed happy with it (see ID: 42188621). I wonder why he then gives me assist points instead of just accepting it as the solution.

@Bojerne, you say it worked for you. Then accept it and open a new question if you have a new question. Please explain.

Thank you,

Mike

Hi Mike

It's because I afterwards realised that although I had the right order, I didn't have the Top5 :) - which was part of the question. There was nothing wrong with your answer - it was just "partial" (don't know if thats the correct English term)

Top 5 vCount for/in each vGroup in this query

I am studying for exam 70-461 as we speak. Now, I am doing offst-fetch which maybe applicable here.

What version of SQL Server are you working with?

Azure - I would assume thats 2016 ?

After line 18, include
ORDER BY ... (include the column names you want)
  OFFSET 0 ROWS
  FETCH FIRST 5 ROWS ONLY

What you want to be done if row number 5 and 6 are exactly the same?

If you add more terms to your order by clause, the will break the tie. The other option is to add WITH TIES to order by to force the result to display top 4 plus as many ties there is.

Here is example where it shows 7 rows (= top 4 + 3  ties)
1
2
3
4
5
5
5

Hi Mike - it sort of works .) - but it only Works on the bottom SELECT Group - I don't get any results from the first Group - I need the Top 5 from both Groups :)

The code now looks like this

		/****** Region ******/		
		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType

		/****** Brand ******/
		UNION ALL
		SELECT	vGroup, vType, count(*) AS vCount
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType
		
		ORDER BY vGroup, vCount desc
		OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

Use ROW_NUMBER()..

try:

;with cte1 as (	
		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
		--GROUP BY  vGroup, vType
cte2 as (
		/****** Brand ******/
		--UNION ALL
		SELECT	vGroup, vType, count(*) AS vCount
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub)
Select vGroup, vType, vCount from cte1 UNION ALL
Select vGroup, vType, vCount from cte2
		GROUP BY  vGroup, vType
		
		ORDER BY vGroup, vCount desc
		OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

Hi Mike,

sorry to bother you, but its throwing this when I put your code into the Create Procedure framework:

"Column 'resSub.vGroup' is invalid in the select list because it is not contained in either an aggregate function or the GROUP BY clause"

I added Group By to cte1 and cte2 as you had it befole. Please try this:

;with cte1 as (	
		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
		GROUP BY  vGroup, vType
cte2 as (
		/****** Brand ******/
		--UNION ALL
		SELECT	vGroup, vType, count(*) AS vCount
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub
		GROUP BY  vGroup, vType)
Select vGroup, vType, vCount from cte1 UNION ALL
Select vGroup, vType, vCount from cte2
		GROUP BY  vGroup, vType, vCount
		
		ORDER BY vGroup, vCount desc
		OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

Note: Line 21 is changed: GROUP BY  vGroup, vType, vCount

Not quite there yet :) ?

I hope this is the last revision:

		/****** Region ******/	
;with cte1 as (	
		SELECT	vGroup, vType, count(*) AS vCount
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
		--GROUP BY  vGroup, vType
cte2 as (
		/****** Brand ******/
		--UNION ALL
		SELECT	vGroup, vType, count(*) AS vCount
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub)
		--GROUP BY  vGroup, vType)
Select vGroup, vType, vCount from cte1 UNION ALL
Select vGroup, vType, vCount from cte2
		GROUP BY  vGroup, vType, vCount
		
		ORDER BY vGroup, vCount desc
		OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

Nope - but thank you for your effort :)

@ste5an

Could you please, using row_number(), prepare the solution for Bojerne to try. Obliviously, he is willing to try it. I have my mind set in the cte solution which eventually I will learn to do it but for now we have to take care of what Bojerne needs. Without some test data, it is challenging for me to do the solutions because I am not as skilled as you are.

Mike

I did .. #42188642.

@Bojerne,

This morning I was in rush and had to leave for an appointment. I am sure the solution from ste5an will be a good one if you want to give a try. Meanwhile for my own peace of mind, I tried a solution similar to yours on a different database. The code below, although has not much of use for you, but shows the structure of the work which will be helpful for your case.
1. I will show the sample structure. (tested)
2, I will show the bare-bone  outline.
3. I will put your code in this structure for you to try.
----------------------------
1. Tested Sample Structure.

;with cte1 as
(SELECT productid ,[unitprice] FROM [TSQL2012].[Sales].[OrderDetails]
Where productid<=7),
cte2 as
(SELECT productid ,[unitprice] FROM [TSQL2012].[Sales].[OrderDetails]
Where productid>7),
cte3 as
(Select productid,[unitprice] from cte1
union all
Select productid,[unitprice] from cte2)
Select productid,[unitprice], count(*) vCount from cte3
Group By productid, [unitprice]
order by [unitprice], productid
OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

2, Bare-bone  Outline.

;with cte1 as
(SELECT...),
cte2 as
(SELECT...),
cte3 as       -- combine cte1 and cte2
(Select ... from cte1
union all
Select ... from cte2)
Select ..., count(*) vCount from cte3     -- add count(*), group by, order by, and OFFSET...FETCH
Group By ...
order by ...
OFFSET 0 ROWS
            FETCH FIRST 5 ROWS ONLY

I will be back with "3.Your code in this structure for you to try." for your use shortly.

Mike

And here is all put together.

3.Your code in this structure for you to try.

;with cte1 as 
		(SELECT	vGroup, vType
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
cte2 as
		(SELECT	vGroup, vType
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
cte3 as
(Select vGroup, vType from cte1
union all
Select vGroup, vType from cte2)
Select vGroup, vType, count(*) vCount from cte3
Group By vGroup, vType
ORDER BY vGroup, vCount desc
OFFSET 0 ROWS 
		FETCH FIRST 5 ROWS ONLY

Select allOpen in new window

Thank you for your patience.

Mike

Hi Mike,

It's me who's thanking you for your patience :) The procedure created with no errors now, but it's still returning only results from the second Group - none from the first

Ok, below code outputs combined record without grouping, ordering, or giving top 5. The idea here is to see if UNION ALL works OK. You could then put it in excel sort, order, etc. so that you are able to get what you are looking for. The reason for this process is:
1. To make sure UNION ALL works ok.
2. If it does, then it is possible our code the way that it groups, orders, etc. in fact correct.
3. Base on the outcomes from 1 and 2 above, we may need to do additional changes to the submitted solution.

Thx,

Mike

;with cte1 as 
		(SELECT	vGroup, vType
		FROM (
		SELECT        'Region visits' AS vGroup, Districts.name AS vType
		FROM             (v_Visits INNER JOIN Customers ON v_Visits.customerId = Customers.id) INNER JOIN Districts ON Customers.districtId = Districts.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
cte2 as
		(SELECT	vGroup, vType
		FROM(
		SELECT        'Brand visits' AS VGroup, Products.brandName as vType
		FROM            v_Visits INNER JOIN Visits_r_Products ON v_Visits.id = Visits_r_Products.visitId INNER JOIN Products ON Visits_r_Products.productId = Products.id
		WHERE        visitTime  between @periodFrom and dateadd(ms, -3, dateadd(day, 1, cast(@periodTo as datetime))) AND v_Visits.userRole like @uRole AND v_Visits.channelName like @channels
		) resSub),
cte3 as
(Select vGroup, vType from cte1
union all
Select vGroup, vType from cte2)
Select * from from cte3

Select allOpen in new window

Hi Mike

The union all Works fine in the code in my initial question :) The code you just put up throws this error :

At line 19, remove one of the froms

Select * from from cte3

It is late here, past 12 pm.

Fully understandable - and it's too early here for me to have noticed that :) It runs now, and the union Works fine - both Groups are represented -  except that the vCount results are not included in either of the groups

I kept count(*) to do after UNION ALL. It is too late now, but I will get up early morning and complete the rest past UNION ALL.

Thanks Mike - most appreciated

Hi Mike,
thank you for your effort - it has been most appreciated. Your latest iteration throws the error shown below. Spurred on by your comment I had another look at Ste5ans code, and realised the reason it didn't Work in the first place was becasue it was missing the ";" in front of the WITH... - I have now restested Ste5ans code it Works just as wanted ! So I'll close the question with you  as the assisted solution, and Ste5an as the Best solution - I hope you'll agree :) Have a nice day

it was missing the ";" in front of the WITH...

Nope. It was not working because you have not terminated the last SQL statement before the WITH correctly. Thus always use the semi-colon at the end of a statement. Which is btw good coding style.

@Bojerne,

I am glad you managed to have a working solution. BTW, line 44 ORDER BY is sort of over kill and could be removed. You could remove it because the line 47 has the necessary order by.

Good luck with your project.

Mike

Thank you both Mike and St5fan for getting me through this

@Bojerne,

I wonder if you could do me a favor. If possible at all, where it is showing an error (see ID: 42195509) at
.
.
line 20:  (Select vGroup, vType from cte3
line 21: ORDER BY vGroup, vType)    ---<-- errors here
line 22: Select vGroup, vType, Count(*) as vCount From cte4
.
.
remove Order By vGroup, vType  
and transfer the closing ) to line above, the final code will be:
.
.
line 20:  (Select vGroup, vType from cte3)  --<-- add closing )
line 21: --ORDER BY vGroup, vType)
line 22: Select vGroup, vType, Count(*) as vCount From cte4
.
.
Try the solution one more time to is if it works okay. I know you have accepted and closed this question. Knowing this revised code works or not will boost my level of confidence going to take my test in a few weeks.

Thank you,

Mike