Cursor runs for a long time

  • I have created a cursor to execute a SP which will insert data from Employees table into users table.

    i told to pass values through a sp which is already created..

    i created a cursor which is as follows:

    declare @PortalID int, --0

    @Username nvarchar(100),

    @FirstName nvarchar(50),

    @LastName nvarchar(50),

    @AffiliateId int, --0

    @IsSuperUser bit, --0

    @Email nvarchar(256),

    @DisplayName nvarchar(100), -- LastName,[SPACE]FirstName[SPACE]MiddleName

    @UpdatePassword bit, --0

    @Authorised bit, --1

    @CreatedByUserID int --1

    declare db_cursor cursor for select fFirstName+fLastName as Username,

    fFirstName,fLastName,fEmail ,fFirstName+fLastName as Displayname from WCDentalSQL_COR..Employees

    open db_cursor

    fetch next from db_cursor into @username,@firstname,@lastname,@email,@displayname

    while @@FETCH_STATUS=0

    begin

    EXEC dbo.AddUser 0,@username,@firstname,@lastname,0,0,@Email,@DisplayName,0,1,1

    fetch next from db_cursor into @username,@firstname,@lastname,@email,@displayname

    END

    close db_cursor

    deallocate db_cursor

    But it runs for more than 2 min and show error as:

    An error occurred while executing batch. Error message is: Exception of type 'System.OutOfMemoryException' was thrown.

    please suggest solution for this as soon a possible

    _______________________________________________________________
    To get quick answer follow this link:
    http://www.sqlservercentral.com/articles/Best+Practices/61537/

  • You need to get rid of that cursor. We could help you but you need to provide the code for the stored procedure.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • sp_helptext 'adduser'

    CREATE PROCEDURE dbo.AddUser

    @PortalID int,

    @Username nvarchar(100),

    @FirstName nvarchar(50),

    @LastName nvarchar(50),

    @AffiliateId int,

    @IsSuperUser bit,

    @Email nvarchar(256),

    @DisplayName nvarchar(100),

    @UpdatePassword bit,

    @Authorised bit,

    @CreatedByUserID int

    AS

    DECLARE @userid int

    SELECT @userid = UserID

    FROM dbo.Users

    WHERE Username = @Username

    IF @userid is null

    BEGIN

    INSERT INTO dbo.Users (

    Username,

    FirstName,

    LastName,

    AffiliateId,

    IsSuperUser,

    Email,

    DisplayName,

    UpdatePassword,

    CreatedByUserID,

    CreatedOnDate,

    LastModifiedByUserID,

    LastModifiedOnDate

    )

    VALUES (

    @Username,

    @FirstName,

    @LastName,

    @AffiliateId,

    @IsSuperUser,

    @Email,

    @DisplayName,

    @UpdatePassword,

    @CreatedByUserID,

    getdate(),

    @CreatedByUserID,

    getdate()

    )

    SELECT @userid = SCOPE_IDENTITY()

    END

    IF @IsSuperUser = 0

    BEGIN

    IF not exists ( SELECT 1 FROM dbo.UserPortals WHERE UserID = @userid AND PortalID = @PortalID )

    BEGIN

    INSERT INTO dbo.UserPortals (

    UserID,

    PortalID,

    Authorised,

    CreatedDate

    )

    VALUES (

    @userid,

    @PortalID,

    @Authorised,

    getdate()

    )

    END

    END

    SELECT @userid

    _______________________________________________________________
    To get quick answer follow this link:
    http://www.sqlservercentral.com/articles/Best+Practices/61537/

  • Hi Luis I added code of sthe stored procedure..

    plzz find a solution for me

    _______________________________________________________________
    To get quick answer follow this link:
    http://www.sqlservercentral.com/articles/Best+Practices/61537/

  • For what I understood, you're trying to have Users and PortalUsers for all the employees.

    This should do the trick. However, you might feel more confident using the OUTPUT clause.

    Be aware that I have nothing to test (no sample data or tables) so that is up to you.

    INSERT INTO dbo.Users (

    Username,

    FirstName,

    LastName,

    AffiliateId,

    IsSuperUser,

    Email,

    DisplayName,

    UpdatePassword,

    CreatedByUserID,

    CreatedOnDate,

    LastModifiedByUserID,

    LastModifiedOnDate

    )

    SELECT fFirstName+fLastName,

    fFirstName,

    fLastName,

    0,

    0,

    fEmail,

    fFirstName+fLastName,

    0,

    GETDATE(),

    1,

    GETDATE()

    FROM WCDentalSQL_COR..Employees e

    WHERE NOT EXISTS( SELECT 1 FROM dbo.Users WHERE u.Username = e.fFirstName+e.fLastName)

    INSERT INTO dbo.UserPortals (

    UserID,

    PortalID,

    Authorised,

    CreatedDate

    )

    SELECT UserID,

    0,

    1,

    GETDATE()

    FROM Users u

    WHERE IsSuperUser = 0

    AND not exists ( SELECT 1 FROM dbo.UserPortals p WHERE p.UserID = u.UserID AND p.PortalID = 0

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • I don't think that Luis example will run (WHERE IsSuperUser AND...) , and he couldn't test it as you didn't supply any table DDL's.

    Yes, He is right it's possible to use OUTPUT clause to collect inserted User's ID's:

    CREATE TABLE #InsertedUsers ( UserID INT ) -- Does Users table have UserID as INT?

    INSERT dbo.Users

    ( Username

    ,FirstName

    ,LastName

    ,AffiliateId

    ,IsSuperUser

    ,Email

    ,DisplayName,

    ,UpdatePassword

    ,CreatedByUserID

    ,CreatedOnDate

    ,LastModifiedByUserID

    ,LastModifiedOnDate

    )

    OUTPUT INSERTED.UserId INTO #InsertedUsers -- that will keep all insrted ID's for futher use (is UserId column name right?)

    SELECT fFirstName + fLastName AS Username

    ,fFirstName AS FirstName

    ,fLastName AS LastName

    ,0 AS AffiliateId

    ,0 AS IsSuperUser

    ,fEmail AS Email

    ,fFirstName + fLastName AS DisplayName,

    ,0 AS UpdatePassword

    ,1 AS CreatedByUserID

    ,getdate() AS CreatedOnDate

    ,1 AS LastModifiedByUserID

    ,getdate() AS LastModifiedOnDate

    FROM WCDentalSQL_COR.dbo.Employees AS E

    WHERE NOT EXISTS (SELECT 1 FROM dbo.Users U WHERE U.Username = fFirstName + fLastName)

    -- looks like all new users you've inserted have @IsSuperUser = 0

    -- as these users are new, you don't need to check if they are in the UserPortals table

    INSERT INTO dbo.UserPortals

    (

    UserID

    ,PortalID

    ,Authorised

    ,CreatedDate

    )

    SELECT UserID AS UserId

    ,0 AS PortalID

    ,1 AS Authorised

    ,getdate() AS CreatedDate

    FROM #InsertedUsers

    -- I'm not sure that if that is really right,

    -- but to fully replicate the your cursor and stored proc logic

    -- we need to insert into UserPortals all users which already existed in Users

    INSERT INTO dbo.UserPortals

    (

    UserID

    ,PortalID

    ,Authorised

    ,CreatedDate

    )

    SELECT U.UserId AS UserId

    ,0 AS PortalID

    ,1 AS Authorised

    ,getdate() AS CreatedDate

    FROM WCDentalSQL_COR.dbo.Employees AS E

    JOIN dbo.Users AS U

    ON U.Username = fFirstName + fLastName

    WHERE NOT EXISTS (SELECT 1 FROM dbo.UserPortals UP WHERE UP.UserId = U.UserId AND UP.PortalId = 0)

    -- Actually you can use just the last INSERT, as it will catch all inserted records as well as pre-existing

    -- So you don't really need to collect Inserted UserId's at all.

    Why your proc checks where IsUperUser = 0? Your cursor always supplies it as 0!

    Names are not the best candidates for finding and checking if user exists, what about if your company have two John Smith's?

    I would probably separate the logic for inserting into UserPortal records for users which are already exist in User table into some other process. Then you can use version with OUTPUT clause and just insert into UserPortal from #InsertedUsers...

    _____________________________________________
    "The only true wisdom is in knowing you know nothing"
    "O skol'ko nam otkrytiy chudnyh prevnosit microsofta duh!":-D
    (So many miracle inventions provided by MS to us...)

    How to post your question to get the best and quick help[/url]

  • how to fetch one by one row using WHILE instead of cursor?

    _______________________________________________________________
    To get quick answer follow this link:
    http://www.sqlservercentral.com/articles/Best+Practices/61537/

  • BUt I told to not to change the stored procedure..

    just need to pass the column value into variables of SP to insert rows one by one from employees

    _______________________________________________________________
    To get quick answer follow this link:
    http://www.sqlservercentral.com/articles/Best+Practices/61537/

  • kapil_kk (10/18/2012)


    how to fetch one by one row using WHILE instead of cursor?

    That will not make much difference to a cursor.

    BUt I told to not to change the stored procedure..

    just need to pass the column value into variables of SP to insert rows one by one from employees

    Do not change stored procedure... Just don't use it!

    Place the given code into the same place as where you have your cursor.

    _____________________________________________
    "The only true wisdom is in knowing you know nothing"
    "O skol'ko nam otkrytiy chudnyh prevnosit microsofta duh!":-D
    (So many miracle inventions provided by MS to us...)

    How to post your question to get the best and quick help[/url]

  • Eugene Elutin


    I don't think that Luis example will run (WHERE IsSuperUser AND...)

    I forgot to complete the condition, it's corrected now.

    kapil_kk


    how to fetch one by one row using WHILE instead of cursor?

    kapil_kk


    BUt I told to not to change the stored procedure..

    just need to pass the column value into variables of SP to insert rows one by one from employees

    Using loops such as cursors or whiles will slow down the performance. SQL Server works with sets of data and that is what we did.

    You don't have to change the SP as this instructions will do different things. You can create a new SP if you want that will update your Users tables whenever you need.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • Luis Cazares (10/18/2012)


    Eugene Elutin


    I don't think that Luis example will run (WHERE IsSuperUser AND...)

    I forgot to complete the condition, it's corrected now.

    kapil_kk


    how to fetch one by one row using WHILE instead of cursor?

    kapil_kk


    BUt I told to not to change the stored procedure..

    just need to pass the column value into variables of SP to insert rows one by one from employees

    Using loops such as cursors or whiles will slow down the performance. SQL Server works with sets of data and that is what we did.

    You don't have to change the SP as this instructions will do different things. You can create a new SP if you want that will update your Users tables whenever you need.

    To be clear... cursors are NOT the problem per se, its what you do in them that can be bad. If you can develop a way to update the data as a set, all at once, and perform the same functionality once, instead of an iterative 1500 times, you will be better off.

    There are times and places for cursors, knowing when to use them is the key.

Viewing 11 posts - 1 through 10 (of 10 total)

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