• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 621
  • Last Modified:

What is wrong with my Stored Procedure??

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
garethtnash
Asked:
garethtnash
  • 4
  • 4
  • 3
  • +2
1 Solution
 
Jim HornMicrosoft SQL Server Developer, Architect, and AuthorCommented:
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
 
Anthony PerkinsCommented:
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
 
DcpKingCommented:
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
Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

 
PortletPaulCommented:
>>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
 
garethtnashAuthor Commented:
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
 
garethtnashAuthor Commented:
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
 
Anthony PerkinsCommented:
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
 
garethtnashAuthor Commented:
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
 
Anthony PerkinsCommented:
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
 
PortletPaulCommented:
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
 
Anthony PerkinsCommented:
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
 
PortletPaulCommented:
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
 
garethtnashAuthor Commented:
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

Hire Technology Freelancers with Gigs

Work with freelancers specializing in everything from database administration to programming, who have proven themselves as experts in their field. Hire the best, collaborate easily, pay securely, and get projects done right.

  • 4
  • 4
  • 3
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now