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

procedure, hoping to improve the performance

The procedure below works, I am just trying to reduce the runtime.  Right now, it runs from 15 to 19 seconds.  Going to need a much better runtime in production.

I've implemented a couple missing indices, and seen notable improvement, but it still needs to be a bit better.

I'm looking for insight on the logic itself, or even recommended index structure.  Basically, if any Expert reviews this construct, and needed to improve the execution -- where would you go first?
/****** Object:  StoredProcedure [dbo].[procedureName]    Script Date: 09/24/2011 14:58:36 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROC [dbo].[procedureName] (
 @RoutingNumber char(9),
 @BankAcctNo char(17)
)
AS
SET NOCOUNT ON;
/*
Allows for the denial check.  
IF any check fails, 1 is returned to the front end, the process terminates.
IF all checks succeed, 0 is returned to the front end.  
1 = Fail, 0 = Pass

EXEC dbo.procedureName @RoutingNumber = '072000096',@BankAcctNo = '6820138904'

Auth:  Me
Date:  9/14/2011

THREE CHECKS:
1. Be the same Routing number/Bank Account number associated with a loan in collection status. 
2. Be associated with a loan app no that contains the same Routing number/Bank account number with a denial dode of 5 that occurred in the last 45 calendar days.
3. Be associated with a customer who has a loan balance > 0 using the same Routing number/bank account combination.
*/
BEGIN
	DECLARE @Status1 TABLE (Status CHAR(1), PRIMARY KEY (Status))
	INSERT @Status1 VALUES
	('B'),('C'),('R'),('N'),('O'),('G');

	DECLARE @Status2 TABLE (Status CHAR(1), PRIMARY KEY (Status))
	INSERT @Status2 VALUES
	('N'),('O'),('P'),('D'),('B'),('R'),('C'),('G'),('V');

		/* Check #1 */
		IF EXISTS(SELECT 1 FROM dbo.LoanTable l WITH (NOLOCK) JOIN dbo.CustomerTable c WITH (NOLOCK)
		  ON l.CustID = c.CustID
		WHERE c.RoutingNo = @RoutingNumber
		AND c.AcctNo = @BankAcctNo
		AND EXISTS(SELECT 1 FROM @Status1 s1 WHERE l.Status = s1.Status))
			BEGIN
				SELECT '1';
			END

		/* Check #2 */
		IF EXISTS(SELECT 1 FROM dbo.LoanTable l WITH (NOLOCK) JOIN dbo.CustomerTable c WITH (NOLOCK)
		  ON l.CustID = c.CustID
		WHERE l.DateEntered >= DATEADD(day,-45,GETDATE())
		AND l.DenialCode = 5
		AND c.RoutingNo = @RoutingNumber
		AND c.AcctNo = @BankAcctNo)
			BEGIN
			  SELECT '1';
			END	

		/* Check #3 */
		IF EXISTS(
			SELECT 1 FROM
			(
			SELECT CAST(la.LoanAmt  AS DECIMAL) + CAST(la.FinCharge AS DECIMAL) - SUM(CAST(COALESCE(p.PmtAmt, 0) AS DECIMAL)) AS Balance
			FROM dbo.LoanTable la WITH (NOLOCK) LEFT JOIN dbo.Payments p WITH (NOLOCK)
			   ON la.ApplNo = p.ApplNo JOIN dbo.CustomerTable c WITH (NOLOCK)
				 ON la.CustID = c.CustID      
			WHERE la.Originated = 1      
			AND EXISTS(SELECT 1 FROM @Status2 s2 WHERE s2.Status = la.Status)
			GROUP BY la.LoanAmt,la.FinCharge) x
			HAVING SUM(x.Balance) > 0)
				BEGIN
				  SELECT '1';
				END

				IF @@ERROR <> 0
				BEGIN
					RAISERROR('Failed to perform the denial check.',16,1)
					RETURN;
				END
				ELSE
				BEGIN
					SELECT '0';
				END

END

SET NOCOUNT OFF;


GO

Open in new window

0
dbaSQL
Asked:
dbaSQL
  • 7
  • 4
1 Solution
 
dqmqCommented:
Performance wise, the first order of business of course is to make sure you have indexes on CustID, RoutingNo, and BankAcctNo.  But I assume you've already taken care of that.

Logic wise, I do not think your procedure is working as described.  Looks to me like it does NOT terminate when a check fails like your comments indicate.  Looks to me like the procedure could return several ones and a zero in different result sets.  

I think you need to modify the logic to return a single result set.  Or, preferably to my way of thinking, pass back the results in a parameter rather than a result set.


Finally, you can easily consolidate your 3 checks into one SQL statement, as follows:

Instead of this structure:
If exists (select 1 ... ) then
   select 1
   exit
If exists (select 2 ... ) then
   select 1
   exit
If exists (select 3 ... ) then
   select 1
   exit

Use this structure:

Select 1 where
    exists(select 1 ... )
