MS SQL Server - Suggest function code improvement...?

Author

Suggest function code improvement...?

The Gekkster via SQLMonster.com

2005-03-30, 7:03 pm

Moving from Access and trying to get acclimated to working with UDFs and
SPs in SQL Server. Wondered if anyone could share any tips or suggestions
on improving (i.e. 'efficiency') the UDF below. It works, and returns the
correct results (a number, a percentage), but it seems that there should be
a 'better' way to do this. CAST was added when I realized that I was
dividing an integer by an integer (and of course getting a result of 0).

Any ideas, tips, suggestions are welcome.

*******
ALTER FUNCTION dbo.udf_MyFunction
(@ClassTypeRequested
varchar(255))
RETURNS TABLE
AS
RETURN ( SELECT COUNT(UnitInDatabase
) AS CountOfUnitInDatabas
e,
(SELECT CAST(COUNT(UnitInDat
abase) AS NUMERIC)
AS CountOfUnitInDatabas
e
FROM dbo. vw_EDB_Current_Inven
tory
WHERE CLASS_TYPE = @ClassTypeRequested AND
Active = 1 AND CurrentlyInStock = 1 AND DATEDIFF(d, AddedToInventory,
CurrentDate)
> 90 AND DATEDIFF(d,
AddedToInventory, CurrentDate) <= 120) /
(SELECT COUNT(UnitInDatabase
) AS
CountOfUnitInDatabas
e
FROM dbo. vw_EDB_Current_Inven
tory
WHERE CLASS_TYPE = @ClassTypeRequested AND
Active = 1 AND CurrentlyInStock = 1) AS Expr1
FROM dbo. vw_EDB_Current_Inven
tory
WHERE (CLASS_TYPE = @ClassTypeRequested)
AND (Active = 1) AND
(CurrentlyInStock = 1) AND (DATEDIFF(d, AddedToInventory, CurrentDate) > 90)
AND
(DATEDIFF(d, AddedToInventory, CurrentDate) <= 120) )
*******

--
Message posted via http://www.sqlmonster.com

Aaron [SQL Server MVP]

2005-03-30, 7:03 pm

Instead of having us reverse engineer your existing code, can you show us
your sample data and desired output (see my sig)?

--
Please post DDL, sample data and desired results.
See http://www.aspfaq.com/5006 for info.

"The Gekkster via SQLMonster.com" <forum@SQLMonster.com> wrote in message
news:b670d83aa1184b0
7960a69e4f65a87d0@SQ
LMonster.com...
> Moving from Access and trying to get acclimated to working with UDFs and
> SPs in SQL Server. Wondered if anyone could share any tips or suggestions
> on improving (i.e. 'efficiency') the UDF below. It works, and returns the
> correct results (a number, a percentage), but it seems that there should
be
> a 'better' way to do this. CAST was added when I realized that I was
> dividing an integer by an integer (and of course getting a result of 0).
>
> Any ideas, tips, suggestions are welcome.
>
> *******
> ALTER FUNCTION dbo.udf_MyFunction
> (@ClassTypeRequested
varchar(255))
> RETURNS TABLE
> AS
> RETURN ( SELECT COUNT(UnitInDatabase
) AS CountOfUnitInDatabas
e,
> (SELECT CAST(COUNT(UnitInDat
abase) AS
NUMERIC)
> AS CountOfUnitInDatabas
e
> FROM dbo. vw_EDB_Current_Inven
tory
> WHERE CLASS_TYPE = @ClassTypeRequested[
/color]
AND[color=darkred]
> Active = 1 AND CurrentlyInStock = 1 AND DATEDIFF(d, AddedToInventory,
> CurrentDate)
> AddedToInventory, CurrentDate) <= 120) /
> (SELECT COUNT(UnitInDatabase
) AS
> CountOfUnitInDatabas
e
> FROM dbo. vw_EDB_Current_Inven
tory
> WHERE CLASS_TYPE = @ClassTypeRequested[
/color]
AND[color=darkred]
> Active = 1 AND CurrentlyInStock = 1) AS Expr1
> FROM dbo. vw_EDB_Current_Inven
tory
> WHERE (CLASS_TYPE = @ClassTypeRequested)
AND (Active = 1) AND
> (CurrentlyInStock = 1) AND (DATEDIFF(d, AddedToInventory, CurrentDate) >
90)
> AND
> (DATEDIFF(d, AddedToInventory, CurrentDate) <=
120) )
> *******
>
> --
> Message posted via http://www.sqlmonster.com

The Gekkster via SQLMonster.com

2005-03-30, 7:03 pm

Hi Aaron,

The posted function, run against this view:

*******
SELECT dbo.tblUnits.UnitInDatabase, dbo.tblRetailers.Active,
dbo.tblUnits.CurrentlyInStock, dbo.SQUISH_V2.CLASS_TYPE,
dbo.tblUnits.AddedToInventory,
dbo.tblUnits. RemovedFromInventory
, dbo.tblRetailers.Name, GETDATE() AS
CurrentDate,
dbo.tblRetailers.ZoneID, dbo.tblUnits.W_Price,
dbo.tblUnits.R_Price
FROM dbo.tblRetailers INNER JOIN
dbo.tblUnits ON dbo.tblRetailers.RetailerID =
dbo.tblUnits.RetailerID INNER JOIN
dbo.SQUISH_V2 ON dbo.tblUnits.Style_ID =
dbo.SQUISHV2.EDB_STYLE_ID
*******

