March 25, 2014 at 6:01 am
Can anyone see where I can make this more efficient:
USE [Abi_Reports]
go
SET ansi_nulls ON
go
SET quoted_identifier ON
go
-- =============================================
ALTER PROC [SPECCOMM].[Usp_sc_provider_1ac]
--declare
@p_OrgCommCode VARCHAR(50)='Q64,Q65,Q66,YDD,Other',
@p_CommCode VARCHAR(10)='1,2,3,4'--'BGSW,BNSSG,DCIOS,Other'
--@p_ServiceLineCode VARCHAR(max)='NCBPS13X,NCBPS17Z'
WITH recompile
--added, this is either benign when added or improves performance by 1 sec (option recompile also used)
AS
BEGIN
SET nocount ON;
SET TRANSACTION isolation level READ uncommitted;
--added - no need for nolock hints elsewhere
DECLARE @Previous2FY AS VARCHAR(10) = (SELECT Min(finyr)
FROM [ABI_Reports].speccomm.tbl_apc_spell_psir)
IF Object_id('tempdb..#tempac') IS NOT NULL
DROP TABLE #tempac
SELECT finyr,
CASE
WHEN finyr = @Previous2FY THEN 'Min'
ELSE finyr
END AS MinYr,
a.finmonno,
m.finmonname,
[activitytypeorder],
CASE
WHEN activitytypeorder = 5
AND aimtc_type_code = '3' THEN 'Elective Day Case'
WHEN activitytypeorder = 3 THEN 'New Outpatient'
ELSE activitytype
END AS ActivityType,
ha_code,
nhs_e_contract_id,
age_flag,
Sum(number) AS Number
INTO #tempac
FROM [ABI_Reports].speccomm.tbl_apc_spell_psir a
JOIN abi_reports.dbo.reference_lkup_month m
ON m.finmonno = a.finmonno
WHERE orgcode_codeofcomm IN (SELECT *
FROM
dbo.Fn_splitstrings(@p_OrgCommCode, N','))
--faster function used
AND aimtc_pctcomml_id IN (SELECT *
FROM
dbo.Fn_splitstrings(@p_CommCode, N','))
--faster function used
GROUP BY finyr,
CASE
WHEN finyr = @Previous2FY THEN 'Min'
ELSE finyr
END,
a.finmonno,
m.finmonname,
[activitytypeorder],
CASE
WHEN activitytypeorder = 5
AND aimtc_type_code = '3' THEN 'Elective Day Case'
WHEN activitytypeorder = 3 THEN 'New Outpatient'
ELSE activitytype
END,
ha_code,
nhs_e_contract_id,
age_flag
UNION ALL
/*******add in outpatient data*************/
SELECT finyr,
CASE
WHEN finyr = @Previous2FY THEN 'Min'
ELSE finyr
END AS MinYr,
o.finmonno,
m.finmonname,
[activitytypeorder],
CASE
WHEN activitytypeorder = 5
AND aimtc_type_code = '3' THEN 'Elective Day Case'
WHEN activitytypeorder = 3 THEN 'New Outpatient'
ELSE activitytype
END AS ActivityType,
ha_code,
nhs_e_contract_id,
age_flag,
Sum(number) AS Number
FROM [ABI_Reports].speccomm.tbl_opa_attend_psir o
JOIN abi_reports.dbo.reference_lkup_month m
ON m.finmonno = o.finmonno
WHERE orgcode_codeofcomm IN (SELECT *
FROM
dbo.Fn_splitstrings(@p_OrgCommCode, N','))
--faster function used
AND aimtc_pctcomml_id IN (SELECT *
FROM
dbo.Fn_splitstrings(@p_CommCode, N','))
--faster function used
GROUP BY finyr,
CASE
WHEN finyr = @Previous2FY THEN 'Min'
ELSE finyr
END,
o.finmonno,
m.finmonname,
[activitytypeorder],
CASE
WHEN activitytypeorder = 5
AND aimtc_type_code = '3' THEN 'Elective Day Case'
WHEN activitytypeorder = 3 THEN 'New Outpatient'
ELSE activitytype
END,
ha_code,
nhs_e_contract_id,
age_flag
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype--,ActivityType2
,
ha_code,
nhs_e_contract_id,
age_flag,
Sum(number) AS Number
FROM (SELECT *
FROM #tempac
UNION ALL
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
'Q64',
nhs_e_contract_id,
age_flag,
0 AS Number
FROM #tempac
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag
UNION ALL
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
'Q65',
nhs_e_contract_id,
age_flag,
0 AS Number
FROM #tempac
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag
UNION ALL
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
'Q66',
nhs_e_contract_id,
age_flag,
0 AS Number
FROM #tempac
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag
UNION ALL
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
'Other',
nhs_e_contract_id,
age_flag,
0 AS Number
FROM #tempac
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag) t
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
ha_code,
nhs_e_contract_id,
age_flag
ORDER BY age_flag,
ha_code,
nhs_e_contract_id,
activitytype,
activitytypeorder
--OPTION (RECOMPILE);
OPTION (MERGE JOIN)
--knocks a second or 2 off processing time if not using OPTION (RECOMPILE)
END
--DROP TABLE #tempac --Moved to start of procedure
I have tried using a table variable instead of the temp table (and added an identity column), but that slows down the execution. The columns are all pretty narrow, so can't change much there. I've also added indexes to the underlying tables which has halved the execution time.
Anything else I could try?
Cheers.
Tim
March 25, 2014 at 6:22 am
Please post table definitions, index definitions and execution plan as per http://www.sqlservercentral.com/articles/SQLServerCentral/66909/
First thing, remove the join hints.
What's the definition of the string splitting function you're using? If it uses a cursor or while loop, look up the Delimited8k_split function to replace it (on this site)
Do you know what read uncommitted does? Do you know it's side effects? Are your users happy with intermittently incorrect data?
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
March 25, 2014 at 7:18 am
Sorry, I should have checked that out.
Please note each of the main tables contain approx. 450,000 each rows and the query returns 3350 rows. I can see that there are 2 row count lazy spools in execution plan with a cost of 29% & 38%, but I'm not sure that's a bad thing or not.
The query alsways takes 8-11 seconds, but my optimisation skills are pretty raw, so any pointers in the right direction would be great.
1st Table defintion:
USE [ABI_Reports]
GO
/****** Object: Table [SPECCOMM].[tbl_APC_SPELL_PSIR] Script Date: 03/25/2014 12:30:23 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [SPECCOMM].[tbl_APC_SPELL_PSIR](
[PK_Id] [bigint] IDENTITY(1,1) NOT NULL,
[NPOCCode] [varchar](15) NULL,
[ServiceLineCode] [varchar](15) NULL,
[Category] [varchar](15) NULL,
[ActivityTypeOrder] [int] NULL,
[ActivityType] [varchar](50) NULL,
[AIMTC_PCTCOMML] [varchar](5) NULL,
[AIMTC_PCTCOMML_ID] [int] NULL,
[OrgCode_CodeOfComm] [varchar](10) NULL,
[OrgCode_NameOfComm] [varchar](20) NULL,
[AIMTC_Type] [varchar](20) NULL,
[AIMTC_Type_Code] [varchar](5) NULL,
[AIMTC_PracticeCodeOfRegisteredGP] [varchar](10) NULL,
[Age_flag] [varchar](2) NULL,
[nhs_e_contract_flag] [varchar](50) NULL,
[nhs_e_contract_ID] [int] NULL,
[aimtc_current_spellhrg] [varchar](10) NULL,
[TreatmentFunctionCode] [varchar](5) NULL,
[aimtc_specialty_trend] [varchar](5) NULL,
[ProviderCode] [varchar](10) NULL,
[Provider] [varchar](150) NULL,
[ProviderOrder] [int] NULL,
[SourceType] [varchar](50) NULL,
[FinYr] [varchar](10) NULL,
[DateYear] [int] NULL,
[FinMonNo] [int] NULL,
[Number] [int] NULL,
[LOS] [int] NULL,
[LOS14] [int] NULL,
[LOS50] [int] NULL,
[PlProcNCO] [int] NULL,
[EmerLOS0] [int] NULL,
[EmerNoProc] [int] NULL,
[XBeddays] [int] NULL,
[SSEmergInd] [int] NULL,
[HSCIC_SpellNPOC] [varchar](50) NULL,
[HSCIC_SpellServiceLine] [varchar](50) NULL,
[HSCIC_FCENPOC] [varchar](50) NULL,
[HSCIC_FCEServiceline] [varchar](50) NULL,
[HSCIC_ActivityServiceLine] [varchar](50) NULL,
[HSCIC_ActivityNPOC] [varchar](50) NULL,
[HSCIC_SpellNPOC_UHB] [varchar](50) NULL,
[HSCIC_SpellServiceLine_UHB] [varchar](50) NULL,
[ERROR1] [varchar](255) NULL,
[ERROR2] [varchar](255) NULL,
[ERROR3] [varchar](255) NULL,
[HA_Code] [varchar](5) NULL,
[LAT_NameProv] [varchar](20) NULL,
[LOSGroup] [varchar](15) NULL,
CONSTRAINT [PK_tbl_APC_SPELL_PSIR] PRIMARY KEY CLUSTERED
(
[PK_Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 80) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
2nd Table defintion:
USE [ABI_Reports]
GO
/****** Object: Table [SPECCOMM].[tbl_OPA_ATTEND_PSIR] Script Date: 03/25/2014 12:32:04 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [SPECCOMM].[tbl_OPA_ATTEND_PSIR](
[PK_Id] [bigint] IDENTITY(1,1) NOT NULL,
[NPOCCode] [varchar](15) NULL,
[ServiceLineCode] [varchar](15) NULL,
[Category] [varchar](15) NULL,
[ActivityTypeOrder] [int] NULL,
[ActivityType] [varchar](50) NULL,
[AIMTC_PCTCOMML] [varchar](5) NULL,
[AIMTC_PCTCOMML_ID] [int] NULL,
[OrgCode_CodeOfComm] [varchar](10) NULL,
[OrgCode_NameOfComm] [varchar](20) NULL,
[AIMTC_Type] [varchar](20) NULL,
[AIMTC_Type_Code] [varchar](5) NULL,
[AIMTC_PracticeCodeOfRegisteredGP] [varchar](10) NULL,
[Age_flag] [varchar](2) NULL,
[NHS_E_Contract_Flag] [varchar](50) NULL,
[NHS_E_Contract_ID] [int] NULL,
[aimtc_current_spellhrg] [varchar](10) NULL,
[AIMTC_Future_HRG] [varchar](10) NULL,
[TreatmentFunctionCode] [varchar](5) NULL,
[AIMTC_Specialty_Trend] [varchar](5) NULL,
[ProviderCode] [varchar](10) NULL,
[Provider] [varchar](150) NULL,
[ProviderOrder] [int] NULL,
[SourceType] [varchar](50) NULL,
[FinYr] [varchar](10) NULL,
[DateYear] [int] NULL,
[FinMonNo] [int] NULL,
[Number] [int] NULL,
[HSCIC_NPoc] [varchar](50) NULL,
[HSCIC_Serviceline] [varchar](50) NULL,
[SQL_NPoC] [varchar](max) NULL,
[SQL_ServiceLine] [varchar](max) NULL,
[Error Message] [varchar](140) NULL,
[HA_Code] [varchar](5) NULL,
[LAT_NameProv] [varchar](20) NULL,
CONSTRAINT [PK_tbl_OPA_ATTEND_PSIR] PRIMARY KEY CLUSTERED
(
[PK_Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 80) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
Lookup table definition:
USE [ABI_Reports]
GO
/****** Object: Table [SPECCOMM].[tbl_NPoCLkUp] Script Date: 03/25/2014 12:42:39 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [SPECCOMM].[tbl_NPoCLkUp](
[NPoCCode] [varchar](255) NOT NULL,
[NPoC Category] [varchar](255) NULL,
CONSTRAINT [PK_tbl_NPoCLkUp] PRIMARY KEY CLUSTERED
(
[NPoCCode] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
Function definition (I think I pinched this from Adam Mechanic's site - or somesuch):
USE [ABI_Reports]
GO
/****** Object: UserDefinedFunction [dbo].[fn_SplitStrings] Script Date: 03/25/2014 12:39:31 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE FUNCTION [dbo].[fn_SplitStrings]
(
@List NVARCHAR(MAX),
@Delimiter NVARCHAR(255)
)
RETURNS TABLE
WITH SCHEMABINDING AS
RETURN
WITH E1(N) AS ( SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1
UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1
UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1),
E2(N) AS (SELECT 1 FROM E1 a, E1 b),
E4(N) AS (SELECT 1 FROM E2 a, E2 b),
E42(N) AS (SELECT 1 FROM E4 a, E2 b),
cteTally(N) AS (SELECT 0 UNION ALL SELECT TOP (DATALENGTH(ISNULL(@List,1)))
ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) FROM E42),
cteStart(N1) AS (SELECT t.N+1 FROM cteTally t
WHERE (SUBSTRING(@List,t.N,1) = @Delimiter OR t.N = 0))
SELECT Item = SUBSTRING(@List, s.N1, ISNULL(NULLIF(CHARINDEX(@Delimiter,@List,s.N1),0)-s.N1,8000))
FROM cteStart s;
GO
Cheers. T
March 25, 2014 at 7:23 am
Right, the previous post was getting a little long, so just to clarify -
I don't mind dirty reads at all.
I've removed the join hint - makes no difference, although it did initially appear to knock a couple of secs off the processing time.
T
March 25, 2014 at 7:25 am
The table hints and read uncommitted should go away as Gail suggested.
Your function looks very similar to the DelimitedSplit8K with two MAJOR exceptions. You changed the delimiter to nvarchar(255) from a char(1). The other difference is you changed the input string from varchar(8000) to nvarchar(MAX). You need to go back and read the article again. This function does not play well when you change the input string to MAX. Right at the top of the article in big bold letters you made the two modifications to this that cause it run a LOT slower.
http://www.sqlservercentral.com/articles/Tally+Table/72993/[/url]
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
March 25, 2014 at 8:17 am
You could replace four of those UNIONs with one:
SELECT *
FROM #tempac
UNION ALL
SELECT e.*
FROM ( -- e
SELECT
d.finyr,
d.minyr,
d.finmonno,
d.finmonname,
d.[activitytypeorder],
d.activitytype,
m.ha_code,
d.nhs_e_contract_id,
d.age_flag,
d.Number
FROM ( -- d
SELECT finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag,
0 AS Number
FROM #tempac
GROUP BY finyr,
minyr,
finmonno,
finmonname,
[activitytypeorder],
activitytype,
nhs_e_contract_id,
age_flag
) d
CROSS JOIN (SELECT ha_code = 'Q64' UNION ALL SELECT 'Q65' UNION ALL SELECT 'Q66' UNION ALL SELECT 'Other') m
) e
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
March 26, 2014 at 6:28 am
Try to replace in the "where" any function for avoid table scan.
March 26, 2014 at 6:52 am
Many thanks for all the contributions - I know there's a lot to read through on this particular enquiry.
I've rejigged the split list function as suggested and ditched the union statements too. These did not result in speedier execution.
Plus I've also reduced the length of the parameters being declared to the minimum that works. The latter gave me a 2 second performance improvement.
I've not yet had a chance to look at the table scan as just suggested, but will do so when I examine the adjusted execution plan.
I shall continue to persevere with this as it's still too slow and shall report back with any improvements.
Any further contributions still welcomed!
Cheers.
Tim
March 26, 2014 at 7:11 am
Sean Lange (3/25/2014)
The table hints and read uncommitted should go away as Gail suggested.Your function looks very similar to the DelimitedSplit8K with two MAJOR exceptions. You changed the delimiter to nvarchar(255) from a char(1). The other difference is you changed the input string from varchar(8000) to nvarchar(MAX). You need to go back and read the article again. This function does not play well when you change the input string to MAX. Right at the top of the article in big bold letters you made the two modifications to this that cause it run a LOT slower.
http://www.sqlservercentral.com/articles/Tally+Table/72993/[/url]
The function the op posted looks like one of Aaron's recent attempts to make a multicharacter delimiter split function. Just changing the input parameter to the MAX datatype slows it down by 100%.
--Jeff Moden
Change is inevitable... Change for the better is not.
March 26, 2014 at 7:31 am
Have you tried resolving out the tables from the strings? Something like this:
SELECT * -- change to column name
INTO #OrgCommCode
FROM dbo.Fn_splitstrings(@p_OrgCommCode, ',')
SELECT * -- change to column name
INTO #CommCode
FROM dbo.Fn_splitstrings(@p_CommCode, ',')
WHERE orgcode_codeofcomm IN (SELECT * FROM #OrgCommCode) -- change * to column name
AND aimtc_pctcomml_id IN (SELECT * FROM #CommCode) -- change * to column name
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
March 26, 2014 at 9:42 am
Thanks Jeff - that's true. I actually got it before I checked yours out as per Sean's earlier post.
Chris, I'll give your suggestion a try.
Cheers T
March 27, 2014 at 6:07 am
Hi SSC
Lookup definition is as follows:
USE [ABI_Reports]
GO
/****** Object: Table [dbo].[REFERENCE_LKUP_Month] Script Date: 03/27/2014 11:37:38 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[REFERENCE_LKUP_Month](
[FinMonNo] [int] NOT NULL,
[FinMonName] [varchar](3) NULL,
[FinMonNameFull] [varchar](100) NULL,
[FinQtrNo] [int] NULL,
[FinQtrName] [varchar](10) NULL,
[CalQtrNo] [int] NULL,
[CalQtrName] [varchar](10) NULL,
CONSTRAINT [PK_REFERENCE_LKUP_Month] PRIMARY KEY CLUSTERED
(
[FinMonNo] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
Yep, rebuilt all the stats and indexes too.
I've re-written the query several times with a table variable instead of a temp table and and also replaced the union statements as per an earlier suggestion. I've corrected the split list function as suggested too. There has been a minot improvement overall, but still taking 7 seconds, which might be bearable if I didn't need to use the SP in a SSRS report which takes much longer.
I'm now going to check if I can do anything with the underlying table structure.
Cheers T
March 27, 2014 at 6:39 am
Can you post the actual plan for the most recent version?
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
Viewing 13 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply