Swapping and nudging items
-
It's amazing how much of a brain activity one simplest task can generate.
Let's say we have a table with a primary key (for simplicity it's char(1)
)and a column containing some logical order for the items. Sort column is unique
but not continuos dues to some deleted records. The table looks like this:
create table testsort(
id char(1),
sort int)
-- And some sample data
insert test
select 'A', 5 union
select 'B', 7 union
select 'C', 11 union
select 'D', 13 union
select 'E', 42I needed T-SQL code that would allow me to a) swap order of two items and b) move item
up or down in the sort order. Sounds simple, eh? I assigned the task to a junior developer
on our team and 4 hours later found one shocker of a code (this forum has G rating
preventing me from publishing the code). I pointed out to the dude that there must be a better
way to accomplish the task and, most likely, it can be done in one statement. The dude
did not believe me, it was late Friday afternoon so we called it a day.
Well, now I probably have to eat my words. Simple requirement yet very elusive solution.
I spent few hours this weekend trying to solve the puzzle and this is the best I could come up with:
create procedure usp_SwapItems(@id1 char(1), @id2 char(1))
as
update
t1
set
sort = t2.sort
from
testsort t1, testsort t2
where
(t1.id = @id1 and t2.id = @id2)
or (t1.id = @id2 and t2.id = @id1)So far so good for swapping. But this is my code for nudging items (after few hours of sweating):
create procedure dbo.usp_MoveItem(@id char(1), @direction int)
-- @direction = -1 move up
-- @direction = 1 move down
as
declare @id2 char(1)
-- When moving down we have to find first item
-- with sort order greater than the current one
-- For moving up we use @direction variable
-- to change the sign of sort column so that
-- task remains the same (without the change of
-- sign we would have to find last column
-- with the sort order less than current one)
select top 1
@id2 = id
from
testsort
where
@direction * sort >
@direction * (select sort from testsort where id = @id)
order by
@direction * sort asc
exec usp_SwapItems @id, @id2
Firstly, there must be a more elegant way to accomplish the task, no doubt about that.
I could probably collapse second procedure into one statement but it probably would not
do any good.
While crunching the problem, I started thinking about what criteria define a great solution.
So far I came up with three items but there could be more:
- Readability. It must be possible to understand code without going over and over. Sometimes
it means splitting your statements (without impact on performance, of course), sometimes
it means more comments (T-SQL is absolutely fantastic for it's ability to mix code and comment
lines)
- Effectiveness. Code must do one thing and do it well. In SQL it usually translates to performance
but factors like locks, memory, etc must be taken into account as well. Sometimes even
making code longer helps: I've seen situations where code selecting on "where (a = b or c = d)"
was performing much worse than two selects with union.
- Elegancy. The 'whoa!' factor of the code. The strike of genius (which is nowhere to be seen
in the code above
). Elegant solutions are usually the fastest, the most readable and the most effective.
Anyway, question to all the gurus here (and I've seen a few): what would be your version of the code
that satisfies these requirements?
--
georged
Edited by - georgedo on 11/29/2003 07:24:05 AM
--
George - Readability. It must be possible to understand code without going over and over. Sometimes
-
create procedure usp_SwapItems
@id1 char(1), @id2 char(1)
as
update testsort
set sort =
case when id = @id1
then (SELECT sort FROM testsort WHERE id = @id2)
else (SELECT sort FROM testsort WHERE id = @id1)
END
WHERE
id IN (@id1, @id2)
-
CREATE PROCEDURE usp_MoveItem
@id char(1),
@direction int
-- @direction = -1 move up
-- @direction = 1 move down
AS
DECLARE @mysort int
DECLARE @swapid char(1)
-- check for anomalies
IF @id IS NULL RETURN
IF @direction NOT IN ( -1, 1 ) RETURN
IF (SELECT COUNT(1) FROM testsort) < 2 RETURN
SELECT @mysort = sort
FROM testsort
WHERE id = @id
IF @mysort IS NULL
RETURN -- for whatever reason
IF @direction > 0
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort > @mysort)
ELSE
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort < @mysort)
IF @swapid IS NULL RETURN
EXEC usp_SwapItems @id, @swapid
-
I had to do this a couple of years ago. I went for a slightly different solution.
I allowed the users to control the position in the list, by specifying the number in the sort column, and then built a SP to parse through the list using a cursor and then to renunber the sort figures with an interval of 10.
So to move an item up, I changed just its number, and then ran the renumber routine which then reset the sort figures.
Thus:
declare @intSort integer
declare curse cursor
for
select char, sort
from testsort
order by sort
select @intSort = 10
open curse
fetch next from curse
while @@fetch_status = 0
begin
update testsort
set sort = @intSort
where current of curse
select @intsort = @intsort + 10
fetch next from curse
end
close curse
deallocate curse
Now all you have to do is change the sort number of the item(s) you want to move and then call the resort SP and it will then fix the sort numbers for you.
-
dkretz: I'm not sure how your code is different from mine (apart from couple error checks and one bug
). I guess it would be helpful if you explain what you were trying to achieve by making code more verbose, e.g. performance gains. --
georged
--
George -
GCottle: Renumbering approach did cross my mind however I envisaged it to be done without cursors. Our data can be few thousands items (which, I have to admit, I failed to mention) and cursoring would bring performance to a grinding halt. I'm not an expert on locking but looks like locks are also going to be an issue.
--
georged
--
George -
quote:
envisaged it to be done without cursorsOne way would be to create a temp table with an additional IDENTITY column. Insert from your table with an ORDER BY clause to get the right sequence. Update original table using the temp table and use small number of rows at a time if locking is a problem.
I have several tables in different databases that have a sequenceid and allow users to move items (ie change sequence) and use the seqenceid to do it. In a menu system I use the following code to action all moves. If I want to delete a row, I move the row to the bottom and then delete it, therefore keeping the incrementation intact.
create procedure ccc_menuitemmove01_set @MenuID int,
@SeqIDFrom int,
@SeqIDTo int
as
IF (@SeqIDFrom < @SeqIDTo)
BEGIN
UPDATEa
SETa.SeqID = (CASE WHEN a.SeqID = @SeqIDFrom THEN b.SeqID
ELSE a.SeqID-1
END)
FROMMenuItem a
INNER JOIN MenuItem b
ON b.MenuID = a.MenuID
AND b.SeqID = @SeqIDTo
WHEREa.MenuID = @MenuID
ANDa.SeqID >= @SeqIDFrom
ANDa.SeqID <= @SeqIDTo
END
ELSE
BEGIN
UPDATEa
SETa.SeqID = (CASE WHEN a.SeqID = @SeqIDFrom THEN b.SeqID
ELSE a.SeqID+1
END)
FROMMenuItem a
INNER JOIN MenuItem b
ON b.MenuID = a.MenuID
AND b.SeqID = @SeqIDTo
WHEREa.MenuID = @MenuID
ANDa.SeqID >= @SeqIDTo
ANDa.SeqID <= @SeqIDFrom
END
GO
Far away is close at hand in the images of elsewhere.
Anon. -
sometimes the best solution is the simplest
CREATE PROCEDURE Usp_MoverItemTest1 (@id char(1),@Direction char(1))
As
SET NOCOUNT ON
DECLARE @sort int, @id2 char(1)
SELECT @sort = sort
FROM testsort
WHERE id = @id
IF @direction = 'U'
BEGIN
EXEC usp_moveitemtestup @sort , @id2 output
END
IF @Direction = 'D'
BEGIN
EXEC usp_moveitemtestdown @sort , @id2 output
END
EXEC Usp_SwapItems @id, @id2
GO
CREATE PROCEDURE Usp_MoveItemTestup (@sort int,@id char(1) output)
As
SET NOCOUNT ON
SELECT TOP 1 @id = id
FROM testsort
WHERE sort > @sort
ORDER BY ID ASC
GO
CREATE PROCEDURE Usp_MoveItemTestdown (@sort int,@id char(1) output)
As
SET NOCOUNT ON
SELECT TOP 1 @id = id
FROM testsort
WHERE sort < @sort
ORDER BY ID DESC
GO
now put an index on both fields and we are set...
a) easy to maintain - any novice in SQL can understand the code
b) the selects will all be seeks if the appropriate indexes were created , this is more than twice as fast as the original with the indexes.
-
quote:
sometimes the best solution is the simplest...
a) easy to maintain - any novice in SQL can understand the code
b) the selects will all be seeks if the appropriate indexes were created , this is more than twice as fast as the original with the indexes.
Point a) taken - in the absence of "whoa!" code more verbose solution is probably better.
Could you please elaborate on b) - why do you think that your approach is faster? From
what I can see, both procedures will use same index on (sort). But I wonder if covered index
on (sort, id) existed then it can be used thus avoiding data pages?
--
georged
--
George -
Glad to,
Generally constructs like these
@direction * sort make the where clause unsargable -
you can check the query plan...create indexes on id & sort fields seperately ...
( composite won't work since the two columns are being accessed seperately )..
Running the query will show that the subquery on Id will be a seek , however the query with the @direction * sort construct will always be a index scan.
what is acceptable performance now with a few rows will get increasingly unscalable as the rows add up...
-
create procedure usp_SwapItems
@id1 char(1), @id2 char(1)
as
UPDATE testsort
SET sort = CASE
WHEN id = @id1 (
SELECT sort
FROM testsort
WHERE id = @id2)
ELSE (
SELECT sort
FROM testsort
WHERE id = @id1)
END
WHERE
id IN (@id1, @id2)I wish I knew how indenting worked here....
Differences - I sure thought mine was more directly reflective of a statement of the problem than yours - your "WHERE" clause I would think is less readable.
I guess if you aren't familiar with CASE syntax that part might be confusing, but I would think this would be the most common way to express "If...Then.." type logic. And the subqueries provide a form of conceptual encapsulation that stands pretty well alone.
I guess anything you spend time on thinking out becomes "most readable" for you. Worst case, the first problem is probably close.
The second problem...
IF @direction > 0
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort > @mysort)
ELSE
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort < @mysort)
Here is the second one with the optional error-checking stripped out - you simply didn't have any...
This is a straightforward IF ... THEN ...
statement, which is about as understandable as you can get, and maps to the problem statement - neither applies to yours.
Your calculations on direction are clever I guess, but add confusion and gain no efficiency...I thought that was possibly contrary to your "clarity" goal, and you'd thrown it in there to give us something easy to pick on.
Further, your WHERE clause with an expression in it simply won't optimize, so that's a bad idea too.
-
I'm going to reconsider here a bit. For the swap procedure, I'm going to go with:
CREATE PROCEDURE usp_SwapItems
@id1 CHAR(1),
@id2 CHAR(2)
AS
DECLARE iHold INT
BEGIN TRAN
SELECT @iHold = sort
FROM testsort
WHERE id = @id1
UPDATE testsort
SET sort = (
SELECT sort
FROM testsort
WHERE id = @id2 )
WHERE id = @id1
UPDATE testsort
SET sort = @iHold
WHERE id = @id2
COMMIT TRAN
The logic is clear, and it will unquestionably be optimized.
All else is vanity.
Normally I think it can be assumed that it is an optimization win to conflate multiple steps into a single query, but in this case, I don't think so.
-
Grasshopper posted usp_MoveItem containing the following snippet
IF @direction > 0SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort > @mysort)
ELSE
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort < @mysort)
Note that the last WHERE clause here should select MAX(sort) rather than MIN
IF @direction > 0SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MIN(sort) FROM testsort WHERE sort > @mysort)
ELSE
SELECT @swapid = id
FROM testsort
WHERE sort = (SELECT MAX(sort) FROM testsort WHERE sort < @mysort)
-
Here's my 2 cents
CREATE PROCEDURE dbo.usp_MoveItem(@id1 char(1), @direction bit)-- @direction = 1move up
-- @direction = 0move down
AS
-- Declare my variables. Note used @id1 in procedure defintion
-- instead of @id to make the code more readable.
DECLARE @id2 char(1)
DECLARE @sort1 int
DECLARE @sort2 int
-- Get sort values
SELECT @sort1 = sort FROM testsort WHERE id = @id1
-- Get id2 and sort2 values at the same time to avoid an extra lookup
IF @direction = 1
SELECT TOP 1 @id2 = id, @sort2 = sort FROM testsort WHERE sort > @sort1 ORDER BY sort asc
ELSE
SELECT TOP 1 @id2 = id, @sort2 = sort FROM testsort WHERE sort < @sort1 ORDER BY sort desc
-- Perform update here. This saves a procedure call and with the
-- select above we save the extra lookup to get the sort2 value.
UPDATE testsort
SET sort = CASE id WHEN @id1 THEN @sort2
WHEN @id2 THEN @sort1
END
WHERE id in (@id1, @id2)
Kenneth FisherI was once offered a wizards hat but it got in the way of my dunce cap.--------------------------------------------------------------------------------For better, quicker answers on T-SQL questions, click on the following... http://www.sqlservercentral.com/articles/Best+Practices/61537/[/url]For better answers on performance questions, click on the following... http://www.sqlservercentral.com/articles/SQLServerCentral/66909/[/url]Link to my Blog Post --> www.SQLStudies.com[/url]
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply