SQL Query with where clause

RIAS
RIAS used Ask the Experts™
on
Hi,
This query works if the ISAdmin and Ispoweruser is false but fails when ISAdmin or IspOwerUser =True .
ALTER PROCEDURE [dbo].[RecentJobs_St] (

  @SelectedDateTime			DATETIME
, @AccountName				NVARCHAR(200)
)
AS
BEGIN

SET NOCOUNT ON 

DECLARE @SQL	NVARCHAR(2000)
DECLARE @IsPowerUser		BIT
DECLARE @IsAdmin			BIT
DECLARE @UserID				INT

-- This query gets a UserID if the user is a power user or an admin
-- If they are then they can see all records, otherwise just their own.

SELECT @UserID = ID
, @IsAdmin = IsAdmin
, @IsPowerUser = IsPowerUser
FROM Users U
WHERE U.AccountName = @AccountName

IF @IsAdmin IS NULL
  SET @IsAdmin = 0
IF @IsPowerUser IS NULL
  SET @IsPowerUser = 0 

SET @SQL = 'SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= ' + CONVERT(NVARCHAR(20),@SelectedDateTime,112) + ' '

IF NOT (@IsPowerUser=1 OR @IsAdmin=1)
  SET @SQL = @SQL + ' AND UserID IN (SELECT ID FROM USERS WHERE UserGroupName = ''XYZ'')'

SET @SQL = @SQL + ' ORDER BY CAST([Date Time] AS datetime) DESC '

Open in new window

Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
ste5anSenior Developer

Commented:
First of all: Why do you use dynamic SQL??
Then: I guess you forgot an @ for the UserID.

ALTER PROCEDURE [dbo].[RecentJobs_St] (
    @SelectedDateTime DATETIME,
    @AccountName NVARCHAR(200)
)
AS
    SET NOCOUNT ON;

    DECLARE @IsPowerUser BIT;
    DECLARE @IsAdmin BIT;
    DECLARE @UserID INT;

    -- Requires that AccountName has an UNIQUE constraint.
    SELECT  @UserID = ID,
            @IsAdmin = ISNULL(IsAdmin, 0),
            @IsPowerUser = ISNULL(IsPowerUser, 0)
    FROM    Users U
    WHERE   U.AccountName = @AccountName;

    SELECT TOP (50) *
    FROM JOB_Recnt R
    WHERE R.[Date Time] <=  @SelectedDateTime
        AND (
            @UserID IN (SELECT ID FROM USERS WHERE UserGroupName = 'XYZ')
            OR @IsAdmin = 1
            OR @IsPowerUser = 1
        )
    ORDER BY [Date Time]  DESC;

Open in new window



And last, but not least: Why can be IsAdmin or IsPowerUser NULL? This is a design flaw..
Paulo LeitaoGlobal Hardware Manager

Commented:
What is the error that returns when it fails?
ste5anSenior Developer

Commented:
p.s. instead of

@UserID IN (SELECT ID FROM USERS WHERE UserGroupName = 'XYZ')

Open in new window

use

EXISTS (SELECT *  FROM USERS WHERE UserGroupName = 'XYZ' AND ID = @UserID)

Open in new window

Should you be charging more for IT Services?

Do you wonder if your IT business is truly profitable or if you should raise your prices? Learn how to calculate your overhead burden using our free interactive tool and use it to determine the right price for your IT services. Start calculating Now!

SQL Server DBA & Architect, EE Solution Guide
Awarded 2009
Distinguished Expert 2018
Commented:
Replace Line#34 and 35 with this..
SET @SQL = CASE WHEN (@IsPowerUser=1 OR @IsAdmin=1) THEN @SQL ELSE @SQL + ' AND UserID IN (SELECT ID FROM USERS WHERE UserGroupName = ''XYZ'')' END

Open in new window

Most Valuable Expert 2012
Distinguished Expert 2018

Commented:
First, I'm not a SQL Sever person.  I just stumbled in here.

I'm not seeing the need for all the variables and if statements .  It can all be done in a single SQL statement.

