Solved

What is wrong with my Stored Procedure??

Posted on 2013-05-27
13
605 Views
Last Modified: 2013-06-02
Hello,

Please can you tell me what is wrong with this stored procedure, and whether it can be more efficient --

CREATE PROCEDURE [dbo].[JobsearchSectorDesc](
@region int,
@location int,
@sector int,
@hours int,
@jobtype int,
@clientid int,
@keywords nvarchar(50)
)
AS
BEGIN
SET NOCOUNT ON;
BEGIN

DECLARE @Label nvarchar(50)
DECLARE @Count int
DECLARE @SectorID int


/****************** Query to get results if @keywords is not null ****************/
IF @keywords IS NOT NULL
BEGIN

DECLARE @MyQuote varchar(10)
DECLARE @MyWildCard varchar(5)

SET @MyQuote = '"'
SET @MyWildCard = '*'
SET @keywords = @MyQuote + @keywords + @MyWildCard + @MyQuote

Select @count = COUNT(JBAID) Adverts, @label= JBACategory Sector, @SectorID = cat.ID SectorID 
FROM [dbo].[JBAdvert] A
inner join dbo.Client C on C.ID = A.ClientID
inner join dbo.sector cat on cat.sector = A.JBACategory
inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

WHERE		(S.JBSSiteID = @region or @region IS NULL) 
AND			(L.JBLID = @location or @location is null) 
AND			(cat.ID = @sector or @sector is null) 
AND			(A.[Hours] = @hours or @hours is NULL) 
AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
AND			(C.ID= @clientid or @clientid is null) 
And		CONTAINS(JBADescription, @keywords)

Group by JBACategory, cat.ID
order by Adverts desc
END
/****************** End of Query to get results if @keywords is not null ****************/
ELSE
/****************** Query to get results if @keywords is null ****************/
BEGIN
Set @count = COUNT(JBAID) Adverts, @label= JBACategory Sector, @SectorID = cat.ID SectorID 
FROM [dbo].[JBAdvert] A
inner join dbo.Client C on C.ID = A.ClientID
inner join dbo.sector cat on cat.sector = A.JBACategory
inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

WHERE		(S.JBSSiteID = @region or @region IS NULL) 
AND			(L.JBLID = @location or @location is null) 
AND			(cat.ID = @sector or @sector is null) 
AND			(A.[Hours] = @hours or @hours is NULL) 
AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
AND			(C.ID= @clientid or @clientid is null) 

Group by JBACategory, cat.ID
order by Adverts desc        
END
/****************** End of Query to get results if @keywords is null ****************/
/****************** now that we have values for @count, @label and @sectorid begin to build @url using dbo.faxchars to remove special charaters and spaces ****************/
/****************** first element in @url is /jobs/ ****************/
BEGIN
DECLARE @URL nvarchar(500)
set @URL = '/jobs/'
/****************** End of first element ****************/
/****************** first section to check is client as @URL in full can be like /jobs/client/sector/region/location/hours/jobtype/?keywords=keywords ****************/
/****************** check if client is needed ****************/
If @clientid is not null
BEGIN
if exists (select Name from dbo.Client where ID = @clientid)
BEGIN
Declare @tempurl nvarchar(250)
Set @tempurl = dbo.fixchars(Name) from dbo.client where id = @clientid
Set @url = @url + @tempurl + '/' + dbo.fixchars(@Label) + '/'
END
/****************** end of check if client is needed ****************/
Else
/****************** client is not needed move straight to nect element which is sector we know this exists so no if statement is needed ****************/
BEGIN
Set @url = @url + dbo.fixchars(@Label) + '/'
END
END
/****************** End of check ****************/
/****************** check if region is needed ****************/
if @region is not null
BEGIN
if exists (select JBSRegion from dbo.JBSite where JBSSiteID = @region)
BEGIN
set @tempurl = dbo.fixchars(JBSRegion)from dbo.JBSite where JBSSiteID = @region
Set @url = @url + @tempurl + '/' 
END
/********** Check if @location exists, this is a nested if as @location can only exist if @region exists******/
if @location is not null
BEGIN
if exists (select JBLocation from dbo.JBLocation where JBLID = @location)
BEGIN
set @tempurl = dbo.fixchars(JBLocation)from dbo.JBLocation where JBLID = @location
Set @url = @url + @tempurl + '/' 
END
END
/********** End of checking if @location exists *************/
END
/****************** end of check if region is needed ****************/
/****************** check if hours is needed ****************/
BEGIN
do something
END
/****************** end of check if hours is needed ****************/
/****************** check if jobtype is needed ****************/
BEGIN
do something
END
/****************** end of check if jobtype is needed ****************/
/****************** finally check if keywords is needed ****************/
if @keywords is not null
begin 
SET @url = @url + '?keywords=' + @keywords
END
/****************** end of finally check if keywords is needed ****************/
/****************** Return data *******************/
BEGIN
Select 
@count as Adverts,
@label as Sector
@SectorID as SectorID
@url as URL
END
/****************** End of Return data *******************/
END
END
GO

Open in new window


So what it is intended to do is -

Return a count of all records grouped by JBACategory and store this values in parameters.

Once this has been established, it should start to build the URL, based on the parameters that were sent.

dbo.fixchars is a fuction that removes all special characteres and spaces.

At present my SP wont compile, I have lots of red underlining starting on row 32 -

Select @count = COUNT(JBAID) Adverts,

Open in new window



Appreciate any suggestions. Thank you so much

G
0
Comment
Question by:garethtnash
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 4
  • 3
  • +2
13 Comments
 
LVL 66

Expert Comment

by:Jim Horn
ID: 39199765
For starters, looking at line 32, what is Adverts, Sector, and SectorID?
The space before these names, and the names themselves, are the cause of the error.

If they are aliases, those aren't needed.
0
 
LVL 75

Expert Comment

by:Anthony Perkins
ID: 39199783
Please can you tell me what is wrong with this stored procedure, and whether it can be more efficient
I have no idea, it had so many compile errors, I gave up.
0
 
LVL 16

Expert Comment

by:DcpKing
ID: 39199806
Never give up, acp <grin>!

Gareth: start by taking your code at/after line 32 out to another query window and reformatting it like this:

Select	@count = COUNT(a.JBAID), 
		@label= cat.JBACategory, 
		@SectorID = cat.ID
FROM [dbo].[JBAdvert] A
inner join dbo.Client C 			on C.ID = A.ClientID
inner join dbo.sector cat 		on cat.sector = A.JBACategory
group by cat.JBACategory, cat.ID

Open in new window


It's my guess that that should work (it won't give the right results, but it should run). If it doesn't then correct my assumptions of which tables contain JBAID and JBACategory.

Once that works, start adding more of your original query until you have it all.

Then keep going like this.

good luck

hth

Mike
0
Comprehensive Backup Solutions for Microsoft

Acronis protects the complete Microsoft technology stack: Windows Server, Windows PC, laptop and Surface data; Microsoft business applications; Microsoft Hyper-V; Azure VMs; Microsoft Windows Server 2016; Microsoft Exchange 2016 and SQL Server 2016.

 
LVL 49

Expert Comment

by:PortletPaul
ID: 39200069
>>Return a count of all records grouped by JBACategory
which would/could be multiple values (one count for each JBACategory)

how then do you put multiple values into a single integer @variable?
>>DECLARE @Count int

shouldn't you be using a cursor for those results?
0
 

Author Comment

by:garethtnash
ID: 39202131
Hi All,

Sorry this is the first opportunity I have had to come back to this.

I've started as per Mike.

Quick question though, can i use --

if @region is not null AND exists (select JBSRegion from dbo.JBSite where JBSSiteID = @region)
			BEGIN

Open in new window


instead of --

if @region is not null
		BEGIN
		if exists (select JBSRegion from dbo.JBSite where JBSSiteID = @region)
			BEGIN

Open in new window


Thank you
0
 

Author Comment

by:garethtnash
ID: 39202194
OK,

I've made a little progress, and managed to royally confuse myself.. :(

So I got the Stored Procedure to compile, which is the progress....

CREATE PROCEDURE [dbo].[JobsearchSectorDescNEW](
@region int,
@location int,
@sector int,
@hours int,
@jobtype int,
@clientid int,
@keywords nvarchar(50)
)
AS
BEGIN
SET NOCOUNT ON;
	BEGIN
	DECLARE @Label nvarchar(50)
	DECLARE @Count int
	DECLARE @SectorID int
	DECLARE @URL nvarchar(500)
	Declare @tempurl nvarchar(250)

	IF @keywords IS NOT NULL
		/******/
		/******/
		BEGIN

		DECLARE @MyQuote varchar(10)
		DECLARE @MyWildCard varchar(5)

		SET @MyQuote = '"'
		SET @MyWildCard = '*'
		SET @keywords = @MyQuote + @keywords + @MyWildCard + @MyQuote

		select @Count = COUNT(JBAID),
		@Label = JBACategory,
		@SectorID = cat.ID 
		FROM [dbo].[JBAdvert] A
		inner join dbo.Client C on C.ID = A.ClientID
		inner join dbo.sector cat on cat.sector = A.JBACategory
		inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
		inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

		WHERE		(S.JBSSiteID = @region or @region IS NULL) 
		AND			(L.JBLID = @location or @location is null) 
		AND			(cat.ID = @sector or @sector is null) 
		AND			(A.[Hours] = @hours or @hours is NULL) 
		AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
		AND			(C.ID= @clientid or @clientid is null) 
		And		CONTAINS(JBADescription, @keywords)

		Group by JBACategory, cat.ID
		order by COUNT(JBAID) desc
		END
		/******/
		/******/
	ELSE
		/******/
		/******/
		BEGIN
		select @Count = COUNT(JBAID),
		@Label = JBACategory,
		@SectorID = cat.ID 
		FROM [dbo].[JBAdvert] A
		inner join dbo.Client C on C.ID = A.ClientID
		inner join dbo.sector cat on cat.sector = A.JBACategory
		inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
		inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

		WHERE		(S.JBSSiteID = @region or @region IS NULL) 
		AND			(L.JBLID = @location or @location is null) 
		AND			(cat.ID = @sector or @sector is null) 
		AND			(A.[Hours] = @hours or @hours is NULL) 
		AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
		AND			(C.ID= @clientid or @clientid is null) 

		Group by JBACategory, cat.ID
		order by COUNT(JBAID) desc        
		END
		/******/
		/******/
		BEGIN
		set @URL = '/jobs/'
		END
		/******/
		/******/
		If @clientid is not null
		BEGIN
		if exists (select Name from dbo.Client where ID = @clientid)
			BEGIN
				Select @tempurl = dbo.fixchars(Name) from dbo.client where id = @clientid
				Set @url = @url + @tempurl + '/' + dbo.fixchars(@Label) + '/'
			END
			Else
			BEGIN
				Set @url = @url + dbo.fixchars(@Label) + '/'
			END
		END
		/******/
		/******/
		if @region is not null
		BEGIN
		if exists (select JBSRegion from dbo.JBSite where JBSSiteID = @region)
			BEGIN
			select @tempurl = dbo.fixchars(JBSRegion)from dbo.JBSite where JBSSiteID = @region
			Set @url = @url + @tempurl + '/' 
			END
		END
		/******/
		/******/
		if @location is not null
		BEGIN
		if exists (select JBLocation from dbo.JBLocation where JBLID = @location)
			BEGIN
			select @tempurl = dbo.fixchars(JBLocation)from dbo.JBLocation where JBLID = @location
			Set @url = @url + @tempurl + '/' 
			END
		END
		/******/
		/******/
		if @hours is not null
		BEGIN
		if exists (select HoursType from dbo.HoursType where ID = @hours)
			BEGIN
			select @tempurl = dbo.fixchars(HoursType)from dbo.HoursType where ID = @hours
			Set @url = @url + @tempurl + '/' 
			END
		END
		/******/
		/******/
		if @jobtype is not null
		BEGIN
		if exists (select etype from dbo.EmploymentType where ID = @jobtype)
			BEGIN
			select @tempurl = dbo.fixchars(etype)from dbo.EmploymentType where ID = @jobtype
			Set @url = @url + @tempurl + '/' 
			END
		END
		/******/
		/******/
		if @keywords is not null
		begin 
		SET @url = @url + '?keywords=' + RTRIM(LTRIM(@keywords))
		END
		/******/
		/******/
		BEGIN
		Select 
			@count as Adverts,
			@label as Sector,
			@SectorID as SectorID,
			@url as URL
		END
		/******/
	END
END


GO

Open in new window


BUT

This only produces one row of data, instead of a row for each sector that has adverts...

Please advise.

Thank you
0
 
LVL 75

Expert Comment

by:Anthony Perkins
ID: 39203191
This only produces one row of data, instead of a row for each sector that has adverts...
And that is because you only have one resultset:
                SELECT  @count AS Adverts,
                        @label AS Sector,
                        @SectorID AS SectorID,
                        @url AS URL

All the others just assign values to variables and do not return a resultset.
0
 

Author Comment

by:garethtnash
ID: 39205483
Thanks ACP,

is this better? efficient?

it seems to work -

CREATE PROCEDURE [dbo].[JobsearchSectorDescNEW](
@region int,
@location int,
@sector int,
@hours int,
@jobtype int,
@clientid int,
@keywords nvarchar(50)
)
AS
BEGIN
SET NOCOUNT ON;
	BEGIN
	DECLARE @URL nvarchar(500)
	DECLARE @URL2 nvarchar(500)
	Declare @tempurl nvarchar(250)
		/******/
		BEGIN
		set @URL = '/jobs/'
		set @URL2 = ''
		END
		/******/
		/******/
		if exists (select Name from dbo.Client where ID = @clientid)
			BEGIN
				Select @tempurl = dbo.fixchars(lower(Name)) from dbo.client where id = @clientid
				Set @URL = @URL + @tempurl + '/'
			END
		/*****/
		/******/
		if exists (select JBSRegion from dbo.JBSite where JBSSiteID = @region)
			BEGIN
			set @tempurl = ''
			select @tempurl = dbo.fixchars(lower(JBSRegion))from dbo.JBSite where JBSSiteID = @region
			Set @URL2 = @tempurl + '/' 
			END
		/******/
		/******/
		if exists (select JBLocation from dbo.JBLocation where JBLID = @location)
			BEGIN
			select @tempurl = dbo.fixchars(lower(JBLocation))from dbo.JBLocation where JBLID = @location
			Set @URL2 = @URL2 + @tempurl + '/' 
			END
		/******/
		/******/
		if exists (select etype from dbo.EmploymentType where ID = @jobtype)
			BEGIN
			select @tempurl = dbo.fixchars(lower(etype))from dbo.EmploymentType where ID = @jobtype
			Set @URL2 = @URL2 + @tempurl + '/' 
			END
		/******/
		/******/
		if exists (select HoursType from dbo.HoursType where ID = @hours)
			BEGIN
			select @tempurl = dbo.fixchars(lower(HoursType))from dbo.HoursType where ID = @hours
			Set @URL2 = @URL2 + @tempurl + '/' 
			END
		/******/
		/******/
		if @keywords is not null
		begin 
		SET @URL2 = @URL2 + '?keywords=' + lower(RTRIM(LTRIM(@keywords)))
		END
		/******/
	IF @keywords IS NOT NULL
		/******/
		/******/
		BEGIN

		DECLARE @MyQuote varchar(10)
		DECLARE @MyWildCard varchar(5)

		SET @MyQuote = '"'
		SET @MyWildCard = '*'
		SET @keywords = @MyQuote + @keywords + @MyWildCard + @MyQuote

		select COUNT(JBAID) Adverts,
		JBACategory Sector,
		cat.ID SectorID,
		@URL + dbo.fixchars(lower(JBACategory)) + '/' + @URL2  as HTTPLink
		FROM [dbo].[JBAdvert] A
		inner join dbo.Client C on C.ID = A.ClientID
		inner join dbo.sector cat on cat.sector = A.JBACategory
		inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
		inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

		WHERE		(S.JBSSiteID = @region or @region IS NULL) 
		AND			(L.JBLID = @location or @location is null) 
		AND			(cat.ID = @sector or @sector is null) 
		AND			(A.[Hours] = @hours or @hours is NULL) 
		AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
		AND			(C.ID= @clientid or @clientid is null) 
		And		CONTAINS(JBADescription, @keywords)

		Group by JBACategory, cat.ID
		order by COUNT(JBAID) desc
		END
		/******/
		/******/
	ELSE
		/******/
		/******/
		BEGIN
		select COUNT(JBAID) Adverts,
		JBACategory Sector,
		cat.ID SectorID,
		@URL + dbo.fixchars(lower(JBACategory)) + '/' + @URL2  as HTTPLink
		FROM [dbo].[JBAdvert] A
		inner join dbo.Client C on C.ID = A.ClientID
		inner join dbo.sector cat on cat.sector = A.JBACategory
		inner join dbo.JBLocation L on L.JBLSiteID = A.JBASiteID AND L.JBLocation = A.JBALocation
		inner join dbo.JBSite S on S.JBSSiteID = A.JBASiteID

		WHERE		(S.JBSSiteID = @region or @region IS NULL) 
		AND			(L.JBLID = @location or @location is null) 
		AND			(cat.ID = @sector or @sector is null) 
		AND			(A.[Hours] = @hours or @hours is NULL) 
		AND			(A.JBAEmplymentType = CASE When @jobtype = 1 then 'Permanent' when @jobtype = 2 then 'Contract' when @jobtype = 3 then 'Temporary' End or @jobtype is null) 
		AND			(C.ID= @clientid or @clientid is null) 

		Group by JBACategory, cat.ID
		order by COUNT(JBAID) desc        
		END
		/******/
	END
