Solved

Impove long SQL Stored Procedure Performance

Posted on 2016-10-03
14
61 Views
Last Modified: 2016-11-02
I have inherited a long complex stored procedure.  The stored proc works OK but is very  slow.   I intend to refactor this and possible move some of the functions to CLR code.   However in the meantime could anyone recommend any quick improvements that would improve the speed of execution.     When i say slow it can take unto 30 second  to complete a search.   The database is currently hosted in Sql Azure on plan S02.

summary
The stored procedure search records based on 3 main parameters , there are a lot more but the minimum is the following 3.
Postcode
Distance
Activity Type

Doing the distance search calculation is itself relatively simple (and working) .  The complexity comes from the addresses it needs to compare against are in multiple tables.  The results then need to be complied into a result set which is then used ( by the client ) as paging data.  The postcode Latitude and Longitude are pre-poulated in a separate table.

The script is quite long but mainly due to the number of parameters,  but again most of these are optional.

Any tips would  be gratefully appreciated.
script.txt
Executitaion-Plan.sqlplan
Executitaion-Plan1.sqlplan
0
Comment
Question by:Kevin Robinson
  • 5
  • 4
  • 4
  • +1
14 Comments
 
LVL 17

Expert Comment

by:Pawan Kumar Khowal
Comment Utility
Is this the first time are you facing this performance issue?
0
 
LVL 17

Expert Comment

by:Pawan Kumar Khowal
Comment Utility
Few pointers...

1. SELECT
                  OpportunityID,
                  [dbo].[fncGetDistance](@PostcodeLat, @PostcodeLong, POST.Lat, POST.Lng),

--I think we should have a CROSS APPLY to call this function. Also use Inline Table Valued function. I am not sure what kind of function is this.

2. (@PostcodeRadius = 0 OR (@PostcodeRadius <> 0 AND ([dbo].[fncGetDistance](@PostcodeLat, @PostcodeLong, POST.Lat, POST.Lng) <= @PostcodeRadius))) AND

--In this case also use a CROSS APPLY to call this function. Also use Inline Table Valued function. I am not sure what kind of function is this. If we call a function like it will always scan the entire
--table

--3. Also I would like to insert all data into the temp table before this function calling in the where clause and the joins one ( # point 1 and # point 2 )

--4. Remove these, they will also lead to scans. Also never use functions on a column in the where clause.

(LOWER(ORG.OrganisationName) LIKE ('%' + LOWER(@KeyWord) + '%')) OR
                                                                              (LOWER(OPP.Title) LIKE ('%' + LOWER(@KeyWord) + '%')) OR
                                                                              (LOWER(OPP.[Description]) LIKE ('%' + LOWER(@KeyWord) + '%'))

--5. Also you are using parameters directly which is also not good.
0
 
LVL 17

Assisted Solution

by:Pawan Kumar Khowal
Pawan Kumar Khowal earned 125 total points
Comment Utility
Modified code for you. Ist section..

DECLARE @GatDate AS DATE = GETDATE()
DECLARE @BitSChoolHoliday AS BIT = CAST(@DuringSchoolHoliday AS bit)

SELECT SIT.Address1 + ' ' + SIT.Address2 + ' ' + SIT.Address3TownCity + ' ' +  SIT.Address4County + ' ' + SIT.Address5Postcode Addr , [FormattedPostalCode] , SIT.SiteId
INTO #Sites  
FROM tblSites SIT
CROSS APPLY
(
	SELECT [FormattedPostalCode] FROM [dbo].[fncFormatPostcode](SIT.Address5Postcode)
)t1
WHERE SIT.Deleted = 0

SELECT [FormattedPostCode],[SomeValuefromR]
INTO #Posts  
FROM tblPostcodes POST
CROSS APPLY
(
	SELECT [FormattedPostCode] FROM [dbo].[fncFormatPostcode](POST.Postcode)
)t1
CROSS APPLY
(
	SELECT [SomeValuefromR] FROM [dbo].[fncGetDistance](@PostcodeLat, @PostcodeLong, POST.Lat, POST.Lng
)t2

SELECT 
			OpportunityID,[MayBeaValueColumn],
			CONCAT(SIT.Address1 , ' ' , SIT.Address2 , ' ' , SIT.Address3TownCity , ' ' , SIT.Address4County , ' ' , SIT.Address5Postcode)
			FROM tblOpportunities OPP
				INNER JOIN tblOrganisations ORG ON OPP.FKOrganisationID = ORG.OrganisationID
				INNER JOIN tblOpportunitySiteAddresses OSA ON OPP.OpportunityID = OSA.FKOpportunityID
				INNER JOIN #Sites SIT ON OSA.FKSiteID = SIT.SiteID
				INNER JOIN #Posts POST ON SIT.[FormattedPostalCode] = POST.[FormattedPostCode]
				--CS 100510 - LEFT JOIN should be INNER JOIN

				INNER JOIN tblOpportunityActivities OPA ON OPP.OpportunityID = OPA.FKOpportunityID					

			WHERE
				-- Filter on the OrganisationAddress type
				OPP.AddressesOption = 'OrganisationAddress' AND	
				-- Make sure only non-deleted and active opportunities are returned
				OPP.Deleted = 0 AND								
				OPP.Approved = 1 AND							
				OPP.Display = 1 AND								
				SIT.Deleted = 0 AND								

		
				-- Filter on distance, a value of 0 means that this filter is not
				-- included
				(@PostcodeRadius = 0 OR (@PostcodeRadius <> 0 AND POST.[SomeValuefromR] <= @PostcodeRadius)) AND
				-- And the opportunity has not expired
				(OPP.EndDate IS NULL OR (OPP.EndDate IS NOT NULL AND @GatDate <= OPP.EndDate)) AND
				-- Filtered by the school holiday flag
				(@DuringSchoolHoliday = -1 OR (@DuringSchoolHoliday <> -1 AND OPP.DuringSchoolHolidays = @BitSChoolHoliday)) AND
			    -- Filter by a keyword, we match this against the organisation name and the opportunity title
				-- and description
				
				-- Filter down by the time availbility
				(
				(@MondayDay = 0 OR (@MondayDay <> 0 AND OPP.MondayDay = @MondayDay)) AND
				(@MondayEvening = 0 OR (@MondayEvening <> 0 AND OPP.MondayEvening = @MondayEvening)) AND
				(@MondayNight = 0 OR (@MondayNight <> 0 AND OPP.MondayNight = @MondayNight)) AND
				(@TuesdayDay = 0 OR (@TuesdayDay <> 0 AND OPP.TuesdayDay = @TuesdayDay)) AND
				(@TuesdayEvening = 0 OR (@TuesdayEvening <> 0 AND OPP.TuesdayEvening = @TuesdayEvening)) AND
				(@TuesdayNight = 0 OR (@TuesdayNight <> 0 AND OPP.TuesdayNight = @TuesdayNight)) AND
				(@WednesdayDay = 0 OR (@WednesdayDay <> 0 AND OPP.WednesdayDay = @WednesdayDay)) AND
				(@WednesdayEvening = 0 OR (@WednesdayEvening <> 0 AND OPP.WednesdayEvening = @WednesdayEvening)) AND
				(@WednesdayNight = 0 OR (@WednesdayNight <> 0 AND OPP.WednesdayNight = @WednesdayNight)) AND
				(@ThursdayDay = 0 OR (@ThursdayDay <> 0 AND OPP.ThursdayDay = @ThursdayDay)) AND
				(@ThursdayEvening = 0 OR (@ThursdayEvening <> 0 AND OPP.ThursdayEvening = @ThursdayEvening)) AND
				(@ThursdayNight = 0 OR (@ThursdayNight <> 0 AND OPP.ThursdayNight = @ThursdayNight)) AND
				(@FridayDay = 0 OR (@FridayDay <> 0 AND OPP.FridayDay = @FridayDay)) AND
				(@FridayEvening = 0 OR (@FridayEvening <> 0 AND OPP.FridayEvening = @FridayEvening)) AND
				(@FridayNight = 0 OR (@FridayNight <> 0 AND OPP.FridayNight = @FridayNight)) AND
				(@SaturdayDay = 0 OR (@SaturdayDay <> 0 AND OPP.SaturdayDay = @SaturdayDay)) AND
				(@SaturdayEvening = 0 OR (@SaturdayEvening <> 0 AND OPP.SaturdayEvening = @SaturdayEvening)) AND
				(@SaturdayNight = 0 OR (@SaturdayNight <> 0 AND OPP.SaturdayNight = @SaturdayNight)) AND
				(@SundayDay = 0 OR (@SundayDay <> 0 AND OPP.SundayDay = @SundayDay)) AND
				(@SundayEvening = 0 OR (@SundayEvening <> 0 AND OPP.SundayEvening = @SundayEvening)) AND
				(@SundayNight = 0 OR (@SundayNight <> 0 AND OPP.SundayNight = @SundayNight))
				)
                AND
			--	Filter on the opportunity details
			--  JR 24/08/2010 - modified this section so that is is applicable to checkboxes instead of drop down list
(
				(@OpportunityDetailsDontMind = 1) OR
(               (@OpportunityDetailsDisabledAccess = 0 OR(@OpportunityDetailsDisabledAccess <> 0 AND (OPP.DisabledAccess = 1))) AND
				(@OpportunityDetailsTravelExpenses = 0 OR(@OpportunityDetailsTravelExpenses <> 0 AND (ORG.TravelExpenses = 1))) AND 
				(@OpportunityDetailsChildcare = 0 OR(@OpportunityDetailsChildcare <> 0 AND (OPP.ChildCareExpenses = 1))) AND 
				(@OpportunityDetailsMillenniumVolunteers = 0 OR(@OpportunityDetailsMillenniumVolunteers <> 0 AND (OPP.MV = 1))) 
)
)		AND
	
	--	Filter on the opportunity activities
(
                (@Activity1ID = 1) OR 
(               (@Activity1ID <> 1 AND (OPA.FKActivityID = @Activity1ID)) 
)
)	  

Open in new window




Few More pointers

1. Very important - REMOVE table Variable @DistinctMatchingOpportunities , Use temp table instead of it. Table Variable dont have stats on them.

2. Do not DELETE , USE TRUNCATE instead if no where clause is there.

3. I didnt get the below part, What exactly you wanted to do here..

DELETE FROM #MatchingOpportunities a
WHERE      OpportunityID IN  (Select OpportunityID from #MatchingOpportunities where OpportunityDistance = 0)
       AND OpportunityID IN  (Select OpportunityID from #MatchingOpportunities where OpportunityDistance = 0.1)
       AND OpportunityDistance = 0.1;

4. Also can you do the sorting in the UI. Sorting in SQL Server is costly.

5. CREATE INDEX ON #MatchingOpportunities(OpportunityID)


I hope it helps.

Pawan
0
 
LVL 45

Accepted Solution

by:
Vitor Montalvão earned 250 total points
Comment Utility
By the execution plans where the SP is taking longer is on the write commands (INSERT and DELETE) meaning that much can't be done unless having better storage but since is Azure I guess that no improvement can be made here.

Anyway, just as information, using functions on columns that are in the WHERE clause will take the ability to use any index that might exist for the column. Example:
(LOWER(ORG.OrganisationName) LIKE ('%' + LOWER(@KeyWord) + '%')) OR
(LOWER(OPP.Title) LIKE ('%' + LOWER(@KeyWord) + '%')) OR
(LOWER(OPP.[Description]) LIKE ('%' + LOWER(@KeyWord) + '%'))

In the above example, any index that might exist for OrganisationName, Title and Description won't be able to be used. Ofc, if you don't have any index on those columns then there's no impact.

Also, do not use DISTINCT:
SELECT DISTINCT 
		OpportunityID,  
		OpportunityDistance, --CS removed minimum
		OpportunityAddress
FROM #MatchingOpportunities
--GROUP BY OpportunityID; -- CS removed for now ... do we need the GROUP BY?

Open in new window

Replace any DISTINCT with a GROUP BY clause:
SELECT OpportunityID,  
		OpportunityDistance, --CS removed minimum
		OpportunityAddress
FROM #MatchingOpportunities
GROUP BY OpportunityID, OpportunityDistance, OpportunityAddress

Open in new window

0
 
LVL 3

Author Comment

by:Kevin Robinson
Comment Utility
Awesome will get right on this and report back shortly
0
 
LVL 3

Author Comment

by:Kevin Robinson
Comment Utility
This is the caculateDistance funciton.    seems a bit much.   I vaguely remember doing something like this as an inline function.  Not as pretty maybe but  it was faster.

USE [vndev]
GO
/****** Object:  UserDefinedFunction [dbo].[fncGetDistance]    Script Date: 10/4/2016 2:44:27 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
/************************************************************
Title:                  dbo.fncGetDistance
Author:             Unknown
Created:             Unknown
Last Modified:      Unknown
Purpose:
Work out the distance between two co-ordinates based on
latitude and longitude
************************************************************/
ALTER FUNCTION [dbo].[fncGetDistance]
(
      @lat1 FLOAT,
      @long1 FLOAT,
      @lat2 FLOAT,
      @long2 FLOAT
)
RETURNS FLOAT
AS
BEGIN

      -- Declare local variables
      DECLARE @earth FLOAT
      DECLARE @dlong FLOAT
      DECLARE @dlat FLOAT
      DECLARE @sinlat FLOAT
      DECLARE @sinlong FLOAT
      DECLARE @a FLOAT
      DECLARE @c FLOAT
      DECLARE @d FLOAT

      -- Set the radius of the earth - to change into km
      -- use the value below
      -- SET @earth = 6371; -- km
      SET @earth = 3960; --miles

      -- Point 1 cords
      SET @lat1 = dbo.fncDegToRad(@lat1);
      SET @long1= dbo.fncDegToRad(@long1);

      -- Point 2 cords
      SET @lat2 = dbo.fncDegToRad(@lat2);
      SET @long2= dbo.fncDegToRad(@long2);

      -- Haversine Formula (http://en.wikipedia.org/wiki/Haversine_formula)
      SET @dlong=@long2-@long1;
      SET @dlat=@lat2-@lat1;

      SET @sinlat=Sin(@dlat/2);
      SET @sinlong=Sin(@dlong/2);

      SET @a=(@sinlat*@sinlat)+Cos(@lat1)*Cos(@lat2)*(@sinlong*@sinlong);
      SET @c=2*Asin(CASE WHEN Sqrt(@a) < 1 THEN Sqrt(@a) ELSE 1 END);
      SET @d=Round(@earth*@c,1);

      RETURN @d;
      
END
0
 
LVL 45

Expert Comment

by:Vitor Montalvão
Comment Utility
Looks that's only math operations and not really accessing any data so no impact.
1
Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 
LVL 69

Assisted Solution

by:ScottPletcher
ScottPletcher earned 125 total points
Comment Utility
Typically the big performance gains come from getting the best clustering on the underlying table(s).  And after that adding/adjusting nonclus indexes.

Given the wide range of possibilities for this query, you should almost certainly force a recompile by adding:
OPTION (RECOMPILE)
to the end of the query.

Beyond that, scalar functions are still much slower than inline-table-valued functions, because the latter are converted to inline code, avoiding the overhead of separate function calls.  Particularly scalars with that many local variables.

I've therefore converted the function converted to an ITVF below.  As with all ITVFs, you use CROSS|OUTER APPLY to invoke the function rather than just SELECTing the value:

--invoke new function
SELECT ..., fgd.distance, ...
FROM table_name tn
OUTER APPLY dbo.fncGetDistance(tn.lat1_col, tn.long1_col, tn.lat2_col, tn.long2_col) AS fgd


SET ANSI_NULLS ON
SET QUOTED_IDENTIFIER ON
GO
/************************************************************
Title:  dbo.fncGetDistance
Author: Unknown
Created: Unknown
Last Modified: Unknown
Purpose:
Works out the distance between two co-ordinates based on
latitude and longitude
************************************************************/
CREATE FUNCTION [dbo].[fncGetDistance]
(
      @lat1 FLOAT,
      @long1 FLOAT,
      @lat2 FLOAT,
      @long2 FLOAT
)
RETURNS TABLE
AS
RETURN (
    SELECT distance
    FROM (
        SELECT earth = 3960 /*3960mi|6371km*/,
            lat1 = Radians(@lat1),
            long1 = Radians(@long1),
            lat2 = Radians(@lat2),
            long2 = Radians(@long2)
    ) AS starting_values
    CROSS APPLY (
        SELECT sinlat = Sin((lat2-lat1)/2),
            sinlong = Sin((long2-long1)/2)
    ) AS Haversine_Formula_Step1 /*http://en.wikipedia.org/wiki/Haversine_formula*/
    CROSS APPLY (
        SELECT a = (sinlat*sinlat)+Cos(lat1)*Cos(lat2)*(sinlong*sinlong)        
    ) AS Haversine_Formula_Step2
    CROSS APPLY (
        SELECT c = 2*Asin(CASE WHEN Sqrt(a) < 1 THEN Sqrt(a) ELSE 1 END)
    ) AS Haversine_Formula_Step3
    CROSS APPLY (
        SELECT distance = Round(earth*c,1)
    ) AS Haversine_Formula_Step4
)
GO
/*end of function*/
DROP FUNCTION [dbo].[fncGetDistance]
0
 
LVL 45

Expert Comment

by:Vitor Montalvão
Comment Utility
Kevin, with all this information do you still have performance issues?
0
 
LVL 3

Author Comment

by:Kevin Robinson
Comment Utility
Sorry still working on this.  Will come back later to close
0
 
LVL 45

Expert Comment

by:Vitor Montalvão
Comment Utility
Kevin, please close this question since you said that's working.
Cheers.
0
 
LVL 17

Expert Comment

by:Pawan Kumar Khowal
Comment Utility
Hi Kevin,

Do you need more help with this ?

Regards,
Pawan
0
 
LVL 3

Author Comment

by:Kevin Robinson
Comment Utility
"Kevin, please close this question since you said that's working".
No, I said I was still working on it.

However I will close the question.
0
 
LVL 45

Expert Comment

by:Vitor Montalvão
Comment Utility
No, I said I was still working on it.
Right. My bad.
0

Featured Post

Highfive + Dolby Voice = No More Audio Complaints!

Poor audio quality is one of the top reasons people don’t use video conferencing. Get the crispest, clearest audio powered by Dolby Voice in every meeting. Highfive and Dolby Voice deliver the best video conferencing and audio experience for every meeting and every room.

Join & Write a Comment

In this article I will describe the Copy Database Wizard method as one possible migration process and I will add the extra tasks needed for an upgrade when and where is applied so it will cover all.
Healthcare organizations in the United States must adhere to the guidance of both the HIPAA (Health Insurance Portability and Accountability Act) and HITECH (Health Information Technology for Economic and Clinical Health Act) for securing and protec…
Via a live example combined with referencing Books Online, show some of the information that can be extracted from the Catalog Views in SQL Server.
Using examples as well as descriptions, and references to Books Online, show the documentation available for datatypes, explain the available data types and show how data can be passed into and out of variables.

771 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

10 Experts available now in Live!

Get 1:1 Help Now