February 7, 2011 at 2:27 pm
The added complexity is because my procedure always adjusts the sequence number so that it is continuous and zero based.
James,
While it is true that your routine does this, it seems to me more proper to put that aspect of your code into the Delete routine instead of renumbering everything every time you change the priority on one task.
--
JimFive
February 7, 2011 at 2:57 pm
James,
I would agree that maybe the delete routine would be a better place for the zero re-numbering and that this does over-complicate the routine somewhat.
James
James
MCM [@TheSQLPimp]
February 7, 2011 at 11:04 pm
Cool solution for custom sequences. If you don't have immediate application needs, you could just wait for Denali (SQL 11) to go RTM and use the new Sequence Object natively in SQL Server ...
February 8, 2011 at 2:15 am
Oh I'm looking forward to that! Currently looking into the new datetimeoffset data type in 2008 to replace my "convert to local date via a utc hour and minute offset lookup table" function! Looks to be the way forward if you deal with data from multiple time zones. I feel amother article coming on this once I've looked into the pros and cons - as let's face it, there are always some cons! Ever tried using the xml nodes method with a very large xml blob? Not good, the old prepare doc and openxml method is way faster but you only find these things out from testing.
Thanks again for the supportive comments.
Cheers, James
James
MCM [@TheSQLPimp]
February 8, 2011 at 9:38 am
I have used a similar procedure to move records in a sequence. I won't repeat the table setup, but I need to point out that I used "1" as the starting value rather than "0" because people normally think of the top item as "1". Here is the procedure I use, formatted to work with the table in the sample.
The procedure allow the caller to specifiy either a direction ("TOP", "UP", "DOWN", or "BOTTOM") or a specific sequence number (move item x to sequence number 5).
SETANSI_NULLS ON
SETANSI_PADDING ON
IFOBJECTPROPERTY(OBJECT_ID('ToDoItemMoveRecord'), 'IsProcedure') = 1
DROP
PROCToDoItemMoveRecord
GO
CREATE
PROCToDoItemMoveRecord
@ToDoIDINTEGER,
@DirectionVARCHAR(10)= NULL,
@NewSortSeqTINYINT= NULL
AS
-----------------------------------------------------------------------------------------------------------
SETNOCOUNTON
DECLARE@ErrNumINTEGER,@ErrDescVARCHAR(2000),
@ErrRowsINTEGER,@ErrSeverityINTEGER,
@CurrentSeqTINYINT,@MaxSeqTINYINT,
@StartSeqTINYINT,@EndSeqTINYINT,
@PersonIDINTEGER
BEGINTRY
--<logic>Verify the parameters.</logic>
SET@Direction = UPPER(@Direction)
IF@NewSortSeq IS NULL
AND@Direction IS NULL
RAISERROR('Please specify either a new SortSeq or a Direction to move.', 16, 1)
IF@NewSortSeq IS NOT NULL
AND@Direction IS NOT NULL
RAISERROR('Please specify either a new SortSeq or a Direction to move, but not both.', 16, 1)
--<logic>Locate the current values from the table.</logic>
SET@PersonID =
(SELECT PersonID FROM ToDoList WHERE ToDoID = @ToDoID)
SET@CurrentSeq =
(SELECT todoSequence FROM ToDoList WHERE ToDoID = @ToDoID)
SET@MaxSeq =
(SELECT MAX(todoSequence) FROM ToDoList WHERE PersonID = @PersonID)
--<logic>If the new sort sequence is provided, determine the direction of the move.</logic>
IF@NewSortSeq IS NOT NULL
BEGIN-- Specific sort
IF@NewSortSeq < 1
BEGIN
RAISERROR('The requested sort sequence cannot be less than 1.', 16, 1)
END
ELSE IF@NewSortSeq > @MaxSeq
BEGIN
RAISERROR('The requested sort sequence cannot be more than the current maximum in the table.', 16, 1)
END
ELSE IF@NewSortSeq = 1
BEGIN
SET@Direction = 'TOP'
SET@StartSeq = @NewSortSeq
SET@EndSeq = @CurrentSeq
END
ELSE IF@NewSortSeq < @CurrentSeq
BEGIN
SET@Direction = 'UP'
SET@StartSeq = @NewSortSeq
SET@EndSeq = @CurrentSeq
END
ELSE IF@NewSortSeq = @MaxSeq
BEGIN
SET@Direction = 'BOTTOM'
SET@StartSeq = @CurrentSeq
SET@EndSeq = @NewSortSeq
END
ELSE IF@NewSortSeq > @CurrentSeq
BEGIN
SET@Direction = 'DOWN'
SET@StartSeq = @CurrentSeq
SET@EndSeq = @NewSortSeq
END
ELSE
BEGIN
RAISERROR('The necessary move could not be determined.', 16, 1)
END
END-- Specific sort
--<logic>If a move direction is requested, determine the new sort sequence.</logic>
ELSE
BEGIN-- Direction requested
IF@Direction = 'TOP'
BEGIN
SET@StartSeq = 1
SET@EndSeq = @CurrentSeq
SET@NewSortSeq = @StartSeq
END
ELSE IF@Direction = 'UP'
BEGIN
SET@StartSeq = @CurrentSeq - 1
SET@EndSeq = @CurrentSeq
SET@NewSortSeq = @StartSeq
END
ELSE IF@Direction = 'DOWN'
BEGIN
SET@StartSeq = @CurrentSeq
SET@EndSeq = @CurrentSeq + 1
SET@NewSortSeq = @EndSeq
END
ELSE IF@Direction = 'BOTTOM'
BEGIN
SET@StartSeq = @CurrentSeq
SET@EndSeq = @MaxSeq
SET@NewSortSeq = @EndSeq
END
END-- Direction requested
--<logic>Check to be sure the requested move can be made; raise an error if it cannot.</logic>
IF@NewSortSeq IS NULL
BEGIN-- Invalid record
RAISERROR('Record not found.', 16, 1)
END-- Invalid record
IF@CurrentSeq = 1 AND @Direction IN ('UP', 'TOP')
BEGIN-- Invalid record
RAISERROR('The record is currently at the top and cannot move up.', 16, 1)
END-- Invalid record
IF@CurrentSeq = @MaxSeq AND @Direction IN ('DOWN', 'BOTTOM')
BEGIN-- Invalid record
RAISERROR('The record is currently at the bottom and cannot move down.', 16, 1)
END-- Invalid record
IF@NewSortSeq = @CurrentSeq
BEGIN-- Invalid record
RAISERROR('The record is currently at the requested position and cannot be moved.', 16, 1)
END-- Invalid record
/*-- Testing
select@DirectionAS '@Direction',
@NewSortSeqAS '@NewSortSeq',
@CurrentseqAS '@CurrentSeq',
@MaxSeqAS '@MaxSeq',
@StartSeqAS '@StartSeq',
@EndSeqAS '@EndSeq'
*/
--<logic> - Move the necessary records.</logic>
UPDATEToDoList
SETtodoSequence = CASE
WHEN todoSequence = @CurrentSeq THEN @NewSortSeq
WHEN @Direction IN ('TOP', 'UP') THEN todoSequence + 1
WHEN @Direction IN ('DOWN', 'BOTTOM') THEN todoSequence - 1
ELSE NULL END -- Null is used so that an error will occur if the three cases above are not valid.
WHEREtodoSequence BETWEEN @StartSeq AND @EndSeq
ENDTRY
BEGINCATCH
SET@ErrNum = ERROR_NUMBER() -- Needed for return value
SET@ErrSeverity = ERROR_SEVERITY() -- Needed to bubble up the same severity
SET@ErrDesc = dbo.FormatErrDesc()
GOTONonTranError
ENDCATCH
RETURN
NonTranError:
RAISERROR (@ErrDesc, @ErrSeverity, 1)
RETURN@ErrNum-- Return the error number
GO
GRANTEXECUTE ON ToDoItemMoveRecord TO Public
GO
February 8, 2011 at 9:54 am
James,
I really liked this article. I especially appreciated how you spelled out the logic in text. Many articles make you puzzle it out in the code. Thank you.
Amy
February 8, 2011 at 9:55 am
I think I used zero as the starting sequence due to all the C# programming I have been doing lately - and the hardened C# community would surely spit on anyone starting a sequence or array with 1 - the mere thought of it!!! 😉
After reading the forum I do have started to come to the conclusion that my procedure was maybe overkill in the terms of the scope of what it was trying to do - i.e. both moving the record to the desired position and renumbering the whole sequence in the same process. I am now of a mind to do the gap removal and resequencing to zero in the delete procedure and then change the proc to be more like yours and another one that was posted.
I don't have as much validation logic in as yours though, as when I last implemented this the front-end code took care of that. Nice proc though, rare nowadays to see such good error checking! Thanks for taking the time to read the article.
James
MCM [@TheSQLPimp]
February 8, 2011 at 9:59 am
Thank you Amy and apologies for the initial typo (which has now been corrected). I have learned a lot about the article writing style from the comments and the whole process has been beneficial for me. I have another one published on Thursday showing how to auto-generate stored procedures and I have triple-checked the attached code to make sure it is 100% correct!
Again, thanks for taking the time to comment, I really appreciate it.
James
MCM [@TheSQLPimp]
February 8, 2011 at 1:21 pm
fahey.jonathan (2/8/2011)
I have used a similar procedure to move records in a sequence....
Jonathan's code (above) made sense to me. But I couldn't really figure out what was going on in the original article's case statement. 🙁 It makes sense conceptually, but the specifics of the code were too funky for me to parse. Partly this is because I'm new to this stuff, but also partly because the Jonathan's code is so much more nicely formatted and easy to reconcile. I'd humbly request that the article author reformat his code so it's more readable.
And also, please note that as of the time of this posting, the incorrect column name ("todoGroup") is still specified in the original article.
February 8, 2011 at 2:06 pm
Unfortunately the funky code is very efficient. The problem with stored procedures with a lot of IF statements in is that they do not cache well, as the compiler stores a different plan each time a new path is followed, making for poor reuse. The code is complicated to make it efficient. Apolgies if it is hard to follow. I will change the typo as soon as I can.
Cheers, James
James
MCM [@TheSQLPimp]
February 8, 2011 at 2:55 pm
The original code updates every record for the person, irrespective of whether it changes or not. I think updating only the rows that change makes up for any inefficiency with an "IF" statement.
I put the two procedures in the database and looked at the execution plans. It is not always an accurate reflection of execution time, I know, but it does provide very useful information. The original procedure was expected to take 66% of the time and the revised procedure was expected to take 34%. The compiler seems to think the "IF" statements are not a significant detriment.
I have one other thing to note. The original procedure accepts as parameters both the PersonID and the ToDoID. The PersonID is dependent on the ToDoID, so either the procedure should not accept the PersonID as a parameter and just look it up, or it should validate that the PersonID passed was the correct PersonID for the ToDoID.
February 8, 2011 at 3:10 pm
Again, this is because my procedure also removes gaps in the sequence, hence why it updates all the records. If you try my procedure with different parameters and look at the cache hits it will always hit, whereas the other one will recompile for different parameters combinations. As I stated in an earlier post, I will remove the resequencing to remove gaps and restate at zero into the delete routine, enabling my procedure to also work on a range and drop drastically in complexity. Apologies if you have not read back this far. My procedure suffered by being to all-encapsulating, as if you had a broken sequence due to deletes of say 1,5,7,11,13, mine would set this back to 0,1,2,3,4, whereas the others wouldn't - hence why it is complex and hence why it updates all records.
Cheers, james
James
MCM [@TheSQLPimp]
February 8, 2011 at 3:23 pm
Apolgies, didn't answer the todoid and personid question. I'd expect the business/application layer to do this, as if the application was passing me a todoid for the wrong personid then I'd be very concerned. Some business logic in the database layer is good but validation such as this should be done at a higher level. Same goes for "this is already the first one" etc.
Cheers, James
James
MCM [@TheSQLPimp]
February 8, 2011 at 3:50 pm
thanks for the article.
Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events
February 9, 2011 at 2:10 am
Thank you. It was a great learning experience for me in terms of writing style and content. All the feedback was beneficial and my future articles will surely improve as a result!
Cheers, James
James
MCM [@TheSQLPimp]
Viewing 15 posts - 16 through 30 (of 36 total)
You must be logged in to reply to this topic. Login to reply