MS AdventureWorks dbo.ufnGetProductListPrice-CodePudding

I'm studing the Ms SQL AdventureWorks 2014, to model an internal dbase for our company

I usually work on Postgres, and I'm trying to "understand" Ms SQL stored procedures... :-)) BUT ....

The store Procedure [dbo.ufnGetProductListPrice ] SEEMS STRANGE to me. the SQL code is found here: (https://dataedo.com/samples/html/AdventureWorks/doc/AdventureWorks_2/functions/dbo_ufnGetProductListPrice_116.html)

 CREATE FUNCTION [dbo].[ufnGetProductListPrice](@ProductID [int], @OrderDate [datetime])
RETURNS [money] 
AS 
BEGIN
    DECLARE @ListPrice money;

    SELECT @ListPrice = plph.[ListPrice] 
    FROM [Production].[Product] p 
        INNER JOIN [Production].[ProductListPriceHistory] plph 
        ON p.[ProductID] = plph.[ProductID] 
            AND p.[ProductID] = @ProductID 
            AND @OrderDate BETWEEN plph.[StartDate] AND COALESCE(plph.[EndDate], CONVERT(datetime, '99991231', 112)); -- Make sure we get all the prices!

    RETURN @ListPrice;
END;

The function is making use of the following two tables:

[Production.Product] (https://dataedo.com/samples/html/AdventureWorks/doc/AdventureWorks_2/tables/Production_Product_153.html)
[Production.ProductListPriceHistory] - (https://dataedo.com/samples/html/AdventureWorks/doc/AdventureWorks_2/tables/Production_ProductListPriceHistory_159.html)

In particular my doubts are:

the function is getting the parameter @ProductID. The same @ProductID is used as the primary key of the Production.Product AND as part of the primary key for the table Production.ProductListPriceHistory so is seems of no help making a join on Product.[ProductID] = ProductListPriceHistory.[ProductID] when we can test the ProductID directly on the ProductListPriceHistory.[ProductID]
Why to create such a join ? Seems of no help...
The given @Orderdate datetime received as second parameter, is checked in the JOIN against the following condition

AND @OrderDate BETWEEN plph.[StartDate] AND COALESCE(plph.[EndDate], CONVERT(datetime, '99991231', 112)); -- Make sure we get all the prices!

BUT, if we are calling the store procedure for @Orderdate = 1/1/2022,

considering that [EndDate] could be NULL,
and we could have in ProductListPriceHistory.StartDate two records with our @ProductID, the first with StartDate=1/1/2020 and the second StartDate=1/1/2021,

such "BETWEEN" condition should match both of them, when obviously we would expect the last one .... is it a bug ?

CodePudding user response：

You are right, there are a number of serious flaws in this code, and I would recommend finding better tutorials instead.

I've noted comments on each flaw

CREATE FUNCTION [dbo].[ufnGetProductListPrice](@ProductID [int], @OrderDate [datetime])
RETURNS [money]   -- money is a bad data type due to rounding problems, decimal should be used
                  -- scalar UDFs are very slow, this should be a inline Table Valued Function
AS 
BEGIN
    DECLARE @ListPrice money;

    SELECT @ListPrice = plph.[ListPrice]    -- as you point out, there should be some kind of aggregation
    FROM [Production].[Product] p           -- as you point out: join is unnecessary
        INNER JOIN [Production].[ProductListPriceHistory] plph 
        ON p.[ProductID] = plph.[ProductID] 
            AND p.[ProductID] = @ProductID 
                           -- BETWEEN should never be used on dates, use ">= AND <"
            AND @OrderDate BETWEEN plph.[StartDate] AND COALESCE(plph.[EndDate], CONVERT(datetime, '99991231', 112));   -- ISNULL is better for performance than COALESCE

    RETURN @ListPrice;
END;

A better function would be this

CREATE FUNCTION dbo.ufnGetProductListPrice (
  @ProductID int,
  @OrderDate datetime
)
RETURNS TABLE
AS RETURN

SELECT plph.ListPrice   -- you can return multiple rows
-- alternatively you can use aggregation
-- SELECT ListPrice = AVG(plph.ListPrice)

FROM Production.ProductListPriceHistory plph 
WHERE plph.ProductID = @ProductID 
  AND @OrderDate >= plph.StartDate
  AND @OrderDate < ISNULL(plph.EndDate, CONVERT(datetime, '99991231', 112));

Instead of doing this with a scalar UDF

SELECT p.*, dbo.ufnGetProductListPrice(p.ProductId, GETDATE())
FROM Production.Product p;

You use an APPLY for a TVF

SELECT p.*, plp.*
FROM Production.Product p
CROSS APPLY dbo.ufnGetProductListPrice(p.ProductId, GETDATE()) plp;

CodePudding user response：

@Charlieface Thanks for your reply. One more note on the function ufnGetProductStandardCost

AdventureWorks is storing historical Retailprices and CostPrices, to let you found the price (list, cost) of a certain product at a certain time. This is why I'm interested in this part of the database

It even offers a "dual" function for Cost prices ufnGetProductStandardCost https://dataedo.com/samples/html/AdventureWorks/doc/AdventureWorks_2/functions/dbo_ufnGetProductStandardCost_117.html
with the same errors :-(

I think that the idea of ufnGetProductListPrice(ProductID, OrderDate), is to report a SINGLE price or Null if at that given time the price did not exist.
NOT a table or listrow.

Let's just recall the case I supposed before

search OrderDate 1/1/2022
one record with StartDate=1/1/2020 , EndDate=null
one record with StartDate=1/1/2021 , EndDate=null

One idea for such function would be to:

use a subquery to find the MAX() StardDate<@OrderDate for the given @ProductId
use such StartDate to get the correspondig ListPrice in the external query or give back NULL if we have NO StartDate ListPrice before @OrderDate (e.g @OrderDate=1/1/2019)

What do you think about this query ? Any improvement ?