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

Impove long SQL Stored Procedure Performance

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
Kevin Robinson
Asked:
Kevin Robinson
  • 5
  • 4
  • 4
  • +1
3 Solutions
 
Pawan KumarDatabase ExpertCommented:
Is this the first time are you facing this performance issue?
0
 
Pawan KumarDatabase ExpertCommented:
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
 
Pawan KumarDatabase ExpertCommented:
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
The 14th Annual Expert Award Winners

The results are in! Meet the top members of our 2017 Expert Awards. Congratulations to all who qualified!

 
Vitor MontalvãoMSSQL Senior EngineerCommented:
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
 
Kevin RobinsonPrivate VB.NET ContractorAuthor Commented:
Awesome will get right on this and report back shortly
0
 
Kevin RobinsonPrivate VB.NET ContractorAuthor Commented:
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
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
Looks that's only math operations and not really accessing any data so no impact.
1
 
Scott PletcherSenior DBACommented:
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
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
Kevin, with all this information do you still have performance issues?
0
 
Kevin RobinsonPrivate VB.NET ContractorAuthor Commented:
Sorry still working on this.  Will come back later to close
0
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
Kevin, please close this question since you said that's working.
Cheers.
0
 
Pawan KumarDatabase ExpertCommented:
Hi Kevin,

Do you need more help with this ?

Regards,
Pawan
0
 
Kevin RobinsonPrivate VB.NET ContractorAuthor Commented:
"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
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
No, I said I was still working on it.
Right. My bad.
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Cloud Class® Course: SQL Server Core 2016

This course will introduce you to SQL Server Core 2016, as well as teach you about SSMS, data tools, installation, server configuration, using Management Studio, and writing and executing queries.

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