or exists(select 2 ...)
or exists(select 3 ...)


FWIW, I don't have time to noodle through it, but I rather expect you could further refine the above structure to a series of joins that may further improve performance.














 

0
 
dbaSQLAuthor Commented:
Yes, you're right on the indexes.  I've already taken care of that.

>>Logic wise, I do not think your procedure is working as described.
Let me take a look.
0
 
dbaSQLAuthor Commented:
Select 1 where
    exists(select 1 ... )
or exists(select 2 ...)
or exists(select 3 ...)



i see what you are saying.  and, i need to return 0 for success
if i wrap all of that into a single check, i need a 1 returned if any checks return data, or fail, and i need a 0 returned if nothing returns data, or they all succeed.

what do you think about the 0 ?
0
Cloud Class® Course: Amazon Web Services - Basic

Are you thinking about creating an Amazon Web Services account for your business? Not sure where to start? In this course you’ll get an overview of the history of AWS and take a tour of their user interface.

 
dbaSQLAuthor Commented:
I do have it all in a single statement, but I'm still not able to drop nthe runtime to any less than :19.  

See the 1st two checks, they're virtually identical, except for  the denial code and the 45 day check.  Surely I could be smarter about this, and do it in one check.  Do you have any thoughts?
0
 
LowfatspreadCommented:
is your problem that check three is checking across every customer rather than a specific one?


/* Check #3 */
            IF EXISTS(
                  SELECT 1 FROM
                  (
                  SELECT CAST(la.LoanAmt  AS DECIMAL) + CAST(la.FinCharge AS DECIMAL) - SUM(CAST(COALESCE(p.PmtAmt, 0) AS DECIMAL)) AS Balance
                  FROM dbo.LoanTable la WITH (NOLOCK) LEFT JOIN dbo.Payments p WITH (NOLOCK)
                     ON la.ApplNo = p.ApplNo JOIN dbo.CustomerTable c WITH (NOLOCK)
                         ON la.CustID = c.CustID      
                  WHERE la.Originated = 1      
                  AND EXISTS(SELECT 1 FROM @Status2 s2 WHERE s2.Status = la.Status)
/*
 missing
and c.RoutingNo = @RoutingNumber
            AND c.AcctNo = @BankAcctNo
?
*/
                  GROUP BY la.LoanAmt,la.FinCharge) x
                  HAVING SUM(x.Balance) > 0)
                        BEGIN
                          SELECT '1';
                        END
0
 
LowfatspreadCommented:
checks 1  and 2 can be merged like this....

i'd avoid the table variables for these scenarios as well...



	IF EXISTS(
                SELECT 1
                  FROM dbo.LoanTable l WITH (NOLOCK) 
                  inner JOIN dbo.CustomerTable c WITH (NOLOCK)
		  ON l.CustID = c.CustID
		WHERE c.RoutingNo = @RoutingNumber
		AND c.AcctNo = @BankAcctNo
		having sum(case when l.status in  ('B','C','R','N','O','G') then 1 else 0 end) > 0 -- Check #1 
                   or sum(case when l.DateEntered >= DATEADD(day,-45,GETDATE())    -- Check #2 
		                 AND l.DenialCode = 5
                                then 1 
                                else 0
                                end) > 0
                )
			BEGIN
				SELECT '1';
			END

Open in new window


personally i'd also not use an if statement for these tests and define variables to be returned by the select statement
and then test those...

also i wouldn't use Select '1' to return my result rather either an output parameter from the procedure or use the procedure return code to indicate its result. (say positive value for an expected result or negative ones to indicate a processing error/condition)


why do you need to cast the amounts to decimal , that implies you have an incorrect datatype on the tables... change it to decimal at the source....
0
 
dbaSQLAuthor Commented:
>>>is your problem that check three is checking across every customer rather than a specific one?
bingo.

Don't know how I missed that.  Having added in the specific customer check, the runtime dropped to :00.

sweet.

I agree with you on the change at the datasource.  Unfortunately, I couldn't get them to buy into it.  The datatype is still money.  

can you show me your thoughts on the variables, and the output parm, lowfat?
0
 
LowfatspreadCommented:
like this ?
/****** Object:  StoredProcedure [dbo].[procedureName]    Script Date: 09/24/2011 14:58:36 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROC [dbo].[procedureName] (
 @RoutingNumber char(9),
 @BankAcctNo char(17),
 @ApplStatus int output
)
AS
SET NOCOUNT ON;
/*
Allows for the denial check.  
IF any check fails, 1 is returned to the front end, the process terminates.
IF all checks succeed, 0 is returned to the front end.  
1 = Fail, 0 = Pass

EXEC dbo.procedureName @RoutingNumber = '072000096',@BankAcctNo = '6820138904',@ApplStatus=@Stat Output

Auth:  Me
Date:  9/14/2011

THREE CHECKS:
1. Be the same Routing number/Bank Account number associated with a loan in collection status. 
2. Be associated with a loan app no that contains the same Routing number/Bank account number with a denial dode of 5 that occurred in the last 45 calendar days.
3. Be associated with a customer who has a loan balance > 0 using the same Routing number/bank account combination.
*/
BEGIN
	
	DECLARE @Status2 TABLE (Status CHAR(1), PRIMARY KEY (Status))
	INSERT @Status2 VALUES
	('N'),('O'),('P'),('D'),('B'),('R'),('C'),('G'),('V');

		
	declare @test int
                SELECT @test=1
                  FROM dbo.LoanTable l WITH (NOLOCK) 
                  inner JOIN dbo.CustomerTable c WITH (NOLOCK)
		  ON l.CustID = c.CustID
		WHERE c.RoutingNo = @RoutingNumber
		AND c.AcctNo = @BankAcctNo
		having sum(case when l.status in  ('B','C','R','N','O','G') then 1 else 0 end) > 0 -- Check #1 
                   or sum(case when l.DateEntered >= DATEADD(day,-45,GETDATE())    -- Check #2 
		                 AND l.DenialCode = 5
                                then 1 
                                else 0
                                end) > 0
                )
	if @test is null 
        begin	
			SELECT @test=1  
                         from dbo.CustomerTable c WITH (NOLOCK)
                         where c.RoutingNo = @RoutingNumber
                           AND c.AcctNo = @BankAcctNo
                           AND exists 
			(
			SELECT 1
			  FROM dbo.LoanTable la WITH (NOLOCK) 
                        INNER JOIN @STATUS2 AS S2
                           ON S2.STATUS=LA.STATUS
                        LEFT JOIN (select applno,SUM(CAST(COALESCE(p.PmtAmt, 0) AS DECIMAL)) AS Balance
                                     from dbo.Payments WITH (NOLOCK)
                                     group by applno
                                  ) as p 
			   ON la.ApplNo = p.ApplNo 
                        WHERE la.Originated = 1 
                          and la.custid=c.custid 
                        HAVING CAST(la.LoanAmt  AS DECIMAL) + CAST(la.FinCharge AS DECIMAL) - COALESCE(p.Balance,0) <> 0
                         )    
	end
			
				IF @@ERROR <> 0
				BEGIN
					RAISERROR('Failed to perform the denial check.',16,1)
					RETURN;
				END
				
       set @applstatus=coalesce(@test,0)
END

return


GO

Open in new window

0
 
dbaSQLAuthor Commented:
I think so, yes, lowfat. but i'm still working your suggestion... i'm trying to get the group by together

Msg 8121, Level 16, State 1, Procedure procedureName, Line 59
Column 'dbo.LoanTable.LoanAmt' is invalid in the HAVING clause because it is not contained in either an aggregate function or the GROUP BY clause.
Msg 8121, Level 16, State 1, Procedure procedureName, Line 59
Column 'dbo.LoanTable.FinCharge' is invalid in the HAVING clause because it is not contained in either an aggregate function or the GROUP BY clause.
Msg 8121, Level 16, State 1, Procedure procedueName, Line 59
Column 'p.Balance' is invalid in the HAVING clause because it is not contained in either an aggregate function or the GROUP BY clause.
Msg 8121, Level 16, State 1, Procedure procedureName, Line 59
Column 'p.Balance' is invalid in the HAVING clause because it is not contained in either an aggregate function or the GROUP BY clause.

0
 
LowfatspreadCommented:
sorry  


SELECT @test=1  
                         from dbo.CustomerTable c WITH (NOLOCK)
                         where c.RoutingNo = @RoutingNumber
                           AND c.AcctNo = @BankAcctNo
                           AND exists
                  (
                  SELECT 1
                    FROM dbo.LoanTable la WITH (NOLOCK)
                        INNER JOIN @STATUS2 AS S2
                           ON S2.STATUS=LA.STATUS
                        LEFT JOIN (select applno,SUM(CAST(COALESCE(p.PmtAmt, 0) AS DECIMAL)) AS Balance
                                     from dbo.Payments WITH (NOLOCK)
                                     group by applno
                                  ) as p
                     ON la.ApplNo = p.ApplNo
                        WHERE la.Originated = 1
                          and la.custid=c.custid
                        and CAST(la.LoanAmt  AS DECIMAL) + CAST(la.FinCharge AS DECIMAL) - COALESCE(p.Balance,0) <> 0
                         )  
0
 
dbaSQLAuthor Commented:
no sorry necessary, lowfat.  i'm away from this project until this evening.  i will get back to you as soon as i can.
0
 
dbaSQLAuthor Commented:
my apologies, lowat. i am dealing with an emergency of sorts, and i cannot focus on this anymore.  i do appreciate your help, though, and i will go ahead and award, and close the inquiry.
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

Free Tool: SSL Checker

Scans your site and returns information about your SSL implementation and certificate. Helpful for debugging and validating your SSL configuration.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

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