December 13, 2011 at 7:32 pm
Cadavre (12/12/2011)
Jim-720070 (12/12/2011)
Thanks for looking into a bit more deeply Cadavre.. I've been working with SQL Server for 5 years now and have literally never had anyone else look at my code to review.I was feeling like I was hitting the tables too many times as well.. Def worth another look! Haha..
I'm the same, although less experienced!
I've worked with SQL Server for 2 years, but the way I work is a little different to everyone I've worked with so far.
This is how I perform a task: -
1. I start with pen and paper and jot down a few algorithms. Normally, I end up with between 3 and 10 algorithms written down.
2. I'll then code up the 3 that I think are most likely to perform the best, as well as 1 RBAR algorithm.
3. I set-up a test environment of 100 rows based on the table structures and data set and run them all against it comparing the result-sets by inserting each one into temp tables. I repeat this test 10 times, assuming that each run produces the same results.
4. Once all of my algorithms have passed the first stage of testing, I recreate my test environment based on the table structures and data set but this time add 1,000,000 rows and test again. I repeat this test 5 times, assuming that each run produces the same results.
5. Once I have the two fastest, I take a sub-set of the actual data and test again, this is not repeated.
6. Assuming the results are the same, I do a final test on the actual tables that will be used. I repeat this test 5 times.
7. Finally, I do a logic check of the fastest algorithm, then submit the code into the correct branch of our software.
It makes development take slightly longer for me than for some others, but it also means that when I submit code I can be confident that it will scale well and performs as expected.
Gosh... sounds like I'm listening to myself. I do essentially the same thing except I jot down step 1 in the form of comments in the SQL Editor window.
I wish more folks would take the attitude you have. So many don't understand how much time doing it right the first time saves even if it takes a little extra time. It's a real problem that many just don't understand. The time not spent up front is magnified by a factor of 8 in rework. Conversly, spending time upfront to do it right saves you much more time on the tail end of a project which actually lets you start working on the next project early.
--Jeff Moden
Change is inevitable... Change for the better is not.
December 13, 2011 at 9:33 pm
drew.allen (12/6/2011)
* You're using 9999 to represent unknown/missing data. The NULL value was specifically designed to represent unknown/missing data. By using a non-NULL value to represent NULL, you've made it impossible to distinguish between records where the value is known to be that non-NULL value (however unlikely it is to occur) and records where the value is truly unknown/missing.Drew
Actually, for date ranges like StartDate and EndDate, NULL can be a bit of a PITA insofar as setting criteria in a WHERE clause. I use 9999-12-30 (NOT 9999-12-31 because I need "outliers" to be picked up by certain calculations using "somedate+1").
--Jeff Moden
Change is inevitable... Change for the better is not.
December 13, 2011 at 10:29 pm
Okidoki... First, please forgive my OCD nature when it comes to formatting. I just couldn't look at multiple INSERT/VALUES or all lower case object names anymore. These old eyes just can't handle all one case any more. 😀
Second, I don't know if this is going to be any faster, but the right indexes on the two tables could really help make this code snappy because of the comparatively very short and simple code.
It's very flexible code. You can change the days since the last job, the days where the user has to be active, and the number of days that must go by to send a letter every "x" days. Some folks call it a "Parameterized View" but it's really just an iTVF.
Third, as is normal for me, how this all works is in the comments in the code. Let me know what you think.
--=================================================================================================
-- Create the test tables
-- THIS SECTION OF CODE IS NOT A PART OF THE SOLUTION!!!
--=================================================================================================
--===== Do this in a nice safe place that everyone has because we drop tables for test purposes.
USE TempDB;
--===== Conditionally drop the test tables to make reruns in SSMS easier.
-- Note that we're using "real" tables in TempDB to simulate the actual tables because
-- you can't use Temp Tables in a function and I want to demonstrate how to make this
-- code almost infinitely flexible as a "parameterized view" using an iTVF.
IF OBJECT_ID('TempDB.dbo.Users' ,'U') IS NOT NULL DROP TABLE dbo.Users;
IF OBJECT_ID('TempDB.dbo.WorkExperience','U') IS NOT NULL DROP TABLE dbo.WorkExperience;
--===== Create the test tables
CREATE TABLE dbo.Users
(
ID INT IDENTITY(1,1),
FirstName VARCHAR(20) NOT NULL,
LastLogin DATETIME NULL, --I changed this for 2005 and up so others may play ;-)
InActive INT NOT NULL
)
;
CREATE TABLE dbo.WorkExperience
(
ID INT IDENTITY(1,1),
UserID INT,
JobName VARCHAR(20) NULL,
TimeStartYear INT,
TimeStartMonth INT,
TimeEndYear INT,
TimeEndMonth INT
)
;
--=================================================================================================
-- Populate the test tables with test data
-- THIS SECTION OF CODE IS NOT A PART OF THE SOLUTION!!!
--=================================================================================================
INSERT INTO dbo.Users
(FirstName, LastLogin, InActive)
SELECT 'Peter' ,'20111201',0 UNION ALL
SELECT 'Robert' ,'20111202',1 UNION ALL
SELECT 'Johan' ,'20111203',0 UNION ALL
SELECT 'Noah' ,'20111204',0 UNION ALL
SELECT 'David' ,'20001205',0 UNION ALL
SELECT 'Evil Twin' ,'20111204',0 UNION ALL
SELECT 'Nofun' ,'20111204',0 UNION ALL
SELECT 'Marge' ,'20111203',0 --Added User
;
INSERT INTO dbo.WorkExperience
(UserID, JobName, TimeStartYear, TimeStartMonth, TimeEndYear, TimeEndMonth)
SELECT 1, 'Manager' ,2011,1,9999,9999 UNION ALL
SELECT 2, 'Pilot' ,2011,2,2011,5 UNION ALL
SELECT 3, 'Clown' ,2011,3,2011,11 UNION ALL
SELECT 3, 'Librarian' ,2010,5,2010,12 UNION ALL
SELECT 3, 'Accountant' ,2010,6,2010,7 UNION ALL
SELECT 4, 'Cattle Driver' ,2006,1,9999,9999 UNION ALL
SELECT 4, 'Farmer' ,2006,1,2009,7 UNION ALL
SELECT 5, 'Driver' ,2007,4,2008,5 UNION ALL
SELECT 6, 'Emperor' ,2006,1,2009,7 UNION ALL
SELECT 6, 'Conqueror' ,2006,1,2009,7 UNION ALL
SELECT 6, 'Sailor' ,2003,1,2004,7 UNION ALL
SELECT 6, 'Captain' ,2002,1,2005,7 UNION ALL
SELECT 7, 'Jailor' ,2005,1,2011,9 UNION ALL
SELECT 8, 'Clown' ,2011,3,2011,1 UNION ALL --Added data, User will be returned
SELECT 8, 'Librarian' ,2010,5,2010,12 UNION ALL --Added data, User will be returned
SELECT 8, 'Accountant' ,2010,6,2010,7 --Added data, User will be returned
;
GO
--=================================================================================================
-- Build a "programmable" view to solve this problem.
-- Yeah... this is definitly part of the solution. ;-)
-- You only need to build this once in production.
--=================================================================================================
--===== Conditionally drop the permanent function we're getting ready to make
IF OBJECT_ID('TempDB.dbo.SendLettersFor','IF') IS NOT NULL DROP FUNCTION dbo.SendLettersFor;
GO
CREATE FUNCTION dbo.SendLettersFor
/**************************************************************************************************
Purpose:
This Inline Table Value Function (iTVF) acts as a "programmable" or "parameterized" view and makes
selections based on the following criteria...
1. User must have logged in within the last @LoginMonths.
2. User must not have the "InActive" flag set. (User must be "Active")
3. Latest job for each User must be >= @LastJobMonths ago.
4. User selected only every @SendEveryXDays since last job end.
Usage Example: (of course, you should use "*")
SELECT * FROM dbo.SendLettersFor(@LoginMonths, @LastJobMonths, @SendEveryXDays)
PROGRAMMER NOTES:
1. If you want tied "end dates" for a give User to produce a row for each, change the ROW_NUMBER()
to RANK() or DENSE_RANK(). Otherwise, only one row per user will be returned with one of the
multiple jobs they may have had a tie for as a last job.
Revision History:
Rev 00 - 13 Dec 2011 - Jeff Moden - Initial Creation
**************************************************************************************************/
--=================================================================================================
-- Presets
--=================================================================================================
--===== Define the function I/0
(
@LoginMonths INT,
@LastJobMonths INT,
@SendEveryXDays INT
)
RETURNS TABLE
--WITH SCHEMABINDING AS --Uncomment this line if you put this function into production
RETURN
--=================================================================================================
-- Produce the return IAW the criteria given in the header
--=================================================================================================
WITH
cteNormalizeDates AS
( --=== Convert the INTEGER dates back to DATETIME dates
SELECT we.UserID,
we.JobName,
u.FirstName,
StartDate = DATEADD(mm,we.TimeStartYear*12-22801+we.TimeStartMonth,0),
EndDate = DATEADD(mm,we.TimeEndYear*12-22801+ISNULL(NULLIF(we.TimeEndMonth,9999),12),0)
FROM dbo.WorkExperience we
INNER JOIN dbo.Users u
ON we.UserID = u.ID
WHERE u.Inactive = 0 --User is NOT inactive
AND u.LastLogin > DATEADD(mm,-@LoginMonths,GETDATE()) --User has activity in last @LoginMonths months
),
cteSequenceDates AS
( --=== Number the EndDates in reverse order. DateSequence = 1 = Latest EndDate per User
SELECT DateSequence = ROW_NUMBER() OVER (PARTITION BY UserID ORDER BY EndDate DESC),
UserID, JobName, FirstName, StartDate, EndDate
FROM cteNormalizeDates
)
SELECT UserID, JobName, FirstName, StartDate, EndDate,
DaysSinceLastJob = DATEDIFF(dd,EndDate,GETDATE())
FROM cteSequenceDates
WHERE DateSequence = 1 --Lastest Job EndDate
AND EndDate <= DATEADD(mm,-@LastJobMonths,GETDATE()) --Last job @LastJobMonths months or more ago
AND DATEDIFF(dd,EndDate,0)%@SendEveryXDays = 0 --Send Letter every @SendEveryXDays from last job
;
GO
--=================================================================================================
-- Once the function is built, demonstrate the function usage according to the given criteria.
--=================================================================================================
SELECT *
FROM dbo.SendLettersFor(6,6,1)
;
GO
--Jeff Moden
Change is inevitable... Change for the better is not.
December 14, 2011 at 1:45 am
December 14, 2011 at 1:50 am
Jeff Moden (12/13/2011)
Okidoki... First, please forgive my OCD nature when it comes to formatting. I just couldn't look at multiple INSERT/VALUES or all lower case object names anymore. These old eyes just can't handle all one case any more. 😀Second, I don't know if this is going to be any faster, but the right indexes on the two tables could really help make this code snappy because of the comparatively very short and simple code.
It's very flexible code. You can change the days since the last job, the days where the user has to be active, and the number of days that must go by to send a letter every "x" days. Some folks call it a "Parameterized View" but it's really just an iTVF.
Damn, I deleted my test set-up for this 😀
I had considered something similar, only using a CLR. I've never come across a View used in this way, so time for some investigations (well, later on when I'm not as busy!). Hopefully the OP will post back with time and IO stats on his actual data.
December 14, 2011 at 6:23 am
Cadavre (12/14/2011)
Jeff Moden (12/13/2011)
I've never come across a View used in this way, ...
Just to be clear, it's not really a "view". It's actually an Inline Table Value Function which can be made to work as if it were a "parameterized view".
As a bit of a side bar, sometimes it IS better to split the code up rather than trying to do it all in a single query. "Set based" code doesn't mean "all in one query". I don't believe that's true in this case but I did want to put that out.
--Jeff Moden
Change is inevitable... Change for the better is not.
December 14, 2011 at 6:36 am
Jeff Moden (12/14/2011)
Just to be clear, it's not really a "view". It's actually an Inline Table Value Function which can be made to work as if it were a "parameterized view".
Sorry, yes I understand. I referred to it as a view because when I read your code the first time I thought that DDL wise it was a view. Obviously upon re-reading I can see it isn't.
Jeff Moden (12/14/2011)
As a bit of a side bar, sometimes it IS better to split the code up rather than trying to do it all in a single query. "Set based" code doesn't mean "all in one query". I don't believe that's true in this case but I did want to put that out.
Agreed 100%. The number of times I find an extremely complicated piece of T-SQL that when split apart performs much better is astounding. Unfortunately, the reverse is true about as often (e.g. where some T-SQL has been split into many smaller chunks because the developer was thinking in OO terms instead of in sets).
Viewing 7 posts - 31 through 36 (of 36 total)
You must be logged in to reply to this topic. Login to reply