END

GO

Open in new window


Please let me know if you think it needs improvements? I'd appreciate your input.
0
 
LVL 75

Accepted Solution

by:
Anthony Perkins earned 500 total points
ID: 39206469
is this better? efficient?
That still only returns one resultset.  As to whether it is more efficient, it is difficult to know for sure.  I personally would be inclined to see if you can combine it into a single query.

The parts that I would change out of preference/efficiency:
1. Just use the LOWER() once at the end in the query and not every time you query it.  As in:
LOWER(@URL + dbo.fixchars(JBACategory) + '/' + @URL2) AS HTTPLink
2. See if you can lose the fixchars() function or do it some other way.
3. See if you are returning more than one row when assigning a value, as in for example:
                    SELECT  @tempurl = dbo.fixchars(LOWER(JBLocation))
                    FROM    dbo.JBLocation
                    WHERE   JBLID = @location
    And change it so that only one row is returned.
4. Lose the @tempurl in favor of this structure (of course this assumes only one row is returned):
                    SELECT  @URL2 = @URL2 +  dbo.fixchars(LOWER(HoursType))  + '/'
                    FROM    dbo.HoursType
                    WHERE   ID = @hours
5.  Make sure that all columns (including JBACategory) have an alias when more than one table is involved.
6. Instead of this:
 ORDER BY COUNT(JBAID) DESC
This:
ORDER BY Adverts
0
 
LVL 49

Expert Comment

by:PortletPaul
ID: 39206491
this seems to be a duplicated question (or at least related)
http://www.experts-exchange.com/Microsoft/Development/MS-SQL-Server/SQL_Server_2008/Q_28140885.html

did you try the use of max() at id 39203312
0
 
LVL 75

Expert Comment

by:Anthony Perkins
ID: 39206535
Ah good point.  I guess I shall desist in contributing any further.  It is a shame that members do that.  It does not really help their cause.
0
 
LVL 49

Expert Comment

by:PortletPaul
ID: 39206551
on the contrary ac, you found a dozen points I glossed over (much to my personal horror)
I was just trying to bring the dual threads back into a cohesive solution
0
 

Author Closing Comment

by:garethtnash
ID: 39214325
Excellent, ACP thank you for your help and sorry for double posting, I was looking at the SP from a slightly different perspective, and it was a day later, I'm conscious that sometimes if you leave a question open too long, it loses the focus..

Thank you so much for your help.

GTN
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Load balancing is the method of dividing the total amount of work performed by one computer between two or more computers. Its aim is to get more work done in the same amount of time, ensuring that all the users get served faster.
Ever wondered why sometimes your SQL Server is slow or unresponsive with connections spiking up but by the time you go in, all is well? The following article will show you how to install and configure a SQL job that will send you email alerts includ…
Familiarize people with the process of retrieving data from SQL Server using an Access pass-thru query. Microsoft Access is a very powerful client/server development tool. One of the ways that you can retrieve data from a SQL Server is by using a pa…
This video shows how to set up a shell script to accept a positional parameter when called, pass that to a SQL script, accept the output from the statement back and then manipulate it in the Shell.

734 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question