Solved

What is wrong with my Stored Procedure??

Posted on 2013-05-27
13
594 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
  • 4
  • 4
  • 3
  • +2
13 Comments
 
LVL 65

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
 
LVL 48

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
Free Gift Card with Acronis Backup Purchase!

Backup any data in any location: local and remote systems, physical and virtual servers, private and public clouds, Macs and PCs, tablets and mobile devices, & more! For limited time only, buy any Acronis backup products and get a FREE Amazon/Best Buy gift card worth up to $200!

 
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 48

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 48

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

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Everyone has problem when going to load data into Data warehouse (EDW). They all need to confirm that data quality is good but they don't no how to proceed. Microsoft has provided new task within SSIS 2008 called "Data Profiler Task". It solve th…
If you have heard of RFC822 date formats, they can be quite a challenge in SQL Server. RFC822 is an Internet standard format for email message headers, including all dates within those headers. The RFC822 protocols are available in detail at:   ht…
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…
Via a live example, show how to setup several different housekeeping processes for a SQL Server.

746 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

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now