July 18, 2018 at 7:37 am
I am a beginner in Microsoft SQL and I’ve come across this stored procedure at work. I sort of understand what the programmer was trying to do but I would like to know if this is a good example of a stored proc.
Ideally, I would like to know:
1. If this stored procedure is a good example.
2. If anything, what is generally wrong with it
3. How it could be improved.
This stored proc has been in use as is for about two years so it’s not something that I’m going to go and change, nobody is going to touch it but since it looks really complex I was wondering if this is how a similar sp should look like.
USE [Database]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [Schema].[Table]
@DateFrom DATE,
@DateTo DATE,
@BranchCode INT
AS
BEGIN
DECLARE @DateFromVAR AS DATE =@DateFrom
DECLARE @DateToVAR AS DATE =@DateTo
DECLARE @BranchCodeVAR AS INT =@BranchCode
SELECT CAST(EIV2.EntityItemValue AS INT) AS EmployeeBranch,
SUM(TotalPaidPrem) AS TotalPremiumPaid,
SUM(TotalPaidPrem)*0.5/100 AS Commission
FROM (
SELECT PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,SUM(PaidMonths.PaidMonths)TotalPaidMonths,MAX(PaidPrem.paidPremium) TotalPaidPrem,paidMonths.CommencementDate
FROM (
SELECT SUM(Premium) AS paidPremium,FT.PolicyNumber,FT.DueDate,FT.RegistrationDate,MAX(FT.PPremiumDebitPk) AS MAXPPremiumDebitPk
FROM(
SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.PPremiumDebitPk,FT.RegistrationDate
FROM [DataWarehouse].[schema].[Table] FT
INNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumber
INNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0
LEFT OUTER JOIN Table AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0
INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the payments
INNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'
INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'
LEFT OUTER JOIN Table AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1
WHERE
FT.PremiumType NOT IN (200, 201, 205, 206, 249, 250)
AND FT.RegistrationDate BETWEEN @DateFromVAR AND @DateToVAR
)FT
GROUP BY FT.PolicyNumber,FT.DueDate,FT.RegistrationDate--,FT.PPremiumDebitPk
)PaidPrem
LEFT OUTER JOIN (
SELECT PT.PolicyNumber,PT.DueDate,MAX(PT.RegistrationDate) AS RegistrationDate,CommencementDate,MAX(PT.PPremiumDebitPk)AS PPremiumDebitPk,ISNULL(12/CPA.PaymentFrequency,0) AS PaidMonths
FROM (
SELECT PolicyNumber,
ContributorType,
ContributorLayer,
DueDate,
FT.CommencementDate,
MAX(FT.RegistrationDate) AS RegistrationDate,
MAX(FT.PPremiumDebitPk)AS PPremiumDebitPk
FROM (SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.ContributorType,FT.PPremiumDebitPk,LayerNumber AS ContributorLayer,PM.CommencementDate,FT.RegistrationDate
FROM [DataWarehouse].[Schema].[FinancialTransaction] FT
INNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumber
INNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0
LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0
INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the payments
INNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'
INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'
LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1
WHERE -- Exclude: Excess Premium, Excess Premium, Excess Refund Payment, Excess Refund Payment (UL), Refund to Suspense (UL), Refund to Suspense
FT.PremiumType NOT IN (200, 201, 205, 206, 249, 250)
AND FT.RegistrationDate BETWEEN PM.CommencementDate AND @DateToVAR
) FT
GROUP BY PolicyNumber,
ContributorType,
ContributorLayer,
DueDate,
FT.CommencementDate
)PT
INNER JOIN DataWarehouse.Schema.Table CC
ON PT.PolicyNumber=CC.PolicyNumber
AND PT.ContributorType=CC.ContributorType
AND PT.ContributorLayer=CC.LayerNumber
AND PT.RegistrationDate BETWEEN CC.OpeningRegistrationDate AND CC.ClosingRegistrationDate-1
INNER JOIN [DataWarehouse].[Schema].[Table] CPA
ON CC.ContributorId=CPA.ClientNumber
AND PT.RegistrationDate BETWEEN CPA.OpeningRegistrationDate AND CPA.ClosingRegistrationDate-1
AND CC.PaymentArrangementId=CPA.ArrangementId
GROUP BY PT.PolicyNumber,PT.DueDate,CommencementDate,ISNULL(12/CPA.PaymentFrequency,0)
)PaidMonths
--calculate paid months that registration and due date of payments in period is grater than other payments registration and due date
--this done to avoid the reversal values in case of the payment frequency modification. example 10037529 for period 1/1/2018-31/3/2018
ON (PaidPrem.PolicyNumber=PaidMonths.PolicyNumber
AND PaidPrem.RegistrationDate>=PaidMonths.RegistrationDate
AND PaidPrem.DueDate>PaidMonths.DueDate
AND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPk
AND PaidPrem.paidPremium>0)
OR (PaidPrem.PolicyNumber=PaidMonths.PolicyNumber
AND PaidPrem.RegistrationDate>PaidMonths.RegistrationDate
AND PaidPrem.DueDate>PaidMonths.DueDate
AND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPk
AND PaidPrem.paidPremium<0)
GROUP BY PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,PaidMonths.CommencementDate
--HAVING MAX(PaidPrem.paidPremium)>0
)X
LEFT OUTER JOIN Table PCD
ON X.PolicyNumber = PCD.PolicyNumber
LEFT OUTER JOIN Table SCC
ON PCD.SourceCode = SCC.SourceCodeId
LEFT OUTER JOIN Table MAD
ON PCD.AgentNumber=MAD.AgentNumber
AND PCD.InsuranceCompany=MAD.InsuranceCompany
LEFT OUTER JOIN Table GM1
ON MAD.GroupCode=GM1.GroupCode
LEFT OUTER JOIN Table EIV2
ON EIV2.EntityNumber = PCD.PolicyNumber
-- AND EIV2.EntitySubNumber = PMA.AgentLayerNumber
AND EIV2.EntityItemType = 5
AND EIV2.EntityItemSubType = 1
AND EIV2.EntityItemId = 2
AND EIV2.ClosingStatus=0
WHERE X.TotalPaidMonths>=12
AND AgentBusinessTypeSourceCd = '02'
AND PCD.InsuranceCompany<>2
AND (CAST(EIV2.EntityItemValue AS INT)=@BranchCodeVAR OR @BranchCodeVAR=0)
GROUP BY CAST(EIV2.EntityItemValue AS INT)
ORDER BY CAST(EIV2.EntityItemValue AS INT)
END
* Ignore the comments
I’m afraid I don’t have an execution plan so a really general comment or general feedback would be more than enough.
Thanks
July 18, 2018 at 7:56 am
This is a very complicated procedure and generally should not be the norm for most queries. I see a series of issues.
First thing I see is several DISTINCT operations. These aggregations can frequently cause poor performance. You have functions on columns i the ON/WHERE/HAVING clause: CC.ClosingRegistrationDate-1, CAST(EIV2.EntityItemValue AS INT)=@BranchCodeVAR,
These will lead to very poor performance. Depending on the function, work to eliminate these from the code.
I'd also get rid of the ISNULL in the GROUP BY. Pretty sure that's going to cause problems: ISNULL(12/CPA.PaymentFrequency,0).
I'd have to really drill down on it to see if all those SELECT FROM.. SELECT FROM... SELECT FROM... are necessary or not. There's nothing inherently wrong with the nesting approach, but that much nesting makes me suspicious.
You also might benefit from using EXCEPT instead of NOT IN. That will require some testing.
That's what I see on a first pass.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
July 18, 2018 at 8:20 am
Couple of things that I observed in the stored procedure:
1. There are a DISTINCT clauses in two places. This should be avoided as it is expensive and if your query is using correct joining conditions, it should not be needed in an inner query,
2. The table subscript AD and AAD has been added with a LEFT OUTER JOIN in multiple occasions, but these tables are not being used in the query at all. In that case, you can remove this table entirely from the joining operation.
July 18, 2018 at 8:26 am
debasis.yours - Wednesday, July 18, 2018 8:20 AMCouple of things that I observed in the stored procedure:
1. There are a DISTINCT clauses in two places. This should be avoided as it is expensive and if your query is using correct joining conditions, it should not be needed in an inner query,
2. The table subscript AD and AAD has been added with a LEFT OUTER JOIN in multiple occasions, but these tables are not being used in the query at all. In that case, you can remove this table entirely from the joining operation.
Oooh, nice catch. I didn't even see that. Yeah, the last thing you want to do is make extra work for the optimizer, and with a query this complex, it increases the likelihood that the simplification process doesn't spot the fact that these tables aren't actually used, which means there's the possibility of unnecessary processing going on (you could see this with the execution plan).
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
July 18, 2018 at 9:56 am
Thought I would take a look but SQL Prompt can't reformat the code and trying to do it by hand to make more readable just isn't viable at this time. I really wish people posting code would take the time to format it to be more human readable.
July 18, 2018 at 10:35 am
ìn case anyone wishes to look at it.
Its not a complex proc - its just a rather complex query
alter procedure [Schema].
@DateFrom date
, @DateTo date
, @BranchCode int
as
begin
declare @DateFromVar as date = @DateFrom
declare @DateToVar as date = @DateTo
declare @BranchCodeVar as int = @BranchCode
select cast(eiv2.EntityItemValue as int) as employeebranch
, sum(TotalPaidPrem) as totalpremiumpaid
, sum(TotalPaidPrem) * 0.5 / 100 as commission
from (select paidprem.maxppremiumdebitpk
, paidprem.PolicyNumber
, sum(paidmonths.paidmonths) totalpaidmonths
, max(paidprem.paidpremium) totalpaidprem
, paidmonths.CommencementDate
from (select sum(Premium) as paidpremium
, ft.PolicyNumber
, ft.DueDate
, ft.registrationdate
, max(ft.ppremiumdebitpk) as maxppremiumdebitpk
from (select distinct ft.Premium
, ft.PolicyNumber
, ft.DueDate
, ft.ppremiumdebitpk
, ft.registrationdate
from [DataWarehouse].[schema].
ft
inner join [DataWarehouse].[schema].
pm
on pm.PolicyNumber = ft.PolicyNumber
inner join [DataWarehouse].[schema].
pod
on pod.PolicyNumber = ft.PolicyNumber
and pod.ClosingStatus = 0
left outer join
ad
on ad.AgentNumber = ft.AgentNumber
and ad.ClosingStatus = 0
inner join [DataWarehouse].[schema].
pt
on pt.PremiumTypeNumber = ft.PremiumType
and pt.PaymentFlag = 1 --Get only the payments
inner join [DataWarehouse].[schema].
gp1
on gp1.ParameterId = ft.ContributorType
and gp1.ParameterName = 'ContributorType'
inner join [DataWarehouse].[schema].
gp2
on gp2.ParameterId = pod.ProductNumber
and gp2.ParameterAdditionalId = pod.ProductVersion
and gp2.ParameterName = 'Product'
left outer join
aad
on aad.OpeningReference = ft.OpeningReference
and aad.ActivityKey = ft.PolicyNumber
and aad.ActivityKeyType = 1
and aad.ActivitySequence = 1
where ft.PremiumType not in (200, 201, 205, 206, 249, 250)
and ft.registrationdate between @DateFromVar and @DateToVar
) ft
group by ft.PolicyNumber
, ft.DueDate
, ft.registrationdate
-- , FT.PPremiumDebitPk
) paidprem
left outer join (select pt.PolicyNumber
, pt.DueDate
, max(pt.registrationdate) as registrationdate
, CommencementDate
, max(pt.ppremiumdebitpk) as ppremiumdebitpk
, isnull(12 / cpa.PaymentFrequency, 0) as paidmonths
from (select PolicyNumber
, ContributorType
, ContributorLayer
, DueDate
, ft.CommencementDate
, max(ft.registrationdate) as registrationdate
, max(ft.ppremiumdebitpk) as ppremiumdebitpk
from (select distinct ft.Premium
, ft.PolicyNumber
, ft.DueDate
, ft.ContributorType
, ft.ppremiumdebitpk
, LayerNumber as contributorlayer
, pm.CommencementDate
, ft.registrationdate
from [DataWarehouse].[Schema].[FinancialTransaction] ft
inner join [DataWarehouse].[schema].
pm
on pm.PolicyNumber = ft.PolicyNumber
inner join [DataWarehouse].[schema].
pod
on pod.PolicyNumber = ft.PolicyNumber
and pod.ClosingStatus = 0
left outer join [DataWarehouse].[schema].
ad
on ad.AgentNumber = ft.AgentNumber
and ad.ClosingStatus = 0
inner join [DataWarehouse].[schema].
pt
on pt.PremiumTypeNumber = ft.PremiumType
and pt.PaymentFlag = 1 --Get only the payments
inner join [DataWarehouse].[schema].
gp1
on gp1.ParameterId = ft.ContributorType
and gp1.ParameterName = 'ContributorType'
inner join [DataWarehouse].[schema].
gp2
on gp2.ParameterId = pod.ProductNumber
and gp2.ParameterAdditionalId = pod.ProductVersion
and gp2.ParameterName = 'Product'
left outer join [DataWarehouse].[schema].
aad
on aad.OpeningReference = ft.OpeningReference
and aad.ActivityKey = ft.PolicyNumber
and aad.ActivityKeyType = 1
and aad.ActivitySequence = 1
where -- Exclude: Excess Premium , Excess Premium , Excess Refund Payment , Excess Refund Payment (UL) , Refund to Suspense (UL) , Refund to Suspense
ft.PremiumType not in (200, 201, 205, 206, 249, 250)
and ft.registrationdate between pm.CommencementDate and @DateToVar
) ft
group by PolicyNumber
, ContributorType
, ContributorLayer
, DueDate
, ft.CommencementDate
) pt
inner join DataWarehouse.Schema.table cc
on pt.PolicyNumber = cc.PolicyNumber
and pt.ContributorType = cc.ContributorType
and pt.contributorlayer = cc.LayerNumber
and pt.registrationdate between cc.OpeningRegistrationDate and cc.ClosingRegistrationDate - 1
inner join [DataWarehouse].[Schema].
cpa
on cc.ContributorId = cpa.ClientNumber
and pt.registrationdate between cpa.OpeningRegistrationDate and cpa.ClosingRegistrationDate - 1
and cc.PaymentArrangementId = cpa.ArrangementId
group by pt.PolicyNumber
, pt.DueDate
, CommencementDate
, isnull(12 / cpa.PaymentFrequency, 0)
) paidmonths
--calculate paid months that registration and due date of payments in period is grater than other payments registration and due date
--this done to avoid the reversal values in case of the payment frequency modification. example 10037529 for period 1/1/2018-31/3/2018
on (paidprem.PolicyNumber = paidmonths.PolicyNumber
and paidprem.registrationdate >= paidmonths.registrationdate
and paidprem.DueDate > paidmonths.DueDate
and paidprem.maxppremiumdebitpk >= paidmonths.ppremiumdebitpk
and paidprem.paidpremium > 0)
or (paidprem.PolicyNumber = paidmonths.PolicyNumber
and paidprem.registrationdate > paidmonths.registrationdate
and paidprem.DueDate > paidmonths.DueDate
and paidprem.maxppremiumdebitpk >= paidmonths.ppremiumdebitpk
and paidprem.paidpremium < 0)
group by paidprem.maxppremiumdebitpk
, paidprem.PolicyNumber
, paidmonths.CommencementDate
--HAVING MAX(PaidPrem.paidPremium)>0
) x
left outer join
pcd
on x.PolicyNumber = pcd.PolicyNumber
left outer join
scc
on pcd.SourceCode = scc.SourceCodeId
left outer join
mad
on pcd.AgentNumber = mad.AgentNumber
and pcd.InsuranceCompany = mad.InsuranceCompany
left outer join
gm1
on mad.GroupCode = gm1.GroupCode
left outer join
eiv2
on eiv2.EntityNumber = pcd.PolicyNumber
-- AND EIV2.EntitySubNumber = PMA.AgentLayerNumber
and eiv2.EntityItemType = 5
and eiv2.EntityItemSubType = 1
and eiv2.EntityItemId = 2
and eiv2.ClosingStatus = 0
where x.totalpaidmonths >= 12
and AgentBusinessTypeSourceCd = '02'
and pcd.InsuranceCompany <> 2
and (cast(eiv2.EntityItemValue as int) = @BranchCodeVar
or @BranchCodeVar = 0)
group by cast(eiv2.EntityItemValue as int)
order by cast(eiv2.EntityItemValue as int)
end
July 18, 2018 at 10:49 am
NikosV - Wednesday, July 18, 2018 7:37 AMI am a beginner in Microsoft SQL and I’ve come across this stored procedure at work. I sort of understand what the programmer was trying to do but I would like to know if this is a good example of a stored proc. Ideally, I would like to know:1. If this stored procedure is a good example. 2. If anything, what is generally wrong with it3. How it could be improved. This stored proc has been in use as is for about two years so it’s not something that I’m going to go and change, nobody is going to touch it but since it looks really complex I was wondering if this is how a similar sp should look like. USE [Database]GOSET ANSI_NULLS ONGOSET QUOTED_IDENTIFIER ONGOALTER PROCEDURE [Schema].[Table] @DateFrom DATE, @DateTo DATE,@BranchCode INTASBEGINDECLARE @DateFromVAR AS DATE =@DateFromDECLARE @DateToVAR AS DATE =@DateToDECLARE @BranchCodeVAR AS INT =@BranchCodeSELECT CAST(EIV2.EntityItemValue AS INT) AS EmployeeBranch, SUM(TotalPaidPrem) AS TotalPremiumPaid, SUM(TotalPaidPrem)*0.5/100 AS Commission FROM (SELECT PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,SUM(PaidMonths.PaidMonths)TotalPaidMonths,MAX(PaidPrem.paidPremium) TotalPaidPrem,paidMonths.CommencementDateFROM (SELECT SUM(Premium) AS paidPremium,FT.PolicyNumber,FT.DueDate,FT.RegistrationDate,MAX(FT.PPremiumDebitPk) AS MAXPPremiumDebitPkFROM(SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.PPremiumDebitPk,FT.RegistrationDateFROM [DataWarehouse].[schema].[Table] FTINNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumberINNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0LEFT OUTER JOIN Table AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the paymentsINNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'LEFT OUTER JOIN Table AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1WHEREFT.PremiumType NOT IN (200, 201, 205, 206, 249, 250) AND FT.RegistrationDate BETWEEN @DateFromVAR AND @DateToVAR)FTGROUP BY FT.PolicyNumber,FT.DueDate,FT.RegistrationDate--,FT.PPremiumDebitPk)PaidPremLEFT OUTER JOIN (SELECT PT.PolicyNumber,PT.DueDate,MAX(PT.RegistrationDate) AS RegistrationDate,CommencementDate,MAX(PT.PPremiumDebitPk)AS PPremiumDebitPk,ISNULL(12/CPA.PaymentFrequency,0) AS PaidMonthsFROM (SELECT PolicyNumber, ContributorType, ContributorLayer,DueDate,FT.CommencementDate,MAX(FT.RegistrationDate) AS RegistrationDate,MAX(FT.PPremiumDebitPk)AS PPremiumDebitPkFROM (SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.ContributorType,FT.PPremiumDebitPk,LayerNumber AS ContributorLayer,PM.CommencementDate,FT.RegistrationDateFROM [DataWarehouse].[Schema].[FinancialTransaction] FTINNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumberINNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the paymentsINNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1WHERE -- Exclude: Excess Premium, Excess Premium, Excess Refund Payment, Excess Refund Payment (UL), Refund to Suspense (UL), Refund to SuspenseFT.PremiumType NOT IN (200, 201, 205, 206, 249, 250) AND FT.RegistrationDate BETWEEN PM.CommencementDate AND @DateToVAR) FTGROUP BY PolicyNumber, ContributorType, ContributorLayer,DueDate,FT.CommencementDate)PTINNER JOIN DataWarehouse.Schema.Table CCON PT.PolicyNumber=CC.PolicyNumberAND PT.ContributorType=CC.ContributorTypeAND PT.ContributorLayer=CC.LayerNumberAND PT.RegistrationDate BETWEEN CC.OpeningRegistrationDate AND CC.ClosingRegistrationDate-1INNER JOIN [DataWarehouse].[Schema].[Table] CPAON CC.ContributorId=CPA.ClientNumberAND PT.RegistrationDate BETWEEN CPA.OpeningRegistrationDate AND CPA.ClosingRegistrationDate-1AND CC.PaymentArrangementId=CPA.ArrangementIdGROUP BY PT.PolicyNumber,PT.DueDate,CommencementDate,ISNULL(12/CPA.PaymentFrequency,0))PaidMonths--calculate paid months that registration and due date of payments in period is grater than other payments registration and due date--this done to avoid the reversal values in case of the payment frequency modification. example 10037529 for period 1/1/2018-31/3/2018ON (PaidPrem.PolicyNumber=PaidMonths.PolicyNumberAND PaidPrem.RegistrationDate>=PaidMonths.RegistrationDateAND PaidPrem.DueDate>PaidMonths.DueDateAND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPkAND PaidPrem.paidPremium>0)OR (PaidPrem.PolicyNumber=PaidMonths.PolicyNumberAND PaidPrem.RegistrationDate>PaidMonths.RegistrationDateAND PaidPrem.DueDate>PaidMonths.DueDateAND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPkAND PaidPrem.paidPremium<0)GROUP BY PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,PaidMonths.CommencementDate--HAVING MAX(PaidPrem.paidPremium)>0)XLEFT OUTER JOIN Table PCDON X.PolicyNumber = PCD.PolicyNumberLEFT OUTER JOIN Table SCCON PCD.SourceCode = SCC.SourceCodeIdLEFT OUTER JOIN Table MADON PCD.AgentNumber=MAD.AgentNumberAND PCD.InsuranceCompany=MAD.InsuranceCompanyLEFT OUTER JOIN Table GM1ON MAD.GroupCode=GM1.GroupCodeLEFT OUTER JOIN Table EIV2 ON EIV2.EntityNumber = PCD.PolicyNumber -- AND EIV2.EntitySubNumber = PMA.AgentLayerNumber AND EIV2.EntityItemType = 5 AND EIV2.EntityItemSubType = 1 AND EIV2.EntityItemId = 2 AND EIV2.ClosingStatus=0WHERE X.TotalPaidMonths>=12AND AgentBusinessTypeSourceCd = '02'AND PCD.InsuranceCompany<>2AND (CAST(EIV2.EntityItemValue AS INT)=@BranchCodeVAR OR @BranchCodeVAR=0)GROUP BY CAST(EIV2.EntityItemValue AS INT)ORDER BY CAST(EIV2.EntityItemValue AS INT)END* Ignore the commentsI’m afraid I don’t have an execution plan so a really general comment or general feedback would be more than enough. Thanks
Here's a "cleaned up" version of this query. There are several things that could be done to improve it. First, formatting more like what I'm posting. Second, don't go quite so far with anonymising table names to the exact same table name. This makes it MUCH harder to take any modified query elements that are demonstrated to you and integrate them into your actual query. That's one of the changes I made - to adapt the anonymised table name to it's alias, so at least those working on the query realize they are dealing with a genuinely different table name. I may have missed some, so you may need to fix what I did. Also, if you change a table name to the same value as another of your table name changes, you might very well mislead someone trying to help you, and in a rather bad way. This is one habit you really MUST stop. It makes getting help one heck of a lot harder.USE [Database];
GO
SET ANSI_NULLS ON;
GO
SET QUOTED_IDENTIFIER ON;
GO
ALTER PROCEDURE [Schema].usp_ProcedureName
@DateFrom date,
@DateTo date,
@BranchCode int
AS
BEGIN
-- added to avoid unnecessary data returning from the procedure.
SET NOCOUNT ON;
DECLARE @DateFromVAR AS date = @DateFrom;
DECLARE @DateToVAR AS date = @DateTo;
DECLARE @BranchCodeVAR AS int = @BranchCode;
SELECT
CAST(EIV2.EntityItemValue AS int) AS EmployeeBranch,
SUM(TotalPaidPrem) AS TotalPremiumPaid,
SUM(TotalPaidPrem) * 0.5 / 100 AS Commission
FROM (
SELECT
PaidPrem.MAXPPremiumDebitPk,
PaidPrem.PolicyNumber,
SUM(PaidMonths.PaidMonths) AS TotalPaidMonths,
MAX(PaidPrem.paidPremium) AS TotalPaidPrem,
paidMonths.CommencementDate
FROM (
SELECT
SUM(Premium) AS paidPremium,
FT.PolicyNumber,
FT.DueDate,
FT.RegistrationDate,
MAX(FT.PPremiumDebitPk) AS MAXPPremiumDebitPk
FROM (
SELECT DISTINCT
FT.Premium,
FT.PolicyNumber,
FT.DueDate,
FT.PPremiumDebitPk,
FT.RegistrationDate
FROM [DataWarehouse].[schema].[TableFT] AS FT
INNER JOIN [DataWarehouse].[schema].[TablePM] AS PM
ON PM.PolicyNumber = FT.PolicyNumber
INNER JOIN [DataWarehouse].[schema].[TablePOD] POD
ON POD.PolicyNumber = FT.PolicyNumber
AND POD.ClosingStatus = 0
LEFT OUTER JOIN TableAD AS AD
ON AD.AgentNumber = FT.AgentNumber
AND AD.ClosingStatus = 0
INNER JOIN [DataWarehouse].[schema].[TablePT] AS PT
ON PT.PremiumTypeNumber = FT.PremiumType
AND PT.PaymentFlag = 1 --Get only the payments
INNER JOIN [DataWarehouse].[schema].[TableGP1] AS GP1
ON GP1.ParameterId = FT.ContributorType
AND GP1.ParameterName = 'ContributorType'
INNER JOIN [DataWarehouse].[schema].[TableGP2] AS GP2
ON GP2.ParameterId = POD.ProductNumber
AND GP2.ParameterAdditionalId = POD.ProductVersion
AND GP2.ParameterName = 'Product'
LEFT OUTER JOIN TableAAD AS AAD
ON AAD.OpeningReference = FT.OpeningReference
AND AAD.ActivityKey = FT.PolicyNumber
AND AAD.ActivityKeyType = 1
AND AAD.ActivitySequence = 1
WHERE FT.PremiumType NOT IN (200, 201, 205, 206, 249, 250)
AND FT.RegistrationDate BETWEEN @DateFromVAR AND @DateToVAR
) AS FT
GROUP BY
FT.PolicyNumber,
FT.DueDate,
FT.RegistrationDate
--,FT.PPremiumDebitPk
) AS PaidPrem
LEFT OUTER JOIN (
SELECT
PT.PolicyNumber,
PT.DueDate,
MAX(PT.RegistrationDate) AS RegistrationDate,
CommencementDate,
MAX(PT.PPremiumDebitPk) AS PPremiumDebitPk,
ISNULL(12/CPA.PaymentFrequency,0) AS PaidMonths
FROM (
SELECT
PolicyNumber,
ContributorType,
ContributorLayer,
DueDate,
FT.CommencementDate,
MAX(FT.RegistrationDate) AS RegistrationDate,
MAX(FT.PPremiumDebitPk)AS PPremiumDebitPk
FROM (
SELECT DISTINCT
FT.Premium,
FT.PolicyNumber,
FT.DueDate,
FT.ContributorType,
FT.PPremiumDebitPk,
LayerNumber AS ContributorLayer,
PM.CommencementDate,
FT.RegistrationDate
FROM [DataWarehouse].[Schema].[FinancialTransaction] AS FT
INNER JOIN [DataWarehouse].[schema].[TablePM] AS PM
ON PM.PolicyNumber = FT.PolicyNumber
INNER JOIN [DataWarehouse].[schema].[TablePOD] AS POD
ON POD.PolicyNumber = FT.PolicyNumber
AND POD.ClosingStatus = 0
LEFT OUTER JOIN [DataWarehouse].[schema].[TableAD] AS AD
ON AD.AgentNumber = FT.AgentNumber
AND AD.ClosingStatus = 0
INNER JOIN [DataWarehouse].[schema].[TablePT] AS PT
ON PT.PremiumTypeNumber = FT.PremiumType
AND PT.PaymentFlag = 1 --Get only the payments
INNER JOIN [DataWarehouse].[schema].[TableGP1] AS GP1
ON GP1.ParameterId = FT.ContributorType
AND GP1.ParameterName = 'ContributorType'
INNER JOIN [DataWarehouse].[schema].[TableGP2] AS GP2
ON GP2.ParameterId = POD.ProductNumber
AND GP2.ParameterAdditionalId = POD.ProductVersion
AND GP2.ParameterName = 'Product'
LEFT OUTER JOIN [DataWarehouse].[schema].[TableAAD] AS AAD
ON AAD.OpeningReference = FT.OpeningReference
AND AAD.ActivityKey = FT.PolicyNumber
AND AAD.ActivityKeyType = 1
AND AAD.ActivitySequence = 1
WHERE -- Exclude: Excess Premium, Excess Premium, Excess Refund Payment, Excess Refund Payment (UL), Refund to Suspense (UL), Refund to Suspense
FT.PremiumType NOT IN (200, 201, 205, 206, 249, 250)
AND FT.RegistrationDate BETWEEN PM.CommencementDate AND @DateToVAR
) AS FT
GROUP BY
PolicyNumber,
ContributorType,
ContributorLayer,
DueDate,
FT.CommencementDate
) AS PT
INNER JOIN DataWarehouse.[Schema].[TableCC] AS CC
ON PT.PolicyNumber = CC.PolicyNumber
AND PT.ContributorType = CC.ContributorType
AND PT.ContributorLayer = CC.LayerNumber
AND PT.RegistrationDate BETWEEN CC.OpeningRegistrationDate AND CC.ClosingRegistrationDate - 1
INNER JOIN [DataWarehouse].[Schema].[TableCPA] AS CPA
ON CC.ContributorId = CPA.ClientNumber
AND PT.RegistrationDate BETWEEN CPA.OpeningRegistrationDate AND CPA.ClosingRegistrationDate - 1
AND CC.PaymentArrangementId = CPA.ArrangementId
GROUP BY
PT.PolicyNumber,
PT.DueDate,
CommencementDate,
ISNULL(12/CPA.PaymentFrequency, 0)
) AS PaidMonths
--calculate paid months that registration and due date of payments in period is grater than other payments registration and due date
--this done to avoid the reversal values in case of the payment frequency modification. example 10037529 for period 1/1/2018-31/3/2018
ON (
PaidPrem.PolicyNumber = PaidMonths.PolicyNumber
AND PaidPrem.RegistrationDate >= PaidMonths.RegistrationDate
AND PaidPrem.DueDate > PaidMonths.DueDate
AND PaidPrem.MAXPPremiumDebitPk >= PaidMonths.PPremiumDebitPk
AND PaidPrem.paidPremium > 0
)
OR (
PaidPrem.PolicyNumber = PaidMonths.PolicyNumber
AND PaidPrem.RegistrationDate > PaidMonths.RegistrationDate
AND PaidPrem.DueDate > PaidMonths.DueDate
AND PaidPrem.MAXPPremiumDebitPk >= PaidMonths.PPremiumDebitPk
AND PaidPrem.paidPremium < 0
)
GROUP BY
PaidPrem.MAXPPremiumDebitPk,
PaidPrem.PolicyNumber,
PaidMonths.CommencementDate
--HAVING MAX(PaidPrem.paidPremium)>0
) AS X
LEFT OUTER JOIN TablePCD AS PCD
ON X.PolicyNumber = PCD.PolicyNumber
LEFT OUTER JOIN TableSCC AS SCC
ON PCD.SourceCode = SCC.SourceCodeId
LEFT OUTER JOIN TableMAD AS MAD
ON PCD.AgentNumber = MAD.AgentNumber
AND PCD.InsuranceCompany = MAD.InsuranceCompany
LEFT OUTER JOIN TableGM1 AS GM1
ON MAD.GroupCode = GM1.GroupCode
LEFT OUTER JOIN TableEIV2 AS EIV2
ON EIV2.EntityNumber = PCD.PolicyNumber
-- AND EIV2.EntitySubNumber = PMA.AgentLayerNumber
AND EIV2.EntityItemType = 5
AND EIV2.EntityItemSubType = 1
AND EIV2.EntityItemId = 2
AND EIV2.ClosingStatus = 0
WHERE X.TotalPaidMonths >= 12
AND AgentBusinessTypeSourceCd = '02'
AND PCD.InsuranceCompany <> 2
AND (CAST(EIV2.EntityItemValue AS int) = @BranchCodeVAR OR @BranchCodeVAR = 0)
GROUP BY CAST(EIV2.EntityItemValue AS int)
ORDER BY CAST(EIV2.EntityItemValue AS int)
END;
GO
GO
EDIT: and NEVER use the same table alias twice. Couldn't fix that because I don't really know what the tables are for sure.
Steve (aka sgmunson) 🙂 🙂 🙂
Rent Servers for Income (picks and shovels strategy)
July 18, 2018 at 12:17 pm
NikosV - Wednesday, July 18, 2018 7:37 AMI am a beginner in Microsoft SQL and I’ve come across this stored procedure at work. I sort of understand what the programmer was trying to do but I would like to know if this is a good example of a stored proc. Ideally, I would like to know:1. If this stored procedure is a good example. 2. If anything, what is generally wrong with it3. How it could be improved. This stored proc has been in use as is for about two years so it’s not something that I’m going to go and change, nobody is going to touch it but since it looks really complex I was wondering if this is how a similar sp should look like. USE [Database]GOSET ANSI_NULLS ONGOSET QUOTED_IDENTIFIER ONGOALTER PROCEDURE [Schema].[Table] @DateFrom DATE, @DateTo DATE,@BranchCode INTASBEGINDECLARE @DateFromVAR AS DATE =@DateFromDECLARE @DateToVAR AS DATE =@DateToDECLARE @BranchCodeVAR AS INT =@BranchCodeSELECT CAST(EIV2.EntityItemValue AS INT) AS EmployeeBranch, SUM(TotalPaidPrem) AS TotalPremiumPaid, SUM(TotalPaidPrem)*0.5/100 AS Commission FROM (SELECT PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,SUM(PaidMonths.PaidMonths)TotalPaidMonths,MAX(PaidPrem.paidPremium) TotalPaidPrem,paidMonths.CommencementDateFROM (SELECT SUM(Premium) AS paidPremium,FT.PolicyNumber,FT.DueDate,FT.RegistrationDate,MAX(FT.PPremiumDebitPk) AS MAXPPremiumDebitPkFROM(SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.PPremiumDebitPk,FT.RegistrationDateFROM [DataWarehouse].[schema].[Table] FTINNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumberINNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0LEFT OUTER JOIN Table AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the paymentsINNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'LEFT OUTER JOIN Table AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1WHEREFT.PremiumType NOT IN (200, 201, 205, 206, 249, 250) AND FT.RegistrationDate BETWEEN @DateFromVAR AND @DateToVAR)FTGROUP BY FT.PolicyNumber,FT.DueDate,FT.RegistrationDate--,FT.PPremiumDebitPk)PaidPremLEFT OUTER JOIN (SELECT PT.PolicyNumber,PT.DueDate,MAX(PT.RegistrationDate) AS RegistrationDate,CommencementDate,MAX(PT.PPremiumDebitPk)AS PPremiumDebitPk,ISNULL(12/CPA.PaymentFrequency,0) AS PaidMonthsFROM (SELECT PolicyNumber, ContributorType, ContributorLayer,DueDate,FT.CommencementDate,MAX(FT.RegistrationDate) AS RegistrationDate,MAX(FT.PPremiumDebitPk)AS PPremiumDebitPkFROM (SELECT DISTINCT FT.Premium,FT.PolicyNumber,FT.DueDate,FT.ContributorType,FT.PPremiumDebitPk,LayerNumber AS ContributorLayer,PM.CommencementDate,FT.RegistrationDateFROM [DataWarehouse].[Schema].[FinancialTransaction] FTINNER JOIN [DataWarehouse].[schema].[Table] PM ON PM.PolicyNumber = FT.PolicyNumberINNER JOIN [DataWarehouse].[schema].[Table] POD ON POD.PolicyNumber = FT.PolicyNumber AND POD.ClosingStatus = 0LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AD ON AD.AgentNumber = FT.AgentNumber AND AD.ClosingStatus = 0INNER JOIN [DataWarehouse].[schema].[Table] PT ON PT.PremiumTypeNumber = FT.PremiumType AND PT.PaymentFlag = 1 --Get only the paymentsINNER JOIN [DataWarehouse].[schema].[Table] GP1 ON GP1.ParameterId = FT.ContributorType AND GP1.ParameterName = 'ContributorType'INNER JOIN [DataWarehouse].[schema].[Table] GP2 ON GP2.ParameterId = POD.ProductNumber AND GP2.ParameterAdditionalId = POD.ProductVersion AND GP2.ParameterName = 'Product'LEFT OUTER JOIN [DataWarehouse].[schema].[Table] AAD ON AAD.OpeningReference = FT.OpeningReference AND AAD.ActivityKey = FT.PolicyNumber AND AAD.ActivityKeyType = 1 AND AAD.ActivitySequence = 1WHERE -- Exclude: Excess Premium, Excess Premium, Excess Refund Payment, Excess Refund Payment (UL), Refund to Suspense (UL), Refund to SuspenseFT.PremiumType NOT IN (200, 201, 205, 206, 249, 250) AND FT.RegistrationDate BETWEEN PM.CommencementDate AND @DateToVAR) FTGROUP BY PolicyNumber, ContributorType, ContributorLayer,DueDate,FT.CommencementDate)PTINNER JOIN DataWarehouse.Schema.Table CCON PT.PolicyNumber=CC.PolicyNumberAND PT.ContributorType=CC.ContributorTypeAND PT.ContributorLayer=CC.LayerNumberAND PT.RegistrationDate BETWEEN CC.OpeningRegistrationDate AND CC.ClosingRegistrationDate-1INNER JOIN [DataWarehouse].[Schema].[Table] CPAON CC.ContributorId=CPA.ClientNumberAND PT.RegistrationDate BETWEEN CPA.OpeningRegistrationDate AND CPA.ClosingRegistrationDate-1AND CC.PaymentArrangementId=CPA.ArrangementIdGROUP BY PT.PolicyNumber,PT.DueDate,CommencementDate,ISNULL(12/CPA.PaymentFrequency,0))PaidMonths--calculate paid months that registration and due date of payments in period is grater than other payments registration and due date--this done to avoid the reversal values in case of the payment frequency modification. example 10037529 for period 1/1/2018-31/3/2018ON (PaidPrem.PolicyNumber=PaidMonths.PolicyNumberAND PaidPrem.RegistrationDate>=PaidMonths.RegistrationDateAND PaidPrem.DueDate>PaidMonths.DueDateAND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPkAND PaidPrem.paidPremium>0)OR (PaidPrem.PolicyNumber=PaidMonths.PolicyNumberAND PaidPrem.RegistrationDate>PaidMonths.RegistrationDateAND PaidPrem.DueDate>PaidMonths.DueDateAND PaidPrem.MAXPPremiumDebitPk>=PaidMonths.PPremiumDebitPkAND PaidPrem.paidPremium<0)GROUP BY PaidPrem.MAXPPremiumDebitPk,PaidPrem.PolicyNumber,PaidMonths.CommencementDate--HAVING MAX(PaidPrem.paidPremium)>0)XLEFT OUTER JOIN Table PCDON X.PolicyNumber = PCD.PolicyNumberLEFT OUTER JOIN Table SCCON PCD.SourceCode = SCC.SourceCodeIdLEFT OUTER JOIN Table MADON PCD.AgentNumber=MAD.AgentNumberAND PCD.InsuranceCompany=MAD.InsuranceCompanyLEFT OUTER JOIN Table GM1ON MAD.GroupCode=GM1.GroupCodeLEFT OUTER JOIN Table EIV2 ON EIV2.EntityNumber = PCD.PolicyNumber -- AND EIV2.EntitySubNumber = PMA.AgentLayerNumber AND EIV2.EntityItemType = 5 AND EIV2.EntityItemSubType = 1 AND EIV2.EntityItemId = 2 AND EIV2.ClosingStatus=0WHERE X.TotalPaidMonths>=12AND AgentBusinessTypeSourceCd = '02'AND PCD.InsuranceCompany<>2AND (CAST(EIV2.EntityItemValue AS INT)=@BranchCodeVAR OR @BranchCodeVAR=0)GROUP BY CAST(EIV2.EntityItemValue AS INT)ORDER BY CAST(EIV2.EntityItemValue AS INT)END* Ignore the commentsI’m afraid I don’t have an execution plan so a really general comment or general feedback would be more than enough. Thanks
A thing i noticed is that the LEFT JOIN on PCD and EIV2 is in effect an INNER JOIN since the below conditions are being checked in the WHERE clause
------
AND PCD.InsuranceCompany<>2
AND (CAST(EIV2.EntityItemValue AS INT)=@BranchCodeVAR OR @BranchCodeVAR=0)
-----
reason being->had it been a LEFT JOINed record then the PCD.InsuranceCompany would be NULL and the condition NULL<>2 would mean the records that are NULLed from PCD would never find its way in the query output. in other words only NOT NULL values of PCD would ever be considered int he query output and those which are have <> 2
July 18, 2018 at 2:09 pm
Sorry for the formatting guys. Uploaded from
The phone and I won’t be doing that again.
Thank you all for your help, your time and your guidance.
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply