June 11, 2021 at 4:41 am
Hi everyone,
I am trying to create a UDF to apply to a dataset. The code for it is below. I think the issue is in the while loop. I am not sure the while loop is required when I used a recursive CTE. When I take the while loop out my function breaks. I am currently testing it on a HL account to ensure it works then will apply to a full dataset. I have taken the code from the function outside of it and run it in steps over different lags and it works (that is what makes me think the issue is in the while loop).
Use sand;
Alter Function UDF_HL_clean;
( @H_account NVCHAR(255) )
Returns @terms table (
H_account NVCHAR(255),
seq INT,
Cardedratedate Date,
expdate Date,
term INT,
Condit VARCHAR(1)
)
As
Begin
Declare @udftemptable TABLE (
H_account NVCHAR(255),
seq INT,
Cardedratedate Date,
expdate Date,
term INT,
Condit VARCHAR(1)
)
insert into @terms select * from sand.dbo.HL_function_test where H_account = @H_account
declare @lag INT = 0, @loopmax INT
SET @loopmax = (Select count(*) FROM (select H_account, expdate, lead(expdate, 1) Over (partition by H_account, expdate Order by H_account, seq desc) as lag_exp From dbo.homeloans) a
where lag_exp = expdate
AND H_account = @H_account
Group by H_account, expdate
)
while @lag <= @loopmax
Begin
with laggedrates (H_account, seq, lag, lageffectivedate, lagexpdate, expdate, lagterm, condit) AS
(
select H_account,
seq,
@lag as lag,
lag(cardedratedate, @lag) over (partition by H_account, expdate order by H_account, seq desc),
lag(expdate, @lag) over (partition by H_account, expdate order by H_account, seq desc),
expdate,
lag(term, @lag) over (partition by H_account, expdate order by H_account, seq desc),
case when lead(expdate,1) over (partition by H_account, expdate order by H_account, seq desc) = expdate THEN 'F' Else Condition END
from @terms
Union all
select a.H_account,
a.seq,
b.lag + 1 as lag,
lag(a.cardedratedate, b.lag + 1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) as lageffectivedate,
lag(a.expdate, b.lag + 1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) as lagexpdate,
a.expdate,
case when lag(a.expdate,b.lag+1) over (partition by a.H_account order by a.H_account, a.seq desc) = a.expdate
THEN lag(a.term, b.lag +1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc)
else a.term end
as lagterm
case when lead(a.expdate,1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) = a.expdate THEN 'F' Else a.Condit END AS Condit
from @terms a
inner join laggedrates b
on a.expdate = b.lagexpdate
where @lag <=loopmax and b.lagexpdate is NULL
)
insert into @udftemptable Select H_account, seq, lageffectivedate, expdate, lagterm, condit
from laggedrates option (maxrecursion 0)
delete from @terms
Insert into @terms select * from @udftemptable
deleted from udftemptable
set @lag = @lag + 1
end
return
end
;
Note: I apologise if there are tpyos, I had to manually type that out and change the variable names due to data security.
There is an issue in my data where a Home loan term can be split across seq when they are part of the same term. This causes the effective date to change to the date of the split and term (date difference between effective date and expiry date) to become no standard term (8 months instead of 12 months).
Can anyone help me get this function working (probably an impossible task with out the data to trial and error on) or help me get rid of the while loop so I can test the function that way.
Thank you everyone
June 11, 2021 at 12:05 pm
could you supply some example representative data and what your expected outcome is?
***The first step is always the hardest *******
June 11, 2021 at 12:34 pm
Not only some representative data along with the expected output but how about a description of what the function is supposed to do, please.
I would also get into the habit of documenting your code because even you won't remember what you did or why six months from now and the idea of of "well, just read the code to find out" is just an excuse fore being lazy. 😉
We'd also need some data from and embedded objects such as sand.dbo.HL_function_test and it would really be nice if that type of information were in a readily consumable format. See the article at the first link in my signature line below for one of the multiple ways to do that.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 11, 2021 at 3:56 pm
Jeff, one of the best things I ever heard about comments is this:
Comments don't exist to describe what the code is doing. They exist to describe what the code is trying to do.
Obviously if the code is working correctly and efficiently, it will rarely be looked at. But when there is a problem, it is much faster to have a clear statement of the purpose of the code as opposed to having to intuit its purpose. This benefits not only other developers but also your future self.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
June 11, 2021 at 7:57 pm
Jeff, one of the best things I ever heard about comments is this:
Comments don't exist to describe what the code is doing. They exist to describe what the code is trying to do.
Obviously if the code is working correctly and efficiently, it will rarely be looked at. But when there is a problem, it is much faster to have a clear statement of the purpose of the code as opposed to having to intuit its purpose. This benefits not only other developers but also your future self.
BWAAA-HAAA!!!!! Yep. I certainly agree with that but that's definitely not the only use. People frequently crab about things like code reuse. With a name like "UDF_HL_clean", it think that even those with the proper esoteric knowledge of what an "HL" might be will have trouble determining what the function does. A simple flower box with a purpose and simple usage example would go a long way in answering the question of "can I use this function to do what I need to do" would go a long way.
Of course, then there's code like this function. It might work perfectly functionally speaking but it has so many poor performance flares in it that it's begging to be helped. What's really bad is the OP is in the process of writing it and just assumes that everyone knows what it does or has the time to figure it out. Never mind a "future benefit", the benefit would be realized now. 😀
--Jeff Moden
Change is inevitable... Change for the better is not.
June 11, 2021 at 9:52 pm
The Dixie Flatline wrote:Jeff, one of the best things I ever heard about comments is this:
Comments don't exist to describe what the code is doing. They exist to describe what the code is trying to do.
Obviously if the code is working correctly and efficiently, it will rarely be looked at. But when there is a problem, it is much faster to have a clear statement of the purpose of the code as opposed to having to intuit its purpose. This benefits not only other developers but also your future self.
BWAAA-HAAA!!!!! Yep. I certainly agree with that but that's definitely not the only use. People frequently crab about things like code reuse. With a name like "UDF_HL_clean", it think that even those with the proper esoteric knowledge of what an "HL" might be will have trouble determining what the function does. A simple flower box with a purpose and simple usage example would go a long way in answering the question of "can I use this function to do what I need to do" would go a long way.
Of course, then there's code like this function. It might work perfectly functionally speaking but it has so many poor performance flares in it that it's begging to be helped. What's really bad is the OP is in the process of writing it and just assumes that everyone knows what it does or has the time to figure it out. Never mind a "future benefit", the benefit would be realized now. 😀
Let's not forget that a little bit of formatting also helps - a lot. It is also much easier to document with a simple snippet in those more complex areas of the code to help identify the purpose of that calculation or expression.
Jeffrey Williams
“We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”
― Charles R. Swindoll
How to post questions to get better answers faster
Managing Transaction Logs
June 11, 2021 at 10:46 pm
Ok apologies I am new to this. On another note how do you find a tables details, like column names and data type (for example in SAS you have proc contents)?
Drop table if exists #data;
create table #data
(
H_account NVCHAR(255),
seq INT,
Cardedratedate Datetime,
expdate Datetime,
term INT,
Condit VARCHAR(1)
)
--===== All Inserts into the IDENTITY column
SET IDENTITY_INSERT #data ON
--===== Insert the test data into the test table
INSERT INTO #data
(H_account, seq, Cardedratedate, expdate, term, Condit )
SELECT 'eee555ww325','1','2019-04-02 00:00:00.000','2019-10-02 00:00:00.000','6','F' UNION ALL
SELECT 'eee555ww325','2','2017-04-01 00:00:00.000','2019-04-01 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325','3','2015-03-28 00:00:00.000','2017-03-31 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325','7','2014-05-12 00:00:00.000','2015-03-13 00:00:00.000','10','D' UNION ALL
SELECT 'eee555ww325','8','2014-04-11 00:00:00.000','2015-03-13 00:00:00.000','11','D' UNION ALL
SELECT 'eee555ww325','5','2014-05-23 00:00:00.000','2015-03-13 00:00:00.000','10','F' UNION ALL
SELECT 'eee555ww325','6','2014-05-13 00:00:00.000','2015-03-13 00:00:00.000','10','D' UNION ALL
SELECT 'eee555ww325','9','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','D' UNION ALL
SELECT 'eee555ww325','10','2012-10-01 00:00:00.000','2017-10-01 00:00:00.000','60','D'
--===== Set the identity insert back to normal
SET IDENTITY_INSERT #data ON
If you order by seq the data will make more sense (I purposefully unordered it because it's not ordered in my data).
Seq 5-9 are all part of the same term. But the customer has done something to their loan that has cause it to split across seq and there for the terms are non standard. A standard HL term is 6,12,24,36,48,60 months. What I want the data to look like is:
Drop table if exists #data_I_want;
create table #data_I_want
(
H_account NVCHAR(255),
seq INT,
Cardedratedate Datetime,
expdate Datetime,
term INT,
Condit VARCHAR(1)
)
--===== All Inserts into the IDENTITY column
SET IDENTITY_INSERT #data_I_want ON
--===== Insert the test data into the test table
INSERT INTO #data
(H_account, seq, Cardedratedate, expdate, term, Condit )
SELECT 'eee555ww325','1','2019-04-02 00:00:00.000','2019-10-02 00:00:00.000','6','F' UNION ALL
SELECT 'eee555ww325','2','2017-04-01 00:00:00.000','2019-04-01 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325','3','2015-03-28 00:00:00.000','2017-03-31 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325','5','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','6','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','7','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','8','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','9','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','10','2012-10-01 00:00:00.000','2017-10-01 00:00:00.000','60','D'
--===== Set the identity insert back to normal
SET IDENTITY_INSERT #data_I_want ON
Currently the condit part of the function is working. But for the Cardedratedate and expdate I am getting NULLs.
The function with comments is:
Use sand;
Alter Function UDF_HL_clean;
-- Input variable
( @H_account NVCHAR(255) )
--output table
Returns @terms table (
H_account NVCHAR(255),
seq INT,
Cardedratedate Date,
expdate Date,
term INT,
Condit VARCHAR(1)
)
As
Begin
--temp table to load data into and then clear at each loop
Declare @udftemptable TABLE (
H_account NVCHAR(255),
seq INT,
Cardedratedate Date,
expdate Date,
term INT,
Condit VARCHAR(1)
)
--sets up the first table
insert into @terms select * from sand.dbo.HL_function_test where H_account = @H_account
declare @lag INT = 0, @loopmax INT
--Sets the loop max. the dummy table is one record but there are many HL with this issue. Sometimes the
--issue is across 4 seq and sometimes only 2
SET @loopmax = (Select count(*) FROM (select H_account, expdate, lead(expdate, 1) Over (partition by H_account, expdate Order by H_account, seq desc) as lag_exp From dbo.homeloans) a
where lag_exp = expdate
AND H_account = @H_account
Group by H_account, expdate
)
--loops over each seq where there is an issue while the number is equal or less than the maximum amount of time there is an issue for each HL
while @lag <= @loopmax
Begin
--Names the columns
with laggedrates (H_account, seq, lag, lageffectivedate, lagexpdate, expdate, lagterm, condit) AS
(
select H_account,
seq,
@lag as lag,
--gets the carded rate date from step before because that is the actual date the rate is from
lag(cardedratedate, @lag) over (partition by H_account, expdate order by H_account, seq desc),
--Not sure this step is required since the expiry date is correct in the first data.
lag(expdate, @lag) over (partition by H_account, expdate order by H_account, seq desc),
expdate,
-- gets the standard term from step before. need to get the standard term.
lag(term, @lag) over (partition by H_account, expdate order by H_account, seq desc),
--this step is actually working. gets condition from in front. These are fixed loans so condit = 'F'
--THis issue is causing the condit to be 'D'
case when lead(expdate,1) over (partition by H_account, expdate order by H_account, seq desc) = expdate THEN 'F' Else Condition END
from @terms
Union all
select a.H_account,
a.seq,
--adding to the lag
b.lag + 1 as lag,
--taking the next lag to look at the next step
lag(a.cardedratedate, b.lag + 1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) as lageffectivedate,
--taking the next lag to look at the next step
lag(a.expdate, b.lag + 1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) as lagexpdate,
a.expdate,
--taking the next lag only while the expiry dates are still matching
case when lag(a.expdate,b.lag+1) over (partition by a.H_account order by a.H_account, a.seq desc) = a.expdate
THEN lag(a.term, b.lag +1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc)
else a.term end
as lagterm
case when lead(a.expdate,1) over (partition by a.H_account, a.expdate order by a.H_account, a.seq desc) = a.expdate THEN 'F' Else a.Condit END AS Condit
from @terms a
--recurcisve CTE to join steps together
inner join laggedrates b
on a.expdate = b.lagexpdate
--ending CTE when loopmax is reached
where @lag <=loopmax and b.lagexpdate is NULL
)
insert into @udftemptable Select H_account, seq, lageffectivedate, expdate, lagterm, condit
from laggedrates option (maxrecursion 0)
delete from @terms
Insert into @terms select * from @udftemptable
deleted from udftemptable
set @lag = @lag + 1
end
return
end
;
I hope that is everything you guys need. I cannot directly copy and past data or code for security reasons so I apologise if there is tpyos. Let me know if I have missed anything.
Thanks again
June 13, 2021 at 7:24 pm
Ok apologies I am new to this. On another note how do you find a tables details, like column names and data type (for example in SAS you have proc contents)?
If you're using SSMS, press the {f8} key to display the "Explorer Window" (if it's not already open). Then, just follow your nose, expanding the databases node, then expanding the database you want, expand the tables node, find the table you want, and right click on it. There are several different scripting options there.
Also, you can type sp_help 'dbo.tablename' (replacing what's in the quotes with the actual table name.
Also, as you're drilling down in the Explorer Window, if you right click on the Tables node (or any other object node), you can select "Filter" to make things a whole lot easier to find. Again, just follow your nose there.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 13, 2021 at 9:23 pm
Thanks, Scott. The readily consumable test data you provided had a couple of bugs in the CREATE TABLE statements. For example, there is no such datatype as "NVCHAR"... that had to be changed to "NVARCHAR". Also, although it's certainly legal, I changed the VARCHAR(1) datatypes to just CHAR(1), which most people do because VARCHAR(1) actually takes 3 bytes... 1 for the data, 2 for the metadata to identify the variable length, which will never really be variable, in this case)
Here's the code that will actually work. I also changed your desired results table name to just #Results to make typing and reading a little easier and also added some visual separators to also make readability a little easier.
--=====================================================================================================================
-- Create and populate a test table for the original data
-- #Data
--=====================================================================================================================
--===== If it exists, drop the test table to make reruns in SSMS easier.
SET IDENTITY_INSERT #Data OFF;
DROP TABLE IF EXISTS #Data
;
GO
--===== Create the test table
CREATE TABLE #Data
(
H_account NVARCHAR(255)
,seq INT IDENTITY(1,1)
,Cardedratedate DATETIME
,expdate DATETIME
,term INT
,Condit CHAR(1)
)
;
--===== Allow Inserts into the IDENTITY column
SET IDENTITY_INSERT #Data ON
;
--===== Insert the test data into the test table
INSERT INTO #Data WITH(TABLOCK)
(H_account, seq, Cardedratedate, expdate, term, Condit )
SELECT 'eee555ww325', '1','2019-04-02 00:00:00.000','2019-10-02 00:00:00.000', '6','F' UNION ALL
SELECT 'eee555ww325', '2','2017-04-01 00:00:00.000','2019-04-01 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325', '3','2015-03-28 00:00:00.000','2017-03-31 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325', '7','2014-05-12 00:00:00.000','2015-03-13 00:00:00.000','10','D' UNION ALL
SELECT 'eee555ww325', '8','2014-04-11 00:00:00.000','2015-03-13 00:00:00.000','11','D' UNION ALL
SELECT 'eee555ww325', '5','2014-05-23 00:00:00.000','2015-03-13 00:00:00.000','10','F' UNION ALL
SELECT 'eee555ww325', '6','2014-05-13 00:00:00.000','2015-03-13 00:00:00.000','10','D' UNION ALL
SELECT 'eee555ww325', '9','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','D' UNION ALL
SELECT 'eee555ww325','10','2012-10-01 00:00:00.000','2017-10-01 00:00:00.000','60','D'
;
--===== Set the identity insert back to normal
SET IDENTITY_INSERT #Data OFF
;
GO
--=====================================================================================================================
-- Create and populate a table that shows the results I want
-- #Results
--NOTE: Where Seq = 5-9 are all part of the same term. But the customer has done something to their loan that has cause
-- it to split across seq and there for the terms are non standard. A standard HL term is 6,12,24,36,48,60 months
-- What I want the data to look like is:
--=====================================================================================================================
--===== If it exists, drop the test table to make reruns in SSMS easier.
SET IDENTITY_INSERT #Results OFF;
DROP TABLE IF EXISTS #Results
;
GO
--===== Create the test table
CREATE TABLE #Results
(
H_account NVARCHAR(255)
,seq INT IDENTITY(1,1)
,Cardedratedate DATETIME
,expdate DATETIME
,term INT
,Condit CHAR(1)
)
;
--===== All Inserts into the IDENTITY column
SET IDENTITY_INSERT #Results ON
;
--===== Insert the desired results into the table
INSERT INTO #Results WITH(TABLOCK)
(H_account, seq, Cardedratedate, expdate, term, Condit )
SELECT 'eee555ww325', '1','2019-04-02 00:00:00.000','2019-10-02 00:00:00.000', '6','F' UNION ALL
SELECT 'eee555ww325', '2','2017-04-01 00:00:00.000','2019-04-01 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325', '3','2015-03-28 00:00:00.000','2017-03-31 00:00:00.000','24','F' UNION ALL
SELECT 'eee555ww325', '5','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325', '6','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325', '7','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325', '8','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325', '9','2014-03-13 00:00:00.000','2015-03-13 00:00:00.000','12','F' UNION ALL
SELECT 'eee555ww325','10','2012-10-01 00:00:00.000','2017-10-01 00:00:00.000','60','D'
;
--===== Set the identity insert back to normal
SET IDENTITY_INSERT #Results OFF
;
GO
--=====================================================================================================================
-- Display the "Before'n'After"
--=====================================================================================================================
SELECT * FROM #Data ORDER BY H_account, seq;
SELECT * FROM #Results ORDER BY H_account, seq;
Shifting gears back to the original problem, will there always only be one "good" row per expdate? And, will that row always have thee earliest Cardedratedate date for that same set of rows?
--Jeff Moden
Change is inevitable... Change for the better is not.
June 14, 2021 at 12:50 am
Thanks Jeff,
Yes there will always be one good row for the sequences where the expdate is the same. And it will be the one with the highest sequence. The way the database works is when a New HL is drawn down then it is given the seq of 1. When there is a critical event on the HL (i.e. a new fixed term or rate change) then the first draw down seq goes to 2 and then new fixed term is seq 1.
S0 for the 1 example in this dummy data the seq that have the issue 5-9 the correct information is in seq 9, cardratedate, term etc. The only column which is incorrect is condit which is supposed to be 'F' which the correct information is in seq 5 the final record where the issue is present.
I hope that makes sense.
Thanks
June 14, 2021 at 2:43 am
Perfect. I think we may have a little work to do on the Condit but check this out. Remember, don't think about what you want to do to a row... think about what you want to do to a column. We might also have to add a check to make sure that we've found the row with the correct term but this should get us started down a simpler path. It DOES return the desired data as is.
WITH ctePreAgg AS
(
SELECT H_account
,seq
,MaxSeq = MAX(seq) OVER (PARTITION BY h_account)
,Cardedratedate = MIN(Cardedratedate) OVER (PARTITION BY h_account,expdate)
,expdate
FROM #Data
WHERE H_account = 'eee555ww325'
)
SELECT H_account
,seq
,Cardedratedate
,expdate
,term = DATEDIFF(mm,Cardedratedate,expdate)
,Condit = IIF(seq < MaxSeq,'F','D')
FROM ctePreAgg
ORDER BY h_account,seq
;
--Jeff Moden
Change is inevitable... Change for the better is not.
July 19, 2021 at 9:01 am
Thanks @jeff Moden
It did not email me your reply so I only saw this a few days ago. That code is interesting but I cannot use it unfortunately because there are heaps of home loans in the data that have condit = 'D' that can also have an expiry date the same as the record before but should remain as 'D' and are not actually fixed loans. It only occurs when the first fixed condition should be last condit = 'F' and all the following records for the same home loan are condit = 'D' with the same expiry date.
Any ideas how I can get this function working or for a nice simple clean code like above?
Thanks
Scott
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply