Improve running time of custom function

  • I am using SQL Server 2012 to improve the running time on a user defined function named "udf_GetPackagesHirearchyofNetwork_NEW". This function takes a little bit long time to run. It is called in .NET application using Microsoft Entity Framework version 5. I do not know if it can be improved in some way. Please advise. Thank you

    CREATE Function [dbo].[udf_GetPackagesHirearchyofNetwork_NEW]

    (

    @networkItemid int

    )

    RETURNS @PackageTable TABLE

    (

    Id int identity (1,1),

    ContentItemTargetMapID int,

    ContentItemMapID int,

    ContentItemId int,

    MappedToID int,

    NetworkItemID int,

    ContentPackageId int,

    ContentPackageTypeID int,

    NetworkItemName NVARCHAR(64),

    ContentItemName NVARCHAR(256),

    ContentitemTypeID int,

    StartDate DateTime

    )

    As

    Begin

    ---------------------------------- main task

    Insert Into @PackageTable

    Select

    CITM.ContentItemTargetMapID,

    CIM.ContentItemMapID,

    CITM.ContentItemId,

    CIM.MappedToID,

    ni.NetworkItemID,

    CP.ContentPackageId,

    CP.ContentPackageTypeID,

    ni.NetworkItemName,

    CI.ContentItemName,

    CI.ContentitemTypeID,

    CITM.StartDate

    from ContentItemTargetMap CITM

    inner join ContentItem CI on CITM.ContentItemId = CI.ContentItemId

    inner join ContentItemMap CIM on CIM.ContentItemId = CI.ContentItemId

    inner join NetworkItem ni on CITM.NetworkItemID = ni.NetworkItemID

    left outer join ContentPackage CP on CI.ContentItemId = CP.ContentItemId

    where CITM.NetworkItemID in (

    select CITM.NetworkItemID from ContentItemTargetMap CITM

    inner join ContentItem CI on CITM.ContentItemId = CI.ContentItemId

    inner join dbo.udf_GetNetworkItemChildren_LIGHT_NEW(@networkItemid) fun on CITM.NetworkItemID = fun.NetworkItemID

    where ContentItemTypeID = 2

    )

    Insert Into @PackageTable

    select

    null,

    CIM.ContentItemMapID,

    CIM.ContentItemId,

    CIM.MappedToID,

    null,

    CP.ContentPackageId,

    CP.ContentPackageTypeID,

    null,

    CI.ContentItemName,

    CI.ContentitemTypeID,

    null

    from ContentItemMap CIM

    inner join ContentItem CI on CIM.ContentItemId = CI.ContentItemId

    left outer join ContentPackage CP on CI.ContentItemId = CP.ContentItemId

    where CIM.MappedToID in (Select ContentItemID from @PackageTable where ContentitemTypeID = 2)

    Insert Into @PackageTable

    select

    null,

    CIM.ContentItemMapID,

    CIM.ContentItemId,

    CIM.MappedToID,

    null,

    CP.ContentPackageId,

    CP.ContentPackageTypeID,

    null,

    CI.ContentItemName,

    CI.ContentitemTypeID,

    null

    from ContentItemMap CIM

    inner join ContentItem CI on CIM.ContentItemId = CI.ContentItemId

    left outer join ContentPackage CP on CI.ContentItemId = CP.ContentItemId

    where CIM.MappedToID in (Select ContentItemID from @PackageTable where ContentitemTypeID = 3)

    RETURN;

    End

    GO

    ----------------------------------------------------------

    CREATE Function [dbo].[udf_GetNetworkItemChildren_LIGHT_NEW](@id int)

    RETURNS @NetworkChildern Table

    (

    NetworkItemID int NOT NULL PRIMARY KEY

    )

    As

    BEGIN

    Insert into @NetworkChildern(NetworkItemID)

    SELECT NetworkItemID

    FROM NetworkItem

    Where ParentID = @id

    Return

    END

    GO

    ---------------------------------------------------------------

    CREATE TABLE [dbo].[NetworkItem](

    [NetworkItemID] [int] IDENTITY(1,1) NOT NULL,

    [NetworkItemName] [nvarchar](64) NULL,

    [ParentID] [int] NULL,

    [NetworkItemTypeID] [int] NOT NULL,

    [NetworkSiteID] [uniqueidentifier] NULL,

    [tempDMSNetworkItemID] [int] NULL DEFAULT ((0)),

    [IsActive] [bit] NOT NULL DEFAULT ((1)),

    [IrisId] [bigint] NOT NULL CONSTRAINT [DF_NetworkItem_IrisId] DEFAULT ((0)),

    PRIMARY KEY CLUSTERED

    (

    [NetworkItemID] ASC

    )WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]

    ) ON [PRIMARY]

    GO

    ALTER TABLE [dbo].[NetworkItem] WITH NOCHECK ADD CONSTRAINT [FK_NetworkItem_Parent] FOREIGN KEY([ParentID])

    REFERENCES [dbo].[NetworkItem] ([NetworkItemID])

    GO

    ALTER TABLE [dbo].[NetworkItem] CHECK CONSTRAINT [FK_NetworkItem_Parent]

    GO

  • Before I make any suggestions, I need to know two things, please.

    1. How many rows are in the NetworkItem table?

    2. How often does data change in the NetworkItem table?

    And, thanks for posting the DDL for the NetworkItem table. That pretty much answered the other questions I was going to have.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • One thing that I note here is that you are using a Multi-Statement Table-Valued Functions, and they consist of just inserting data into the (output) table variable to be returned. Change this into an Inline Table-Valued Function (just take those select statements, and do a UNION ALL between them) instead. You won't believe the performance boost that you'll get from doing just this.

    I recently blogged about this (comparing all three T-SQL functions) here[/url].

    Wayne
    Microsoft Certified Master: SQL Server 2008
    Author - SQL Server T-SQL Recipes


    If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!
    Links:
    For better assistance in answering your questions
    Performance Problems
    Common date/time routines
    Understanding and Using APPLY Part 1 & Part 2

  • WayneS (7/29/2015)


    One thing that I note here is that you are using a Multi-Statement Table-Valued Functions, and they consist of just inserting data into the (output) table variable to be returned. Change this into an Inline Table-Valued Function (just take those select statements, and do a UNION ALL between them) instead. You won't believe the performance boost that you'll get from doing just this.

    I recently blogged about this (comparing all three T-SQL functions) here[/url].

    Absolutely agreed and great article from one of the true masters of the art but, mTVF or iTVF or not, there's a huge over-complexity in all that was posted on the original post that I don't believe is necessary especially since it appears to be a true and clean Adjacency List problem.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • Jeff Moden (7/29/2015)


    Before I make any suggestions, I need to know two things, please.

    1. How many rows are in the NetworkItem table?

    2. How often does data change in the NetworkItem table?

    And, thanks for posting the DDL for the NetworkItem table. That pretty much answered the other questions I was going to have.

    First Thank you, Jeff.

    1. How many rows are in the NetworkItem table?

    >> About 76,731 rows

    2. How often does data change in the NetworkItem table?

    >> It is changed biweekly or monthly. But the user defined function in question is run at 3:00 AM daily.

  • WayneS (7/29/2015)


    One thing that I note here is that you are using a Multi-Statement Table-Valued Functions, and they consist of just inserting data into the (output) table variable to be returned. Change this into an Inline Table-Valued Function (just take those select statements, and do a UNION ALL between them) instead. You won't believe the performance boost that you'll get from doing just this.

    I recently blogged about this (comparing all three T-SQL functions) here[/url].

    Thank you for your reply.

    As it is the result from the first SELECT statement, the table variable "@PackageTable" is re-used in the second and third Select statements. How can I use UNION ALL statements between the first, second and third SELECT statements?

  • johnsql-193053 (7/29/2015)


    Jeff Moden (7/29/2015)


    Before I make any suggestions, I need to know two things, please.

    1. How many rows are in the NetworkItem table?

    2. How often does data change in the NetworkItem table?

    And, thanks for posting the DDL for the NetworkItem table. That pretty much answered the other questions I was going to have.

    First Thank you, Jeff.

    1. How many rows are in the NetworkItem table?

    >> About 76,731 rows

    2. How often does data change in the NetworkItem table?

    >> It is changed biweekly or monthly. But the user defined function in question is run at 3:00 AM daily.

    In that case, I'd scrap everything except the table and start over. The code you currently have to work with is going to be slow, resource intensive, and difficult to maintain. And, it's only going to get worse as time marches on especially since it mixes the resolution of the hierarchy with the accumulation and presentation of data at the same time. I'll be back after work tonight to explain. And, no... it's not that difficult and will open many more effective opportunities behind the scenes.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • I got off work just about an hour ago (9PM) and I've been looking at this code and I have to admit a bit of confusion. I might just be tired or I may have spoken a bit too soon.

    1. Is this function going to be called for every NetworkItemID in the Network table on every 3AM run?

    2. And, correct me if I'm wrong... the first query appears to only get one level deep from the provided @NetworkItemID and only for those rows that have a ContentItemTypeID = 2. Is that correct?

    3. From there, the next two nearly identical queries seem to shift gears by getting data by ContentItemId and, unless that's a rather unique value, the rows returned will simply explode because of possible 1 to many joins.

    My original thought was to do something like what I have in one of the other following articles to resolve the hierarchy but your hierarchy is only one level deep for each node and has a rather explosive set of rows for each node.

    http://www.sqlservercentral.com/articles/Hierarchy/94040/

    http://www.sqlservercentral.com/articles/T-SQL/94570/

    Shifting gears a bit, Wayne is correct and you don't really need a table valued function. The first query in the function could be written as a CTE, the second as a "cascading" CTE (reads from the first as if it were a table valued function) and the final query could simply be another cascading CTE off that. Since it can all be written in a single query, it's like a view, which is one of the qualifiers for turning this into a high performance Inline Table Valued Function. Of course, the nested function is already in the proper form for a conversion to an iTVF.

    Like I said, apologies for not truly understanding what this is doing. My brain is just a little fried.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • Jeff Moden (7/29/2015)


    I got off work just about an hour ago (9PM) and I've been looking at this code and I have to admit a bit of confusion. I might just be tired or I may have spoken a bit too soon.

    1. Is this function going to be called for every NetworkItemID in the Network table on every 3AM run?

    2. And, correct me if I'm wrong... the first query appears to only get one level deep from the provided @NetworkItemID and only for those rows that have a ContentItemTypeID = 2. Is that correct?

    3. From there, the next two nearly identical queries seem to shift gears by getting data by ContentItemId and, unless that's a rather unique value, the rows returned will simply explode because of possible 1 to many joins.

    My original thought was to do something like what I have in one of the other following articles to resolve the hierarchy but your hierarchy is only one level deep for each node and has a rather explosive set of rows for each node.

    http://www.sqlservercentral.com/articles/Hierarchy/94040/

    http://www.sqlservercentral.com/articles/T-SQL/94570/

    Shifting gears a bit, Wayne is correct and you don't really need a table valued function. The first query in the function could be written as a CTE, the second as a "cascading" CTE (reads from the first as if it were a table valued function) and the final query could simply be another cascading CTE off that. Since it can all be written in a single query, it's like a view, which is one of the qualifiers for turning this into a high performance Inline Table Valued Function. Of course, the nested function is already in the proper form for a conversion to an iTVF.

    Like I said, apologies for not truly understanding what this is doing. My brain is just a little fried.

    Thank you for your reply.

    1. Is this function going to be called for every NetworkItemID in the Network table on every 3AM run?

    >> Yes, there are about 3000 different NetworkItemIDs call this function at 3AM currently.

    2. And, correct me if I'm wrong... the first query appears to only get one level deep from the provided @NetworkItemID and only for those rows that have a ContentItemTypeID = 2. Is that correct?

    >> Yes but for ContentItem table filter as full query as ContentItem.ContentItemTypeID = 2. Column ContentItemTypeID does not belong to table NetworkItem.

    I will try nested CTEs.

    My question: Is CTE is worse than DTS method?

    P/S. Around 2007 or 2008, when I started using SQL Server v. 2005 from v. 2000, I read your couple of your articles at sqlteam.com site about "New features in SQL Server 2005", and I learned a lot from those.

Viewing 9 posts - 1 through 8 (of 8 total)

You must be logged in to reply to this topic. Login to reply