views:

56

answers:

4

I made the following function in SQL Server 2008 earlier this week that takes two parameters and uses them to select a column of "detail" records and returns them as a single varchar list of comma separated values. Now that I get to thinking about it, I would like to take this table and application-specific function and make it more generic.

I am not well-versed in defining SQL functions, as this is my first. How can I change this function to accept a single "column" worth of data, so that I can use it in a more generic way?

Instead of calling:

SELECT ejc_concatFormDetails(formuid, categoryName)

I would like to make it work like:

SELECT concatColumnValues(SELECT someColumn FROM SomeTable)

Here is my function definition:

FUNCTION [DNet].[ejc_concatFormDetails](@formuid AS int, @category as VARCHAR(75))
RETURNS VARCHAR(1000) AS
BEGIN
 DECLARE @returnData VARCHAR(1000)
 DECLARE @currentData VARCHAR(75)
 DECLARE dataCursor CURSOR FAST_FORWARD FOR
  SELECT data FROM DNet.ejc_FormDetails WHERE formuid = @formuid AND category = @category

 SET @returnData = ''

 OPEN dataCursor

 FETCH NEXT FROM dataCursor INTO @currentData
 WHILE (@@FETCH_STATUS = 0)
 BEGIN
  SET @returnData = @returnData + ', ' + @currentData
  FETCH NEXT FROM dataCursor INTO @currentData
 END

 CLOSE dataCursor
 DEALLOCATE dataCursor

 RETURN SUBSTRING(@returnData,3,1000)
END

As you can see, I am selecting the column data within my function and then looping over the results with a cursor to build my comma separated varchar.

How can I alter this to accept a single parameter that is a result set and then access that result set with a cursor?

+2  A: 

You can use a table variable:

CREATE FUNCTION MyFunction(
    @Data AS TABLE (
        Column1 int,
        Column2 nvarchar(50),
        Column3 datetime
    )
)
RETURNS NVARCHAR(MAX)
AS BEGIN
    /* here you can do what you want */
END
TcKs
+2  A: 

You can use Table Valued Parameters as of SQL Server 2008, which would allow you to pass a TABLE variable in as a parameter. The limitations and examples for this are all in that linked article.

However, I'd also point out that using a cursor could well be painful for performance. You don't need to use a cursor, as you can do it all in 1 SELECT statement:

SELECT @MyCSVString = COALESCE(@MyCSVString + ', ', '') + data 
FROM DNet.ejc_FormDetails 
WHERE formuid = @formuid AND category = @category

No need for a cursor

AdaTheDev
+1 Just had that ready to paste in and you beat me to it!
Martin Smith
+1 for cursor free!!!! just remember, you need to loop sometimes, but you never need a cursor to loop: http://stackoverflow.com/questions/2622230/how-can-i-iterate-over-a-recordset-within-a-stored-procedure/2622279#2622279http://stackoverflow.com/questions/935336/sql-server-cursor-reference-syntax-etc/935367#935367 By just replacing the cursor and still looping using these techniques, you will usually see a performance gain.
KM
Thanks for the cursor-free solution Ada. I had hoped for a way of doing this without cursors and did not know I could use variables in this way. However, as far as performance goes (and I see many groans here about performance!) my cursor-based function runs almost 5 times quicker than your suggested solution. I don't know why that would be, but I can pull 6,000 "header" records with 4 of my function calls per row (processing 150,000 detail records) in about 2 seconds, but SELECT @Var = COALESCE(@Var,'') + data takes longer than 10.I will continue to test both methods, thanks for the link!
brandon k
A: 

Your question is a bit unclear. In your first SQL statement it looks like you're trying to pass columns to the function, but there is no WHERE clause. In the second SQL statement you're passing a collection of rows (results from a SELECT). Can you supply some sample data and expected outcome?

Without fully understanding your goal, you could look into changing the parameter to be a table variable. Fill a table variable local to the calling code and pass that into the fuction. You could do that as a stored procedure though and wouldn't need a function.

Tom H.
+3  A: 

Others have answered your main question - but let me point out another problem with your function - the terrible use of a CURSOR!

You can easily rewrite this function to use no cursor, no WHILE loop - nothing like that. It'll be tons faster, and a lot easier, too - much less code:

FUNCTION DNet.ejc_concatFormDetails
            (@formuid AS int, @category as VARCHAR(75))
RETURNS VARCHAR(1000) 
AS
    RETURN 
      SUBSTRING(
        (SELECT ', ' + data
         FROM DNet.ejc_FormDetails 
         WHERE formuid = @formuid AND category = @category
         FOR XML PATH('')
        ), 3, 1000)

The trick is to use the FOR XML PATH('') - this returns a concatenated list of your data columns and your fixed ', ' delimiters. Add a SUBSTRING() on that and you're done! As easy as that..... no dogged-slow CURSOR, no messie concatenation and all that gooey code - just one statement and that's all there is.

marc_s