You don't have an END to your BEGIN so I'm not sure how it all goes together and I'm not sure about proper syntax on the wildcard check.

All that said, I hope this gives you a basic idea of what I'm trying to show.

ALTER PROCEDURE [dbo].[RecentJobs_St] (

  @SelectedDateTime			DATETIME
, @AccountName				NVARCHAR(200)
)
AS
BEGIN

SET NOCOUNT ON 

DECLARE @SQL	NVARCHAR(2000)
DECLARE @IsPowerUser		BIT
DECLARE @IsAdmin			BIT
DECLARE @UserID				INT

-- This query gets a UserID if the user is a power user or an admin
-- If they are then they can see all records, otherwise just their own.

SET @SQL = 'SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= ' + CONVERT(NVARCHAR(20),@SelectedDateTime,112) + ' ' +
	'1 = ( ' +
	'SELECT 1 FROM USERS WHERE userid=@AccountName and userid like case when ispower=1 or isadmin=1 then "%" else userid end  ' +
	') ' +
	'or userid = @AccountName ' +
	' ORDER BY CAST([Date Time] AS datetime) DESC '

Open in new window

Most Valuable Expert 2012
Distinguished Expert 2018

Commented:
WOW, I should have refreshed before posting!!!!!!!

Same basic idea as above.  I'll go back to Oracle now!     ;)

Author

Commented:
Thanks Experts! Will try and brb
Mark WillsTopic Advisor, Page Editor
Distinguished Expert 2018

Commented:
I am also wondering what "it fails" means

Couple of questions...

1) what datatype is R.[Date Time] ?
2) is UserID a column in JOB_Recnt ?
3) Do you really want / need dynamic SQL ?
4) If R.[Date Time] is a character based column, what  is the string format  - hopefully 'yyyymmdd' ?

There is an error in encapsulating @selectedDateTime in single quotes.

There is a technical problem converting R.[Date Time] to nvarchar in so much as you are better off casting as date (similar to your order by)
And if it is a date based datatype, then no need to convert in your order by - which makes me think it is not date based, but a string ?

casting as a date keeps the underlying datatype as a date rather than transforming to a character string.

Better still, is dont touch it (assuming it is some kind of date based datatype) and use

where r.[date time] < cast(dateadd(d,1,@selecteddatetime) as date

that way you will be 1 day greater (in selecteddatetime) to make sure you include all hours / minutes / seconds on that last day of r.[date time]

Also, rather than : and userid in (SELECT ID FROM USERS ...)
Better to use : and EXISTS (SELECT NULL FROM Users ..... where ID = USERID)
The EXISTS will exit when it finds an instance - so dont have to select all the ID's
and we select NULL (or 1 or whatever) because we dont want a result set as such, just check if it exists.


But depending on your answers above, I think below is a better starting point by encapsulating the selecteddatetime equation in single quotes.

SET @SQL = 'SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= ''' + CONVERT(NVARCHAR(20),@SelectedDateTime,112) + ''' '

IF  (@IsPowerUser=1 OR @IsAdmin=1)
  SET @SQL = @SQL + ' AND UserID IN (SELECT ID FROM USERS WHERE UserGroupName = ''XYZ'')'

SET @SQL = @SQL + ' ORDER BY CAST(R.[Date Time] AS datetime) DESC '

-- and when doing Dynamic SQL, always do a PRINT @SQL

print @sql

/*
-- here are the results of either condition :

SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= '20180616'  AND UserID IN (SELECT ID FROM USERS WHERE UserGroupName = 'XYZ') ORDER BY CAST(R.[Date Time] AS datetime) DESC 

SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= '20180616'  ORDER BY CAST(R.[Date Time] AS datetime) DESC

*/

Open in new window

Author

Commented:
Thanks Experts ! Spot on very helpful comments!
ste5anSenior Developer

Commented:
You haven't told us, why you need dynamic SQL.. This is important.
Paulo LeitaoGlobal Hardware Manager

Commented:
He didn't told us what the error is !!!
I'm assuming that for safety reasons he is placing only half script and hence why, we don't understand the need for dynamic SQL.
The error code will help to understand what is the reason of the failure.
ste5anSenior Developer

Commented:
Even then, there is no need for SQL concatenation, cause the 2 queries can be written as one:


ALTER PROCEDURE [dbo].[RecentJobs_St] (
    @SelectedDateTime DATETIME,
    @AccountName NVARCHAR(200)
)
AS
    SET NOCOUNT ON;

    -- Requires that AccountName has an UNIQUE constraint, otherwise the result is arbitrary.
    WITH Account AS
        (
            SELECT  TOP (1) ID AS UserID,
                    ISNULL(IsAdmin, 0) AS IsAdmin,
                    ISNULL(IsPowerUser, 0) AS IsPowerUser
            FROM    Users U
            WHERE   U.AccountName = @AccountName
        )
    SELECT TOP (50) R.*
    FROM JOB_Recnt R
        CROSS JOIN Account A
    WHERE R.[Date Time] <=  @SelectedDateTime
        AND (
            EXISTS (SELECT * FROM Users U WHERE U.UserGroupName = 'XYZ' AND U.ID = A.UserID)
            OR A.IsAdmin = 1
            OR A.IsPowerUser = 1
        )
    ORDER BY R.[Date Time] DESC;

Open in new window

or

ALTER PROCEDURE [dbo].[RecentJobs_St] (
    @SelectedDateTime DATETIME,
    @AccountName NVARCHAR(200)
)
AS
    SET NOCOUNT ON;

    -- Requires that AccountName has an UNIQUE constraint, otherwise the result is arbitrary.
    DECLARE @Statment NVARCHAR(MAX) = N'
        WITH Account AS
            (
                SELECT  TOP (1) ID AS UserID,
                        ISNULL(IsAdmin, 0) AS IsAdmin,
                        ISNULL(IsPowerUser, 0) AS IsPowerUser
                FROM    Users U
                WHERE   U.AccountName = ''' + REPLACE(@AccountName, '''', '''''') + '''
            )
        SELECT TOP (50) R.*
        FROM JOB_Recnt R
            CROSS JOIN Account A
        WHERE R.[Date Time] <= ''' + CONVERT(NVARCHAR(255), @SelectedDateTime, 126) + '''
            AND (
                EXISTS (SELECT * FROM Users U WHERE U.UserGroupName = ''XYZ'' AND U.ID = A.UserID)
                OR A.IsAdmin = 1
                OR A.IsPowerUser = 1
            )
        ORDER BY R.[Date Time] DESC;
    ';

Open in new window


He didn't told us what the error is !!!
He clearly wants to filter for @UserID according to the used variables. But the "final" statement doesn't use it. Thus the guess where it normally should be placed. Another guess could be:

ALTER PROCEDURE [dbo].[RecentJobs_St] (
    @SelectedDateTime DATETIME,
    @AccountName NVARCHAR(200)
)
AS
    SET NOCOUNT ON;

    WITH Account AS
        (
            SELECT  TOP (1) ID AS UserID,
                    ISNULL(IsAdmin, 0) AS IsAdmin,
                    ISNULL(IsPowerUser, 0) AS IsPowerUser
            FROM    Users U
            WHERE   U.AccountName = @AccountName
        )
    SELECT TOP (50) R.*
    FROM JOB_Recnt R
        CROSS JOIN Account A
    WHERE R.[Date Time] <= @SelectedDateTime
        AND (
            R.UserID = A.UserID)
            OR A.IsAdmin = 1
            OR A.IsPowerUser = 1
        )
    ORDER BY R.[Date Time] DESC;

-- or

ALTER PROCEDURE [dbo].[RecentJobs_St] (
    @SelectedDateTime DATETIME,
    @AccountName NVARCHAR(200)
)
AS
    SET NOCOUNT ON;

    -- Requires that AccountName has an UNIQUE constraint, otherwise the result is arbitrary.
    DECLARE @Statment NVARCHAR(MAX) = N'
        WITH Account AS
            (
                SELECT  TOP (1) ID AS UserID,
                        ISNULL(IsAdmin, 0) AS IsAdmin,
                        ISNULL(IsPowerUser, 0) AS IsPowerUser
                FROM    Users U
                WHERE   U.AccountName = ''' + REPLACE(@AccountName, '''', '''''') + '''
            )
        SELECT TOP (50) R.*
        FROM JOB_Recnt R
            CROSS JOIN Account A
        WHERE R.[Date Time] <= ''' + CONVERT(NVARCHAR(255), @SelectedDateTime, 126) + '''
            AND (
                R.UserID = A.UserID)
                OR A.IsAdmin = 1
                OR A.IsPowerUser = 1
            )
        ORDER BY R.[Date Time] DESC;
    ';

Open in new window

Paulo LeitaoGlobal Hardware Manager

Commented:
ste5an, I know what peace of the script is creating the failing, he was clear on that.
What we don't know is what the output error is.
He only says it fails, what does that means !!! Is there an value that can't be converted? Or is a parsing error? Or invalid comparison, or it just doesn't get the expected data?

I'm in holidays and following in the mobile and may had loss something, but if we had the error code or a more specific on the fail, will not jumping into possible solutions based on guessing, just the way you are doing...otherwise what's the point on having output errors?

Nevertheless, I'm happy that he solved his issue by using above answers.
Saludos
Mark WillsTopic Advisor, Page Editor
Distinguished Expert 2018

Commented:
*laughing*, I like the way one asks questions, indicating "a starting point" and get no answers :)

Always the same with RIAS

And checking, I am a bit surprised, because it seems a duplicate of https://www.experts-exchange.com/questions/29104797/SQL-query-with-a-condtion.html?cid=1752

Author

Commented:
Sorry Experts, looks like there are lot of queries. Yes, the issue is resolved now as I had mentioned the query was not getting in the if condition.
The Answer which I accepted did the trick. Please let me know if you need more explanation.

Thanks for all your support!

Author

Commented:
Mark you are absolutely correct , it is in reference with the link you mentioned. But, it was the second part of the query so a new question makes more sense .
Mark WillsTopic Advisor, Page Editor
Distinguished Expert 2018

Commented:
RIAS,

When I ran the query as you already had it, I posted the outputs of : IF (@IsPowerUser=1 OR @IsAdmin=1)

SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= '20180616'  AND UserID IN (SELECT ID FROM USERS WHERE UserGroupName = 'XYZ') ORDER BY CAST(R.[Date Time] AS datetime) DESC 

SELECT TOP 50 * 
FROM JOB_Recnt R
WHERE CONVERT(NVARCHAR(20),R.[Date Time],112) <= '20180616'  ORDER BY CAST(R.[Date Time] AS datetime) DESC

Open in new window


So, what was the error you were getting whereby a CASE made the difference ?

It is obvious now that you were getting the additional string ( 'and userid ...') being created when @IsAdmin <> 1, or, @IsPowerUser<>1
which is a problem when using NOT with OR

Had you clarified to begin with (ie creating the additional SQL string) , rather than the generic "it fails" then it would have solved a lot of effort and questions and guessing.

There were quite a few questions being asked, but you didnt really respond to them directly. So, it becomes difficult for the Experts to provide you with potential solutions, instead, having to guess what the problem might be.

Author

Commented:
Mark,
Thanks for getting back. I really don't know what went wrong, the solution i have accepted did work.
Also by the time I tried the solution lots of comments and posts were present.

In future I will try to give more explanation if the reason is known to me.
Hope it is fine with you and apologies for nay convenience caused.

Thanks
ste5anSenior Developer

Commented:
Just a further thought:

Your starting example is incomplete. Cause it raises further questions, like why those CONVERTs or why dynamic SQL. Thus it's hard to give a correct answer.
And even now, I don't know why the marked answer is correct. Just keep in mind just "working" is not the same as "correct".

The additional benefit of a forum is that people can find correct answers.

Just my 2¢.

Author

Commented:
Ste5an,
Thanks. You are correct, there can be multiple correct answers. But, this browser allows me to accept only 1 answer. If you let me know I can test all the queries which are correct/working and can mark them as assisted.
Thanks

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial