Stored procedure situation

[sngapoonage]

Hi everyone,

I have a stored procedure which takes 2 integer parameters and returns either a 1 or a 0. It's quite long so I won't post it here unless you feel it's nessesary.

Currently I have a table named 'Books', a table named 'Filters'. Each row in 'Books' has a column named 'FilterId'. Basically what happens is that when a user visits the books page, the rows from the 'Books' table are retrieved from the server. I then have to loop though each row and call my stored procedure, passing in the user's id and the 'FilterId' from the book row. So I end up getting a 1 or a 0 for each book, which fyi is used to decide whether the user can view the book.

This works, but it seems like I could do it better and faster with a nested stored procedure (if thats possible). So what I want to do is call one stored procedure and just give it the user's ID. This procedure will then loop through each row of the 'Books' table and call my orginal stored procedure on that row. I then need some way to select only the rows that returned a 1 from my original stored procedure and that would be the result set.

Thanks for your help.

[urtrivedi]

I think you can do 1/0 using only left outer join (creaing views) instead of using nested stored procedure.

If you could post structure of your table, then I could help you to create view for it.

[sngapoonage]

I think you can do 1/0 using only left outer join (creaing views) instead of using nested stored procedure.

If you could post structure of your table, then I could help you to create view for it.

Ok thanks, The structure of my tables are like this

BOOKS

Id       Title      Desc       FilterId
1       'blah'     'blahbla'      1

FILTERS

Id       Name
1      'Some Filter'

FILTER_P

FilterId     Type      Value
1             0         '123'
1             1        'Something'

Sorry I forgot to mention the Filter_p table. It's used by my main stored procedure but won't be needed outside of it

[BitBlt]

Wow. As far as I can see, there's nothing to tie a user to a filter or a book. You'd better post your stored proc...otherwise we're going to think that your app does some kind of magic to get your result.

[urtrivedi]

Where is the userid in these three tables.

[sngapoonage]

Wow. As far as I can see, there's nothing to tie a user to a filter or a book. You'd better post your stored proc...otherwise we're going to think that your app does some kind of magic to get your result.

Haha yeah, boy did I not make that clear. See the value '123' in the value column of Filter_P that is a user id, it can also be other types of information associated with the users profile.

Heres the Proc

@filter int,
		@userid int,
		@ucount int,
		@pcount int,
		@ocount int,
		@countr int,
		@result int OUTPUT
AS
BEGIN
	-- SET NOCOUNT ON added to prevent extra result sets from
	-- interfering with SELECT statements.
	SET NOCOUNT ON;
	SET @countr = 0
	SET @ucount = 0
	SET @pcount = 0
	SET @ocount = 0
	SET @result = 0
	SELECT @pcount = Count(*) FROM Filter_Parameters WHERE Filterid = @filter AND [Type] = 1
	SELECT @ocount = Count(*) FROM Filter_Parameters WHERE Filterid = @filter AND [Type] = 2
	SELECT @ucount = Count(*) FROM Filter_Parameters WHERE Filterid = @filter AND [Type] = 3
	IF(@pcount > 0)
		BEGIN
			IF((SELECT Count(*) FROM BusinessUsers WHERE Platform IN (
				SELECT [Value] AS 'Platform' 
				FROM Filter_Parameters 
				WHERE Filterid = @filter 
				AND [Type] = 1
			) 
			AND Id = @userid) > 0)
				BEGIN
					SET @result = 1
					RETURN
				END
			ELSE
				SET @countr = @countr + 1
		END
	ELSE 
		SET @countr = @countr + 1
	IF(@ocount > 0)
		BEGIN
			IF((SELECT Count(*) FROM BusinessUsers WHERE Outlet IN (
				SELECT [Value] AS 'Outlet' 
				FROM Filter_Parameters 
				WHERE Filterid = @filter 
				AND [Type] = 2
			) 
			AND Id = @userid) > 0)
				BEGIN
					SET @result = 1
					RETURN
				END
		END
	ELSE 
		SET @countr = @countr + 1
	IF(@ucount > 0)
		BEGIN
			IF((SELECT Count(*) FROM BusinessUsers WHERE Id IN (
				SELECT [Value] AS 'Userid' 
				FROM Filter_Parameters 
				WHERE Filterid = @filter 
				AND [Type] = 3
			) 
			AND Id = @userid) > 0)
				BEGIN
					SET @result = 1
					RETURN
				END
		END	
	ELSE 
		SET @countr = @countr + 1
	IF(@countr = 3)
		BEGIN
			SET @result = 1
			RETURN 
		END
	ELSE
		BEGIN
			SET @result = 0
			RETURN
		END
END

EDIT: Also, I'm pretty sure my proc is terrible and could be improved in a million ways to any suggestions are welcome.

[Tess James]

Try something like this.

DECLARE @UserID INT

DECLARE @TempTable TABLE
(
FilterID INT
)

INSERT INTO @TempTable
(
FP.FilterID
)
SELECT DISTINCT T.FilterID FROM
(
SELECT FP.FilterID FROM BusinessUsers BU
INNER JOIN Filter_Parameters FP
ON BU.Platform = FP.[Value] AND FP.[Type] = 1 AND BU.ID = @UserID

UNION

SELECT DISTINCT FP.FilterID FROM BusinessUsers BU
INNER JOIN Filter_Parameters FP
ON BU.Outlet = FP.[Value] AND FP.[Type] = 2 AND BU.ID = @UserID

UNION

SELECT DISTINCT FP.FilterID FROM BusinessUsers BU
INNER JOIN Filter_Parameters FP
ON BU.ID = FP.[Value] AND FP.[Type] = 3 AND BU.ID = @UserID
) AS T

SELECT @UserID , B.Title , CASE WHEN T.FilterID IS NOT NULL THEN 1 ELSE 0 END AS [Accessibility]
FROM
BOOKS B
LEFT OUTER JOIN
@TempTable T
ON B.FilterID = T.FilterID

[BitBlt]

Whew. So many things to point out...
1. You are using procedure parameters as work areas instead of local variables. Bad idea. You should declare @ucount, @pcount, @ocount and @countr as local variables and remove them from the parameter list.
2. Trying to follow your logic, it appears that if ANY of the "if" statements get a hit, you immediately set result to 1 and exit. If you get NO hits, your counter increments each time, so if it ever gets to the last "if" statement, it will ALWAYS return 0. So, the conditional to test the value of @countr is superfluous.
3. You aren't really using any data from any of your "select @pcount = count(*) from..." statements. So you could get away with "if exists(select 1 from...)" rather than doing the count(*) and the "if (@pcount > 0)...". Saves db access time and statements.
4. You should be using inner joins on your selects rather than "in" clauses (those are very inefficient in most cases).
5. You don't really need to do separate statements for each test. Since you really only care that data exists and not what the data actually is, you can combine them into a single conditional existence test.

So if you put it all together, you wind up with something like this:

create procedure dbo.myproc (
		@filter int,
		@userid int,
		@result int OUTPUT
               )
AS
BEGIN

if exists
    (SELECT 1 FROM BusinessUsers a
     inner join Filter_Parameters b
     on a.platform = b.value
     and b.Filterid = @filter 
     and b.[Type] = 1
     where a.Id = @userid
     ) 
or exists
    (SELECT 1 FROM BusinessUsers a
     inner join Filter_Parameters b
     on a.outlet = b.value
     and b.Filterid = @filter 
     and b.[Type] = 2
     where a.Id = @userid
     ) 
or exists
    (SELECT 1 FROM BusinessUsers a
     inner join Filter_Parameters b
     on a.id = b.value
     and b.Filterid = @filter 
     and b.[Type] = 3
     where a.Id = @userid
     ) 
    begin
        SET @result = 1
        RETURN
    end
else
    begin
        SET @result = 0
        RETURN
    end

end

Then you call it with this (assuming these are all integers...your datatypes may vary):

declare @filter1 int
declare @userid1 int
declare @result1 int

select @filter1 = 1, @userid1 = 1
exec dbo.myproc @filter1, @userid1, @result1 OUTPUT
select @result1

Tessy7's example was good too. I hope we've given you some things to think with. Good luck!

[sngapoonage]

Thanks guys, I didn't realize I was doing so much wrong!

One difference is that the end if statement was nessesary in my proc, because if the @*count variables were 0 the counter would be incremented but if the value was not 0 and the value did not satisfy the if statement it would not be incremented and therefore would return 0 if it hit the end before getting any matches. Still, it doesn't really matter now because your way is a lot easier to both use and read.

Anyhow, Tessy's statement was kind of what I was looking for because it returns rows of actual data, I have modified it a bit to suit, is there a way to just not return the rows that have 'Access' as 0? It's not of the utmost importance. Here's what I have

DECLARE @UserID INT
DECLARE @Category NCHAR(20)
SET @Category = 'dia0'
SET @UserId = 20
DECLARE @TempTable TABLE
(
	FilterId INT 
)
INSERT INTO @TempTable
(
	FP.FilterId
)
SELECT DISTINCT T.FilterId FROM 
(
	SELECT FP.FilterId FROM BusinessUsers BU
	INNER JOIN Filter_Parameters FP
	ON BU.Platform = FP.[Value] AND FP.[Type] = 1 AND BU.Id = @UserId

	UNION 

	SELECT DISTINCT FP.FilterId FROM BusinessUsers BU
	INNER JOIN Filter_Parameters FP
	ON BU.Outlet = FP.[Value] AND FP.[Type] = 2 AND BU.Id = @UserId
	
	UNION

	SELECT DISTINCT FP.FilterId FROM BusinessUsers BU
	INNER JOIN Filter_Parameters FP
	ON BU.Id = FP.[Value] AND FP.[Type] = 3 AND BU.ID = @UserId 
) AS T

SELECT s.* ,CASE WHEN T.FilterId IS NOT NULL OR s.FilterId = 0 THEN 1 ELSE 0 END AS [Access]
FROM
Stills s 
LEFT OUTER JOIN 
@TempTable T
ON s.FilterId = T.FilterId
WHERE s.Category = @Category

Category is another field in the Books table which will be passed depending on the selected page.

Thanks both for your excellent help. I like it when people actually help instead of posting irrelevant links.

[BitBlt]

Add this (in green) to your last SQL statement:

...
WHERE s.Category = @Category
AND (CASE WHEN T.FilterId IS NOT NULL OR s.FilterId = 0 THEN 1 ELSE 0 END ) <> 0

Should do the trick. Good work!

[sngapoonage]

Add this (in green) to your last SQL statement:

...
WHERE s.Category = @Category
AND (CASE WHEN T.FilterId IS NOT NULL OR s.FilterId = 0 THEN 1 ELSE 0 END ) <> 0

Should do the trick. Good work!

Awesome! You've made me very happy lol.

[BitBlt]

Glad to help.

[Tess James]

"is there a way to just not return the rows that have 'Access' as 0"

I think you should use inner join instead of left outer join in the last select statement if you don't want to have records with [Access] as zero.

Left Outer Join is kind of expensive. So it's better to avoid using this if you have a choice.

I thought you require both(the books that the user can view and can't view). That's why I used Left Outer Join.

[urtrivedi]

I am not sure
But please check whether this query helps you or not

I think you may create view (remove only where part below), no need of stored procedure

select a.id, a.title, a.desc, a.filterid,b.name,c.Type 
from books a
left outer join filters b on a.filterid=b.id
left outer join filters_p c on b.id=c.filterId
where c.value='123'