CODEWITHSUNDEEP
sqlsql-serveradventureworks
Giuliano69
Giuliano69

Reputation: 11

MS AdventureWorks dbo.ufnGetProductListPrice

I'm studying the SQL Server AdventureWorks 2014, to model an internal database for our company.

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

The stored 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:

In particular my questions are:

  1. The function is getting the parameter @ProductID. The same @ProductID is used as the primary key of the Production.Product table, 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 use such a join? Seems of no help...

  2. 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 stored 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 with StartDate = 1/1/2021 such a BETWEEN condition should match both of them, when obviously we would expect the last one .... - is it a bug?

Upvotes: 0

Views: 257

Answers (2)

Charlieface
Charlieface

Reputation: 71544

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 TOP (1)
  plph.ListPrice
FROM Production.ProductListPriceHistory plph 
WHERE plph.ProductID = @ProductID 
  AND @OrderDate >= plph.StartDate
  AND @OrderDate < ISNULL(plph.EndDate, CONVERT(datetime, '99991231', 112))
ORDER BY
  plph.StartDate DESC;

Assuming it's possible for there to be multiple active prices (I don't think it's possible in AdventureWorks), then you also need TOP (1) and ORDER BY plph.StartDate DESC. If this is not possible then you can leave that out.

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
OUTER APPLY dbo.ufnGetProductListPrice(p.ProductId, GETDATE()) plp;

An OUTER APPLY works like a LEFT JOIN, which means you get NULL if there are no rows.

Upvotes: 3

Giuliano69
Giuliano69

Reputation: 11

@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 ?

CREATE FUNCTION dbo.ufnGetProductListPrice (
  @ProductID int,
  @OrderDate datetime
)
AS 
BEGIN

DECLARE @ListPrice money;

SELECT @ListPrice= plph.ListPrice
FROM Production.ProductListPriceHistory plph 
WHERE plph.ProductID = @ProductID 
  AND plhp.StartDate = (
        SELECT MAX(StardDate)
        FROM Production.ProductListPriceHistory plph1
        WHERE plph1.ProductID = @ProductID
            AND plhp1.StartDate <=  @OrderDate)         
RETURN @ListPrice;
END;

Upvotes: 0

Related Questions