Produces the following result:

CountOfUnitInDatabas
e 544
Expr2 0.1574551645

The purpose of the function is to (i) quantify the number of units within
the specified CLASS_TYPE that fall within the date (aging) range of '>90
AND <=120'; and (ii) to 'compare' this number against the total of the
designated CLASS_TYPE (expressed as a percentage).

Does this help? Thanks.

--
Message posted via http://www.sqlmonster.com

Aaron [SQL Server MVP]

2005-03-30, 7:03 pm

First, I don't think you should use GETDATE() within the view.
(See http://www.aspfaq.com/2439 for an illustration.)

Second, what are the datatypes in the table? Do any of them allow NULLs
(e.g. UnitInDatabase)?

Third, do you really want the numbers to change throughout the day? By
using GETDATE() instead of a date at midnight, your 90-day window will move
and will potentially cause a query run at 9:00 AM to yield different results
than a query run at 6:00 PM.

Fourth, I recommend dropping the redundant tbl prefixes, and making data
element naming more consistent (would anyone guess that Style_ID and
EDB_STYLE_ID mean the same thing?).

Finally, it is not clear if you want to see the total count in the table or
only those who were added 90-120 days ago. I took a guess. If it does not
yield the result you want, then PLEASE go back to http://www.aspfaq.com/5006
and review how to post your table structure (CREATE TABLE), sample data
(INSERT), and desired results. It's tough to re-create your view when we
have absolutely no knowledge of the tables it references or the data
contained within them.

CREATE VIEW dbo. vw_EDB_Current_Inven
tory
AS
SELECT
u.UnitInDatabase,
r.Active,
u.CurrentlyInStock,
s.CLASS_TYPE,
u.AddedToInventory,
u. RemovedFromInventory
,
r.Name,
r.ZoneID,
u.W_Price,
u.R_Price
FROM
dbo.tblRetailers r
INNER JOIN tblUnits u
ON r.RetailerID = u.RetailerID
INNER JOIN SQUISH_V2 s
ON u.Style_ID = s.EDB_STYLE_ID
GO

DECLARE @dt SMALLDATETIME, @sdt SMALLDATETIME, @edt SMALLDATETIME
SET @dt = DATEADD(DAY, 0, DATEDIFF(DAY, 0, GETDATE()))
SET @sdt = @dt - 120
SET @edt = @dt - 90

SELECT
CountOfUnitInDatabas
e = COUNT(UnitInDatabase
),
Percentage = SUM
(
CASE WHEN AddedToInventory >= @sdt -- 120 days ago
AND AddedToInventory < @edt -- 90 days ago
THEN 1
ELSE 0 END
) / 1.0 * COUNT(UnitInDatabase
)
FROM
dbo. vw_EDB_Current_Inven
tory
WHERE
-- CLASS_TYPE = @classTypeRequested AND
Active = 1
AND CurrentlyInStock = 1

--
Please post DDL, sample data and desired results.
See http://www.aspfaq.com/5006 for info.

"The Gekkster via SQLMonster.com" <forum@SQLMonster.com> wrote in message
news:67434fa817e3453
f87c9660ded23e679@SQ
LMonster.com...
> Hi Aaron,
>
> The posted function, run against this view:
>
> *******
> SELECT dbo.tblUnits.UnitInDatabase, dbo.tblRetailers.Active,
> dbo.tblUnits.CurrentlyInStock, dbo.SQUISH_V2.CLASS_TYPE,
> dbo.tblUnits.AddedToInventory,
> dbo.tblUnits. RemovedFromInventory
, dbo.tblRetailers.Name, GETDATE() AS
> CurrentDate,
> dbo.tblRetailers.ZoneID, dbo.tblUnits.W_Price,
> dbo.tblUnits.R_Price
> FROM dbo.tblRetailers INNER JOIN
> dbo.tblUnits ON dbo.tblRetailers.RetailerID =
> dbo.tblUnits.RetailerID INNER JOIN
> dbo.SQUISH_V2 ON dbo.tblUnits.Style_ID =
> dbo.SQUISHV2.EDB_STYLE_ID
> *******
>
> Produces the following result:
>
> CountOfUnitInDatabas
e 544
> Expr2 0.1574551645
>
> The purpose of the function is to (i) quantify the number of units within
> the specified CLASS_TYPE that fall within the date (aging) range of '>90
> AND <=120'; and (ii) to 'compare' this number against the total of the
> designated CLASS_TYPE (expressed as a percentage).
>
> Does this help? Thanks.
>
> --
> Message posted via http://www.sqlmonster.com

The Gekkster via SQLMonster.com

2005-03-30, 7:03 pm

Thanks, Aaron. This is very helpful as your comments illustrate ideas that
I can apply to similar tasks.

I appreciate the guidelines link on aspfaq.com; I'll be sure to follow this
'process' going forward. After all, it only makes things easier for all
concerned.

--
Message posted via http://www.sqlmonster.com

Sponsored Links