views:

37

answers:

3

I got this code, I would like to optimize. I basically can add new columns to "Disp" table later on, and I don't want to come back modify this function. I cannot use dynamic SQL. Right? Is there anything else that would work in my case?

This is the function:

ALTER FUNCTION [GetDate] 
(@hdrnumber INT, @DateColName VARCHAR(50))
RETURNS DATETIME
AS
BEGIN
    DECLARE @dt DATETIME
        SELECT  @dt = CASE
          WHEN @DateColName = 'ord_bookdate' THEN [ord_bookdate]
          WHEN @DateColName = 'ord_startdate' THEN [ord_startdate]
          WHEN @DateColName = 'ord_completiondate' THEN [ord_completiondate]
          WHEN @DateColName = 'pack_date_from' THEN [pack_date_from]
          WHEN @DateColName = 'pack_date_to' THEN [pack_date_to]
         END
        FROM    [Disp]
        WHERE   [hdrnumber] = @hdrnumber

    RETURN @dt
END

(removed some of the code, because it's a long one, but hopefully what I left in here will make sense to you guys)

how do i use this function? well it basically looks like this:

 insert into tablename (...)
 select somedate, [GetDate](somedate, somecolumn)
 from sometable
 where 1 = 1
A: 

I think you have a bigger design issue here. How in the world are you using this? If you need to specify the column as a string (??!) then dynamic SQL will be >>SO<< much faster in many key situations. Something like this:

declare @sql NVARCHAR(MAX);
set @sql = 'SELECT ' + @DateColName + ' FROM disp WHERE hdrnumber = @hdrnumber';
EXEC (@sql, @hdrnumber);

(just watch out that someone cant SQL inject into @DateColName).

But a bigger issue is the design smell that's all over this code. The fact that this function exists worries me about how you could possibly be using it. Take a bigger look, and maybe post another, more detailed question about your design in general and I think you could have even more helpful answers.

Dave Markle
A: 

You either need to use dynamic sql which you can't do within a user-defined function (so you'd need to remove it out of the function), or like you are doing with a CASE statement (which isn't fully generic).

I think I'd personally go with the CASE approach to start with and consider dynamic sql if proves to give better performance (maybe with a larger number of different possible fields). You would have some maintenance work to do to keep the CASE up to date, but I can't imagine more fields would be added that often?

AdaTheDev
i edited the main post to show you how do i use thisand YES there are actually 20+ cases, i just left there 5 cases in this example, to make the code look smaller
Swoosh
+1  A: 

Certainly agree with comments provided in previous two answers.

Anyways, you could write following function to get Column names of a given table and then compare the column names to @DatecolumName to return the value from it..

Create

function [dbo].[ftTableSchema](@TableName varchar(100))  returns table as
return 
--Declare @tableName varchar(30); select @TABLENAME='excelInBom'

SELECT ColumnName=Column_Name
            ,DataType= case data_type
                            When  'DECIMAL' then 'DECIMAL('+convert(varchar,Numeric_precision)+','+Convert(varchar,Numeric_scale)+')'
                            When  'NUMERIC' then 'DECIMAL('+convert(varchar,Numeric_precision)+','+Convert(varchar,Numeric_scale)+')'
                            when 'VARCHAR' then 'VARCHAR('+Convert(varchar,Character_maximum_length)+')'
                            WHEN 'CHAR' THEN  'CHAR('+Convert(varchar,Character_maximum_length)+')'
                            ELSE data_type      
                          end

,ColumnOrder=Ordinal_Position,* FROM INFORMATION_SCHEMA.COLUMNS
WHERE TABLE_NAME=@tableName
TonyP