SQL Server while loop not printing last value

Question

I have below code, why i am not trying to get the last value in the temp table for @waitlistitmeID

10011 is not getting stored, though its len is 19 and it should be inserted. Need help in the query.

DECLARE @pos INT
    DECLARE @len INT
    DECLARE @value VARCHAR(MAX)
    declare @WaitlistItemID varchar(max)='10008,10009,10010,10011'
    declare @MovingWaitListItems table ( WaitlistItemID int, WaitlistItemGUID numeric(16,0))

    set @pos = 0
    set @len = 0

    while CHARINDEX(',', @WaitlistItemID, @pos+1)>0
    BEGIN
        set @len = CHARINDEX(',', @WaitlistItemID, @pos+1) - @pos
        set @value = SUBSTRING(@WaitlistItemID, @pos, @len)
        print @pos
        print @len
        print @value

        INSERT INTO @MovingWaitListItems 
        SELECT WaitlistItemID, WaitlistItemGUID
        FROM SXAAMWLWaitList    
        Where WaitlistItemID = @value
        select * from @MovingWaitListItems

        set @pos = CHARINDEX(',', @WaitlistItemID, @pos+@len) +1
        print @pos
    END 

Answer 1

For the moment I'll give you the benefit of the doubt and assume that there is some good reason for @WaitlistItemID to be a comma-delimited string. Your while loop condition says that processing should continue as long as a comma exists somewhere in the part of the string you haven't parsed yet. Of course, as Juan Carlos Oropeza points out in his answer, this will always be false for the last item in your list, because you haven't placed a comma after the last value (10011).

However, I would not recommend repeating the insert statement after the loop as in Juan's answer, because there's an easy way to avoid the duplication: just reformulate the loop so its condition instead asks whether there is any part of the string you haven't processed yet. For instance:

declare @cursorPosition int = 1;
declare @commaPosition int;
declare @valueLength int;
declare @value varchar(max);

declare @WaitlistItemID varchar(max)='10008,10009,10010,10011';
declare @MovingWaitListItems table (WaitlistItemID int, WaitlistItemGUID numeric(16,0));

while @cursorPosition < len(@WaitlistItemID)
begin
    set @commaPosition = charindex(',', @WaitlistItemID, @cursorPosition);
    set @valueLength = case 
        when @commaPosition = 0 then len(@WaitlistItemID) - @cursorPosition + 1 
        else @commaPosition - @cursorPosition 
        end;
    set @value = substring(@WaitlistItemID, @cursorPosition, @valueLength);
    set @cursorPosition = @cursorPosition + @valueLength + 1;

    insert @MovingWaitListItems 
    select WaitlistItemID, WaitlistItemGUID
    from SXAAMWLWaitList    
    where WaitlistItemID = @value;
end;

select * from @MovingWaitListItems;

Hopefully this answers the question of why your loop isn't performing as you expect. The question I have for you is why you're resorting to parsing a delimited string in SQL in the first place. If @WaitlistItemID is a table of identifiers instead of a delimited string, then the equivalent logic is:

declare @WaitlistItemID table (ID int);
insert @WaitlistItemID values (10008),(10009),(10010),(10011);
declare @MovingWaitListItems table (WaitlistItemID int, WaitlistItemGUID numeric(16,0));

insert @MovingWaitListItems
select W.WaitlistItemID, W.WaitlistItemGUID
from
    SXAAMWLWaitList W
    inner join @WaitlistItemID ID on W.WaitlistItemID = ID.ID;

select * from @MovingWaitListItems;

Aside from being much easier to understand, this is also going to have vastly better performance than the equivalent iterative solution; set-based logic like this is SQL's forte. while loops in SQL are something you should fall back on only when it's absolutely necessary.

If, in your actual application, @MovingWaitListItems needs to be a parameter to a stored procedure (or similar), you can enable that functionality by creating a custom type. For instance:

create type dbo.IdentifierList as table (ID int);

You can use this type for parameters to stored procedures and functions like so:

create procedure DoSomething(@WaitlistItemID dbo.IdentifierList readonly)
as
select W.WaitlistItemID, W.WaitlistItemGUID
from
    SXAAMWLWaitList W
    inner join @WaitlistItemID ID on W.WaitlistItemID = ID.ID;

And invoke it like this:

declare @WaitlistItemID dbo.IdentifierList;
insert @WaitlistItemID values (10008),(10009),(10010),(10011);
exec DoSomething @WaitlistItemID;

Perhaps you're already aware of all this and there is a good reason for @WaitlistItemID to be a delimited string. For instance, maybe you're maintaining a stored procedure that was written to accept a delimited string as a parameter and you're not able to refactor the caller(s) to use a table-type parameter instead. In that case, I would still recommend a change to your original design: instead of writing the parsing logic directly into this specific application, consider writing yourself a generic function that takes a delimited string and outputs a table, as described in questions like this one, so you can reuse it the next time a similar scenario comes up instead of having to struggle with the parsing logic again.

Answer 2

Problem is related to the number of commas i think. You can add one more item as dummy, you will see 10011 when you add a comma like this:

 declare @WaitlistItemID varchar(max)='10008,10009,10010,10011,'

or

 declare @WaitlistItemID varchar(max)='10008,10009,10010,10011,0'

Answer 3

Imagine if you have a @WaitlistItemID with a single element 10100 you will also skip it because your loop condition will be false.

while CHARINDEX(',', @WaitlistItemID, @pos+1)>0

Same is true for the last element. So you have to add another interaction at the end of the loop

while  ...
 begin
 end

set @len = LEN(@WaitlistItemID)

if @len > 0 
BEGIN    
    set @value = SUBSTRING(@WaitlistItemID, @pos, @len)

    INSERT INTO @MovingWaitListItems 
        SELECT WaitlistItemID, WaitlistItemGUID
        FROM SXAAMWLWaitList    
        Where WaitlistItemID = @value
END

Answer 4

For above, i would like to use of xml node method instead of use of loops with substring(), CHARINDEX() function

declare @WaitlistItemID varchar(max)='10008,10009,10010,10011'
declare @MovingWaitListItems table ( WaitlistItemID int, WaitlistItemGUID numeric(16,0))

;with  cte as
(
       select 
             a.value('.', 'varchar(max)') [WaitlistItemID] from 
       (
          select CAST('<m>'+REPLACE(@WaitlistItemID, ',', '</m><m>')+'</m>' as xml) as waitlist
       ) w cross apply waitlist.nodes ('/m') as split(a)
)

INSERT INTO @MovingWaitListItems
select S.WaitlistItemID, S.WaitlistItemGUID from SXAAMWLWaitList S
JOIN CTE C ON C.[WaitlistItemID] = S.WaitlistItemID

select * from @MovingWaitListItems