x

UDF "order by" comes out in the opposite order desired - SQL 2005

I have a UDF that I had to do the order by desc to get it to be asc and vice-versa. Has anyone else had this problem? or is this a "feature"? Here is my code:

ALTER FUNCTION [dbo].[fn_Get_Acq_Cost_By_GPI] 
    (@cGPI char(14), @dDateSubmitted datetime)  

RETURNS float

BEGIN 
    declare @ACQ_Cost float
    declare @dDateSub datetime 

    select @dDateSub = isnull(@dDateSubmitted, getdate() )

    select @ACQ_Cost =  receive_unit_price 
    from drugpricing.dbo.VendorPO
       inner join pricing.dbo.mddb_GPI  
    on drugpricing.dbo.VendorPO.ndc = Pricing.dbo.MDDB_GPI.ndc
    where Pricing.dbo.MDDB_GPI.GenericProductIdentifier = @cGPI
         and @dDateSub >= datestamp 
         and receive_unit_price is not null 
    order by drugpricing.dbo.VendorPO.datestamp, receive_Unit_Price desc

    select @Acq_Cost = isNull(@ACQ_Cost, 99999) -- added 6/24/10 msk 

    return @ACQ_Cost
END

From this UDF, I want the record with the newest date and the lowest price for that date. If I do it with: order by drugpricing.dbo.VendorPO.datestamp desc , receive_Unit_Price

I do not get the right price. I get the oldest date and the highest price.

Thanks.

more ▼

asked Sep 09 '10 at 03:05 PM in Default

Russ38546 gravatar image

Russ38546
11 1 1 1

(comments are locked)
10|1200 characters needed characters left

3 answers: sort voted first

You are going to laugh at this one. Here is the source of your "feature":

You have a select statement which returns multiple records, but then you use a variable assignment which is executed as many times as there are records in your select. By the time it bails out, it gets the value which belongs to the last record of your select which is of course the opposite from what you want because you really need it to be equal to the value from the first record not the last. All you need to do is limit your select such that it returns only one record. This way you don't have to flip your order by clause to the opposite and also you will have your function return desired result much faster. To make the long story short, make your select look like this:

select @ACQ_Cost = t.receive_unit_price 
from 
(
    select 
       top 1 receive_unit_price
    from drugpricing.dbo.VendorPO inner join pricing.dbo.mddb_GPI  
       on drugpricing.dbo.VendorPO.ndc = Pricing.dbo.MDDB_GPI.ndc
    where Pricing.dbo.MDDB_GPI.GenericProductIdentifier = @cGPI
        and @dDateSub >= datestamp 
        and receive_unit_price is not null 
    order by 
       drugpricing.dbo.VendorPO.datestamp desc, 
       receive_Unit_Price
) t;

select @Acq_Cost = isNull(@ACQ_Cost, 99999);
Oleg
more ▼

answered Sep 09 '10 at 03:25 PM

Oleg gravatar image

Oleg
15.9k 2 4 24

THANK YOU!!!! that is what I'll do! I was ignorant about how to set my return variable like you did!! doh!
Sep 09 '10 at 03:35 PM Russ38546
(comments are locked)
10|1200 characters needed characters left

I may be overlooking something here, but it looks like you only have to use DESC after the date column rather than after the receive_Unit_Price clumn:

order by drugpricing.dbo.VendorPO.datestamp DESC, receive_Unit_Price
(Since ASC is the default, there no need to add it after receive_Unit_Price unless it is clearer to the reader.)
more ▼

answered Sep 09 '10 at 03:30 PM

Mark gravatar image

Mark
2.6k 21 25 27

@Oleg, I didn't see your answer before posting mine. Yours is the best answer if only one record is needed (and that -is- implied).
Sep 09 '10 at 03:33 PM Mark

Mark, yes, I think you overlooked something. I don't put ASC anywhere in my order by's.

I'm giong to use Oleg's suggestion. thank you for responding.
Sep 09 '10 at 03:37 PM Russ38546

@Mark Here is the behaviour revealed (best with results to text):

set nocount on;

declare @t table(number int); declare @result int;

insert into @t select 1 union select 2 union select 3;

print 'this is looking good'; select number from @t order by number desc;

print 'this ain''t looking right'; select @result = number from @t order by number desc;

select @result wrong_result;

print 'this one is correct'; select @result = number from ( select top 1 number from @t order by number desc ) t;

select @result correct_result;

set nocount off;

-- results

this is looking good

number

3 2 1

this ain't looking right

wrong_result

1

this one is correct

correct_result

3
Sep 09 '10 at 03:51 PM Oleg
I didn't notice the "select @ACQ_Cost = " at the beginning of the query.
In your example, this format would be more concise, but not like the OP's syntax: select @result = (select top 1 number from @t order by number desc)
Sep 10 '10 at 07:32 AM Mark
(comments are locked)
10|1200 characters needed characters left

hi again! funny thing, when I changed it to be how Oleg suggested, it took 55:57 minutes to update 320,335 records. when I changed it back, it took 3:33 mins!!! wow... needless to say I went back to my original code and will just accept that I have to do the opposite order by that I want...

weird!!!

Mark, what do you mean by this: In your example, this format would be more concise, but not like the OP's syntax: select @result = (select top 1 number from @t order by number desc)

mine was more concise or OP's was?

thanks!
more ▼

answered Sep 10 '10 at 10:03 AM

Russ38546 gravatar image

Russ38546
11 1 1 1

P.S. - I have indexes on the GPI field, the price field, and the date field -- and it was still SLOWWWWWWWW
Sep 10 '10 at 10:11 AM Russ38546
@Russ38546 This must be something else. You did not mention anything about update statement, I just thought that logically thinking, a single assignment versus multiple assignments of the same variable when only one is actually needed should perform a little better. If you can elaborate on your update, maybe some other solution can be found, because it is difficult to believe that the update should take 3 minutes just to affect 320,335 records unless there are some triggers involved. Usually, update of this size should take seconds not minutes.
Sep 10 '10 at 10:18 AM Oleg

there are no triggers... here's the update code:

update RFP..table set Acq_Cost_by_GPI = dbo.[fn_Get_Acq_Cost_By_GPI](genericproductidentifier, Datestamp)

thanks!
Sep 10 '10 at 10:45 AM Russ38546

it cut off my parameters!!

update table set Acq_Cost_by_GPI = dbo.fn_Get_Acq_Cost_By_GPI(genericproductidentifier, datestamp)
Sep 10 '10 at 10:47 AM Russ38546
@Russ38546, actually the snyntax in your example in the original post was lengthier than my example. If you only want to determine the value of one variable, it's shorter to just say: SET @Variable = (SELECT FROM ...)
Sep 10 '10 at 11:04 AM Mark
(comments are locked)
10|1200 characters needed characters left
Your answer
toggle preview:

Up to 2 attachments (including images) can be used with a maximum of 524.3 kB each and 1.0 MB total.

New code box

There's a new way to format code on the site - the red speech bubble logo will automatically format T-SQL for you. The original code box is still there for XML, etc. More details here.

Follow this question

By Email:

Once you sign in you will be able to subscribe for any updates here

By RSS:

Answers

Answers and Comments

SQL Server Central

Need long-form SQL discussion? SQLserverCentral.com is the place.

Topics:

x1936
x8

asked: Sep 09 '10 at 03:05 PM

Seen: 1281 times

Last Updated: Sep 10 '10 at 01:30 AM