Take the 2-minute tour ×
Database Administrators Stack Exchange is a question and answer site for database professionals who wish to improve their database skills and learn from others in the community. It's 100% free, no registration required.

I wrote a Table Valued Function in Microsoft SQL Server 2008 to take a comma delimited column in a database to spit out separate rows for each value.

Ex: "one,two,three,four" would return a new table with only one column containing the following values:

one
two
three
four

Does this code look error prone to you guys? When I test it with

SELECT * FROM utvf_Split('one,two,three,four',',') 

it just runs forever and never returns anything. This is getting really disheartening especially since there are no built in split functions on MSSQL server (WHY WHY WHY?!) and all the similar functions I've found on the web are absolute trash or simply irrelevant to what I'm trying to do.

Here is the function:

USE *myDBname*
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER FUNCTION [dbo].[utvf_SPlit] (@String VARCHAR(MAX), @delimiter CHAR)

RETURNS @SplitValues TABLE
(
    Asset_ID VARCHAR(MAX) NOT NULL
)

AS
BEGIN
            DECLARE @FoundIndex INT
            DECLARE @ReturnValue VARCHAR(MAX)

            SET @FoundIndex = CHARINDEX(@delimiter, @String)

            WHILE (@FoundIndex <> 0)
            BEGIN
                  DECLARE @NextFoundIndex INT
                  SET @NextFoundIndex = CHARINDEX(@delimiter, @String, @FoundIndex+1)
                  SET @ReturnValue = SUBSTRING(@String, @FoundIndex,@NextFoundIndex-@FoundIndex)
                  SET @FoundIndex = CHARINDEX(@delimiter, @String)
                  INSERT @SplitValues (Asset_ID) VALUES (@ReturnValue)
            END

            RETURN
END
share|improve this question
    
Have you considered using a table-valued parameter instead? –  A-K Jul 18 '12 at 16:29
    
@AlexKuznetsov I offered that as an option, but apparently it's too complex. –  Aaron Bertrand Jul 18 '12 at 17:55

4 Answers 4

up vote 1 down vote accepted

Re-worked it slightly...

DECLARE @FoundIndex INT
DECLARE @ReturnValue VARCHAR(MAX)

SET @FoundIndex = CHARINDEX(@delimiter, @String)

WHILE (@FoundIndex <> 0)
BEGIN
      SET @ReturnValue = SUBSTRING(@String, 0, @FoundIndex)
      INSERT @SplitValues (Asset_ID) VALUES (@ReturnValue)
      SET @String = SUBSTRING(@String, @FoundIndex + 1, len(@String) - @FoundIndex)
      SET @FoundIndex = CHARINDEX(@delimiter, @String)
END

INSERT @SplitValues (Asset_ID) VALUES (@String)

RETURN
share|improve this answer
    
thank you! It's actually spitting out correct output right now vs. getting into some sort of infinite loop like it previously was. Right now if I pass in 'one,two,three,four' your code only spits back one two three. So I'll work on it from here, I should be able to get it working 100% properly. Aaron - not a big fan of that. IMHO that's too much code for too little of a project. This should be short and concise code for a short and concise task. Thanks though. And to boot, Aaron has +3 while Derek doesn't have any upvotes (I can't vote, this isn't a full account)...WTF –  OvetS Jul 18 '12 at 17:16
    
I specifically need to be using a TVF here. Also Derek your code actually does work exactly how I wanted it to, I just forgot to type in that last insert statement...silly me, I'm tired. –  OvetS Jul 18 '12 at 17:22
    
@AaronBertrand My argument is that your code is confusing to me, who has mild SQL knowledge. And my boss has even less SQL knowledge than I do. Whereas the other solution is simple and easy to follow along with, all while producing desired output and leaving few holes (someone pointed out that there is a potential hole in your solution). –  OvetS Jul 18 '12 at 17:52

I wouldn't do this with a loop; there are much better alternatives. By far the best, when you have to split, is CLR, and Adam Machanic's approach is the fastest I've tested.

Next best approach IMHO, if you can't implement CLR, is a numbers table:

SET NOCOUNT ON;

DECLARE @UpperLimit INT = 1000000;

WITH n AS
(
    SELECT
        x = ROW_NUMBER() OVER (ORDER BY s1.[object_id])
    FROM       sys.all_objects AS s1
    CROSS JOIN sys.all_objects AS s2
    CROSS JOIN sys.all_objects AS s3
)
SELECT Number = x
  INTO dbo.Numbers
  FROM n
  WHERE x BETWEEN 1 AND @UpperLimit
  OPTION (MAXDOP 1); -- protecting from Paul White's observation

GO
CREATE UNIQUE CLUSTERED INDEX n ON dbo.Numbers(Number) 
    --WITH (DATA_COMPRESSION = PAGE);
GO

... which allows this function:

CREATE FUNCTION dbo.SplitStrings_Numbers
(
   @List       NVARCHAR(MAX),
   @Delimiter  NVARCHAR(255)
)
RETURNS TABLE
WITH SCHEMABINDING
AS
   RETURN
   (
       SELECT Item = SUBSTRING(@List, Number, 
         CHARINDEX(@Delimiter, @List + @Delimiter, Number) - Number)
       FROM dbo.Numbers
       WHERE Number <= CONVERT(INT, LEN(@List))
         AND SUBSTRING(@Delimiter + @List, Number, 1) = @Delimiter
   );
GO

I believe all of these will perform better than the function you have, when you get it working, especially since they are inline instead of multi-statement. I haven't investigated why yours isn't working, because I don't think it's worth it to get that function working.

But that all said...

Since you are using SQL Server 2008, is there a reason you need to split in the first place? I would rather use a TVP for this:

CREATE TYPE dbo.strings AS TABLE
(
  string NVARCHAR(4000)
);

Now you can accept this as a parameter to your stored procedures, and use the contents just like you would use a TVF:

CREATE PROCEDURE dbo.foo
  @strings dbo.strings READONLY
AS
BEGIN
  SET NOCOUNT ON;

  SELECT Asset_ID = string FROM @strings;
  -- SELECT Asset_ID FROM dbo.utvf_split(@other_param, ',');
END

And you can pass a TVP directly from C# etc. as a DataTable. This will almost certainly outperform any of the solutions above, especially if you are building a comma-separated string in your app specifically so that your stored procedure can call a TVP to split it apart again. For a lot more info on TVPs see Erland Sommarskog's great article.

share|improve this answer
1  
Just a note that STVFs like that can create a nasty to debug issue when combined with more complex queries: rusanu.com/2011/08/10/… –  Remus Rusanu Jul 18 '12 at 16:53
    
@Remus yep, it's certainly not my top option, as I tried to make clear, but it's still better than the loop the OP has. I would definitely prefer TVPs or CLR over any native TVF. –  Aaron Bertrand Jul 18 '12 at 16:55
    
+1 There are some equally fast alternatives with recursive CDEs, but those fail where there are more than MAXRECURSION+1 entries in the list. –  Jon of All Trades Jul 18 '12 at 19:29
    
One small gotcha: if your numbers table starts with zero (or a negative number), use Number BETWEEN 1 AND LEN(@List) in the WHERE clause rather than Number <= .... –  Jon of All Trades Aug 1 '12 at 18:54
3  
@KennyEvitt Aaron's code is perfectly sound, though a MAXDOP 1 hint would prevent running into this bug, which I suspect you did. –  Paul White Feb 15 '13 at 5:51
CREATE FUNCTION [dbo].[fnSplit]
(

    @sInputList VARCHAR(8000),         -- List of delimited items

    @sDelimiter VARCHAR(8000) = ','    -- delimiter that separates items

)
RETURNS @List TABLE (colData VARCHAR(8000))

BEGIN

DECLARE @sItem VARCHAR(8000)

    WHILE CHARINDEX(@sDelimiter,@sInputList,0) <> 0

    BEGIN

        SELECT @sItem=RTRIM(LTRIM(SUBSTRING(@sInputList,1,CHARINDEX
(@sDelimiter,@sInputList,0)-1))),

        @sInputList=RTRIM(LTRIM(SUBSTRING(@sInputList,CHARINDEX(@sDelimiter,@sInputList,0)
+LEN(@sDelimiter),LEN(@sInputList))))

        IF LEN(@sItem) > 0
            INSERT INTO @List SELECT @sItem
        END

        IF LEN(@sInputList) > 0
            INSERT INTO @List SELECT @sInputList -- Put the last item in
        RETURN
    END

--TEST

--Example 1: select * from fnSplit('1,22,333,444,,5555,666', ',')

--Example 2: select * from fnSplit('1##22#333##444','##')  --note second colData has embedded #

--Example 3: select * from fnSplit('1 22 333 444  5555 666', ' ')

enter image description here

share|improve this answer

Don't run Aaron's (original) 'numbers table' code above; try this to create a table dbo.Numbers and (quickly) add one million rows:

CREATE TABLE dbo.Numbers (
    Number int
);
GO

INSERT dbo.Numbers ( Number ) VALUES ( 1 );

WHILE @@ROWCOUNT > 0
    INSERT dbo.Numbers ( Number )
    SELECT Number = x.MaxNumber + n.Number
    FROM    dbo.Numbers n
            CROSS JOIN (    SELECT MaxNumber = MAX(Number)
                            FROM dbo.Numbers
                        ) x
    WHERE n.Number <= 1000000 - x.MaxNumber;
share|improve this answer
4  
Since populating a Numbers table is something you only do once, I don't think it matters much which method you use (though see my comment on Aaron's question in response to yours). The method shown here is no worse than many others from that point of view, but the self-reference does mean each iteration of your code requires an Eager Table Spool for Halloween Protection, which other methods do not. –  Paul White Feb 15 '13 at 5:54
    
I would like this method if there wasn't this horrible WHERE clause. And since it's one-time only thing, there is not much to gain - even if this is faster. –  ypercube Feb 15 '13 at 7:56
1  
@Aaron, when Kenny ran it, it (perhaps) failed due to a bug: all bets on 'quicker' are presumably off when you get into bug territory? –  Jack Douglas Feb 15 '13 at 8:40
    
@ypercube – what's so horrible about the WHERE clause? –  Kenny Evitt Feb 15 '13 at 22:58
2  
Let me explain: I like recursive and self-referential solutions - for their beauty alone, they are often less efficient than other types of solutions. But this one is spoiled by the useless (here and in my opinion) WHERE clause. If you want to insert a trillion (10^12) rows, it's certainly not efficient to check each and every one of them. Just insert 2^40, doing 40 repeats of the single Insert statement. Then delete the extra few millions load, if you have to. –  ypercube Feb 15 '13 at 23:16

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.