current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I have two riscProfileTypeIds (both of type uniqueidentifier) of which I need the one with the highest sequence number (the highest of the two risc profiles, at business level). The below query does the job, but I'm not very happy with the union, I wonder if there's a way to do the same without it.

SELECT [ID]
FROM [RiscProfileType]
WHERE 1 = 1
AND [Active] = 1
AND [Sequence] = (
    SELECT MAX([Sequence])
    FROM (
        SELECT [Sequence]
        FROM [RiscProfileType]
        WHERE 1 = 1
        AND [Active] = 1
        AND [ID] = @riscProfileTypeId1
        UNION
        SELECT [Sequence]
        FROM [RiscProfileType]
        WHERE 1 = 1
        AND [Active] = 1
        AND [ID] = @riscProfileTypeId2
    ) src
)

The [RiscProfileType] table is very small (4 records), so performance isn't much of an issue but still I prefer to write the best query possible.

Put simply it contains:

[Risc]          [Sequence]
Unacceptable    4
High            3
Normal          2
Low             1

I don't think a join is possible, because there's no real link between the entities.

Any suggestions on improving this query, or is this fine?

share|improve this question
    
Is this code review question or stackoverflow? – Think2ceCode1ce Nov 27 '15 at 9:48
    
Meaning what exactly? – Spikee Nov 27 '15 at 9:50
    
I mean should not this fit better to be asked on Stackoverflow instead of codereview site! I am just trying to understand. – Think2ceCode1ce Nov 27 '15 at 9:54
    
I'm not asking for help on achieving something, I'm looking for a better way to do what I already have. That's the definition of a code review, imho. – Spikee Nov 27 '15 at 9:56
    
Cool. Got it. I got lazy to observe that difference. Thanks. – Think2ceCode1ce Nov 27 '15 at 9:57
up vote 2 down vote accepted

Why don't you use IN instead of the JOIN?

SELECT MAX([Sequence])
FROM [RiscProfileType]
WHERE 1 = 1
   AND [Active] = 1
   AND [ID] IN (@riscProfileTypeId1, @riscProfileTypeId2)

Or rethink the logic:

SELECT [ID]
FROM [RiscProfileType]
WHERE 1 = 1
   AND [Active] = 1
   AND [ID] IN (@riscProfileTypeId1, @riscProfileTypeId2)
ORDER BY [Sequence] DESC 
LIMIT 1
share|improve this answer
    
Good call, that works! – Spikee Nov 27 '15 at 9:48
    
I prefer the former of the two, looks cleaner, also (might be incorrect on this, but) think it's (fractionally) faster than the latter. – Spikee Nov 27 '15 at 10:04
    
I checked, if there's a clustered index the performance difference is theoretical (identical in human experience), for a non-clustered index Max beats order by limit (or top). – Spikee Nov 27 '15 at 10:16
up vote 1 down vote

The above answer is correct but you can also use top instead of limit if you are using SQL Server for this case. we do not need 1=1 in where clause. Query will look like:

SELECT TOP 1 [ID]
FROM [RiscProfileType]
WHERE [Active] = 1
   AND [ID] IN (@riscProfileTypeId1, @riscProfileTypeId2)
ORDER BY [Sequence] DESC
share|improve this answer
    
The where 1 = 1 is a cosmetic trick to prevent copy paste headaches. Granted, it's silly for small queries, but once you start using it, you'll regret it that one time you didn't (laziness is your friend). – Spikee Dec 2 '15 at 12:48

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.