September 14, 2010 at 6:13 pm
I've recently run across a query that uses two nested cursors to cycle through a bunch of inserts. I've been attempting to convert it into a set based solution using CTE's but I find I'm having a problem since the actual inserts rely on the cursor queries for it's information. Can someone point me in the right direction?
-- Declare local vars
DECLARE @username varchar(20)
,@itemcode varchar(25)
,@levelval varchar(2)
,@groupcode char(2)
,@adminusername char(25)
,@now datetime
-- Initialize vars
SET @now = GETDATE()
SET @adminusername = 'ADMIN'
SET @groupcode = 'AD'
-- Create cursor from the Groups table
DECLARE curGroups INSENSITIVE SCROLL CURSOR FOR
SELECT RTRIM(g.Itemcode), RTRIM(g.Levelval)
FROM dbo.groups g (NOLOCK)
WHERE g.Groupcode = @groupcode
-- Open Groups cursor
OPEN curGroups
-- Create cursor from the Privs table
DECLARE curPrivs CURSOR FAST_FORWARD FOR
SELECT DISTINCT RTRIM(p.username) AS 'username'
FROM dbo.privs p (NOLOCK)
JOIN dbo.users u (NOLOCK) ON u.usertype = 'S' AND u.username = p.username
WHERE p.groupcode = @groupcode AND u.inactivedate IS NULL
-- Open Privs cursor
OPEN curPrivs
-- Loop through Privs cursor
FETCH NEXT FROM curPrivs INTO @username
WHILE @@FETCH_STATUS = 0
BEGIN
-- Loop through Groups cursor and perform inserts
FETCH FIRST FROM curGroups INTO @itemcode, @levelval
WHILE @@FETCH_STATUS = 0
BEGIN
IF NOT EXISTS(SELECT username FROM dbo.privs (NOLOCK) WHERE username = @username AND groupcode = @groupcode AND itemcode = @itemcode)
BEGIN
BEGIN TRANSACTION privsinsert
-- Execute insert
INSERT INTO
dbo.Privs
(
Username,
Groupcode,
Itemcode,
Levelval
)
VALUES
(
@username,
@groupcode,
@itemcode,
@levelval
)
-- Copy records inserted into tbl_privs_history
INSERT INTO
dbo.tbl_privs_history
(
col_username,
col_groupcode,
col_itemcode,
col_levelval,
col_adminusername,
col_eventdate
)
VALUES
(
@username,
@groupcode,
@itemcode,
@levelval,
@adminusername,
@now
)
COMMIT TRANSACTION privsinsert
END
FETCH NEXT FROM curGroups INTO @itemcode, @levelval
END
FETCH NEXT FROM curPrivs INTO @username
END
-- Clean up
-- Close cursor
CLOSE curGroups
DEALLOCATE curGroups
CLOSE curPrivs
DEALLOCATE curPrivs
September 14, 2010 at 7:10 pm
:ermm: Yikes. That is one twisted piece of code you've got there.
Let me see if I get this straight. You set the group code, get the itemcode/levelval, and then a list of all users once that have any privs at all (where usertype = S... ) that are also for the same group code and aren't inactive.
From this...
For each user, and then for each pair of itemcodes and levelvals, you check for existance of the combination in privs and if nonexisting drop them to the privs table and then to the history table.
Well, for starters, not sure you even need to go the CTE route here. Just need a little fancy footwork, try this out:
-- Declare local vars
DECLARE @username varchar(20)
,@itemcode varchar(25)
,@levelval varchar(2)
,@groupcode char(2)
,@adminusername char(25)
-- Initialize vars
SET @adminusername = 'ADMIN'
SET @groupcode = 'AD'
INSERT INTO
dbo.Privs
(
Username,
Groupcode,
Itemcode,
Levelval
)
SELECT
drvU.Username,
@groupcode,
drvG.Itemcode,
drvG.levelval
FROM
(SELECT DISTINCT
RTRIM( u.username)
FROM
users AS u
JOIN
privs AS p
ONu.username = p.username
WHERE
u.usertype = 'S'
AND u.InactiveDate IS NULL
and p.groupcode = @groupcode
) AS drvU
CROSS JOIN
(SELECT
RTRIM(g.Itemcode),
RTRIM(g.Levelval)
FROM
dbo.groups g (NOLOCK)
WHERE
g.Groupcode = @groupcode
) AS drvG
LEFT JOIN
privs AS p
ONp.username = drvU.UserName
AND p.groupcode = @groupcode
and p.itemcode = drvU.ItemCode
WHERE
p.username IS NULL
CREATE TRIGGER trg_priv_audit FOR INSERT ON privs
AS
BEGIN
INSERT INTO
dbo.tbl_privs_history
(
col_username,
col_groupcode,
col_itemcode,
col_levelval,
col_adminusername,
col_eventdate
)
SELECT
username,
groupcode,
itemcode,
levelval,
GETDATE()
FROM
inserted
END
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 15, 2010 at 10:13 am
I've tried running this code and these are the errors I'm getting:
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 1 of 'drvU'.
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 1 of 'drvG'.
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 2 of 'drvG'.
Msg 207, Level 16, State 1, Line 60
Invalid column name 'UserName'.
Msg 207, Level 16, State 1, Line 62
Invalid column name 'ItemCode'.
Msg 207, Level 16, State 1, Line 32
Invalid column name 'Username'.
Msg 207, Level 16, State 1, Line 34
Invalid column name 'Itemcode'.
Msg 207, Level 16, State 1, Line 35
Invalid column name 'levelval'.
I've been exploring the CROSS JOIN angle but I've had little success.
September 15, 2010 at 10:43 am
brian_winzeler (9/15/2010)
I've tried running this code and these are the errors I'm getting:
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 1 of 'drvU'.
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 1 of 'drvG'.
Msg 8155, Level 16, State 2, Line 23
No column was specified for column 2 of 'drvG'.
Msg 207, Level 16, State 1, Line 60
Invalid column name 'UserName'.
Msg 207, Level 16, State 1, Line 62
Invalid column name 'ItemCode'.
Msg 207, Level 16, State 1, Line 32
Invalid column name 'Username'.
Msg 207, Level 16, State 1, Line 34
Invalid column name 'Itemcode'.
Msg 207, Level 16, State 1, Line 35
Invalid column name 'levelval'.
I've been exploring the CROSS JOIN angle but I've had little success.
Well, it's kinda hard to test it when you didn't give us anything to test against. (Please see the first two links in my signature for how to do this.)
Modifying Craig's code to resolve those errors:
-- Declare local vars
DECLARE @username varchar(20)
,@itemcode varchar(25)
,@levelval varchar(2)
,@groupcode char(2)
,@adminusername char(25)
-- Initialize vars
SET @adminusername = 'ADMIN'
SET @groupcode = 'AD'
INSERT INTO
dbo.Privs
(
Username,
Groupcode,
Itemcode,
Levelval
)
SELECT
drvU.Username,
@groupcode,
drvG.Itemcode,
drvG.levelval
FROM
(SELECT DISTINCT
Username = RTRIM( u.username)
FROM
users AS u
JOIN
privs AS p
ON u.username = p.username
WHERE
u.usertype = 'S'
AND u.InactiveDate IS NULL
and p.groupcode = @groupcode
) AS drvU
CROSS JOIN
(SELECT
Itemcode = RTRIM(g.Itemcode),
levelval = RTRIM(g.Levelval)
FROM
dbo.groups g (NOLOCK)
WHERE
g.Groupcode = @groupcode
) AS drvG
LEFT JOIN
privs AS p
ON p.username = drvU.UserName
AND p.groupcode = @groupcode
and p.itemcode = drvU.ItemCode
WHERE
p.username IS NULL
CREATE TRIGGER trg_priv_audit FOR INSERT ON privs
AS
BEGIN
INSERT INTO
dbo.tbl_privs_history
(
col_username,
col_groupcode,
col_itemcode,
col_levelval,
col_adminusername,
col_eventdate
)
SELECT
username,
groupcode,
itemcode,
levelval,
GETDATE()
FROM
inserted
END
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
September 15, 2010 at 10:48 am
Yeah, I didn't build DDL out for the table so I didn't doublecheck my code for functional running, but I forgot to drop AS [name] in at the select levels in the subqueries, which screwed up the outer queries. Try this, and adjust for minor issues. (or feed the lazy and send me some DDLs and test data inclusions...)
[EDIT:] Heheh, Wayne, fastest code gun in the west. 🙂 [/EDIT]
-- Declare local vars
DECLARE @groupcode char(2)
,@adminusername char(25)
-- Initialize vars
SET @adminusername = 'ADMIN'
SET @groupcode = 'AD'
INSERT INTO
dbo.Privs
(
Username,
Groupcode,
Itemcode,
Levelval
)
SELECT
drvU.Username,
@groupcode,
drvG.Itemcode,
drvG.levelval
FROM
(SELECT DISTINCT
RTRIM( u.username) AS username
FROM
users AS u
JOIN
privs AS p
ON u.username = p.username
WHERE
u.usertype = 'S'
AND u.InactiveDate IS NULL
and p.groupcode = @groupcode
) AS drvU
CROSS JOIN
(SELECT
RTRIM(g.Itemcode) as itemcode,
RTRIM(g.Levelval) as levelval
FROM
dbo.groups g (NOLOCK)
WHERE
g.Groupcode = @groupcode
) AS drvG
LEFT JOIN
privs AS p
ON p.username = drvU.UserName
AND p.groupcode = @groupcode
and p.itemcode = drvU.ItemCode
WHERE
p.username IS NULL
CREATE TRIGGER trg_priv_audit FOR INSERT ON privs
AS
BEGIN
INSERT INTO
dbo.tbl_privs_history
(
col_username,
col_groupcode,
col_itemcode,
col_levelval,
col_adminusername,
col_eventdate
)
SELECT
username,
groupcode,
itemcode,
levelval,
GETDATE()
FROM
inserted
END
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 15, 2010 at 10:52 am
Ahh that fixed it. My whole point was to be pointed in the right direction (which this most certainly did!). I will definitely keep that in mind for future problems.
Thanks very much to both you guys for helping!
September 15, 2010 at 11:03 am
brian_winzeler (9/15/2010)
Ahh that fixed it. My whole point was to be pointed in the right direction (which this most certainly did!). I will definitely keep that in mind for future problems.Thanks very much to both you guys for helping!
Whelp, if it helps, the problem is that you can end up being too close to the code after a certain point. I find that when you're dealing with issues like this, it's easier to extract the logic, write it down somewhere, and abandon the old code. Then you start thinking about the solution to the real problem, the logic, then trying to clean up a specific code piece.
So, to help you get in the right direction, here were the pieces of the logic and my initial thoughts to completion:
- Need a list of every user that has any item in another table according to parameters.
-- Subquery to find these users
- For these users, they need all items from somewhere else...
-- Cross Join to create master list of user/itemcodes
- ... that they don't have yet...
-- LEFT JOIN to find missing connections, then use only NULL results from the left join.
- ... and insert them into table x.
-- One bulk insert statement off the results of the above.
- And then I want to audit this action.
-- One trigger, coming up.
Hopefully that helps. 🙂
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 15, 2010 at 11:05 am
You really should provide TEST DATA. Despite that, I think that this is what you want.
INSERT INTO dbo.Privs(Username, GroupCode, ItemCode, LevelVal)
SELECT DISTINCT p.Username, p.GroupCode, g.ItemCode, g.LevelVal
FROM dbo.Privs AS p
INNER JOIN dbo.Users AS u
ON p.Username = u.Username
INNER JOIN dbo.Groups AS g
ON p.GroupCode = g.GroupCode
WHERE u.UserType = 'S' -- This probably does not belong in the join clause
AND u.InactiveDate IS Null
AND p.GroupCode = @GroupCode -- You may not need this line if you want to do this for all GroupCodes
EXCEPT
SELECT p.Username, p.GroupCode, p.ItemCode, p.LevelVal
FROM dbo.Privs AS p
The key here is the EXCEPT clause which replaces the NOT EXISTS clause from the cursor.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
September 15, 2010 at 11:14 am
Craig Farrell (9/15/2010)
- For these users, they need all items from somewhere else...-- Cross Join to create master list of user/itemcodes
It's actually not a CROSS JOIN, because the value of the variable @groupcode for privs is set from the value of the groupcode in the groups table. This translates to an INNER JOIN on those values.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
September 15, 2010 at 11:22 am
drew.allen (9/15/2010)
Craig Farrell (9/15/2010)
- For these users, they need all items from somewhere else...-- Cross Join to create master list of user/itemcodes
It's actually not a CROSS JOIN, because the value of the variable @groupcode for privs is set from the value of the groupcode in the groups table. This translates to an INNER JOIN on those values.
Drew
You have a point, and had I looked into optimizing this, instead of solving the immediate problem, I might have caught that. I also have a date with the 'EXCEPT' command now, I got to learn something new too. 🙂
To the OP: None of this code is optimized completely. If this is more then a once a month maintenance cleanup, and you have design control in the database, you might want to look into doing the following:
Create a many to many reference for user/group so that you don't have to go distinct selecting or over-sized result sets to get user/group lists. Then, alter your procedure that does the insert for the itemcode in the first place to assign it to a group of active users.
(Edit: I need to learn to use my adverbs properly...)
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 15, 2010 at 11:45 am
Thanks to both of you for the solutions. I'll be testing both of them to see what performs better (more for my own education). I'm not actually sure how often this runs, I was just told it was timing out for a particular user type when called from the front end web page.
I don't yet have any creative control in this area, but I'll definitely bring up your suggestion to the people who do. It depends on what other processes are tied in to all of that.
Thank you both again for the the time and effort put in to helping me solve this problem.
September 15, 2010 at 1:08 pm
Craig Farrell (9/15/2010)
[EDIT:] Heheh, Wayne, fastest code gun in the west. 🙂 [/EDIT]
Maybe, but sometimes it's draw,shoot,aim instead of aim,shoot,draw... heh, wait-a-minute... :crazy:
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
September 15, 2010 at 1:13 pm
WayneS (9/15/2010)
Craig Farrell (9/15/2010)
[EDIT:] Heheh, Wayne, fastest code gun in the west. 🙂 [/EDIT]Maybe, but sometimes it's draw,shoot,aim instead of aim,shoot,draw... heh, wait-a-minute... :crazy:
And thus, we find out why two of your nicknames are "Swiss cheese feet" and "Wild Shot Wayne"? 😉
*ducks and runs the bad joke tomatoes*
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 15, 2010 at 1:41 pm
Craig Farrell (9/15/2010)
WayneS (9/15/2010)
Craig Farrell (9/15/2010)
[EDIT:] Heheh, Wayne, fastest code gun in the west. 🙂 [/EDIT]Maybe, but sometimes it's draw,shoot,aim instead of aim,shoot,draw... heh, wait-a-minute... :crazy:
And thus, we find out why two of your nicknames are "Swiss cheese feet" and "Wild Shot Wayne"? 😉
*ducks and runs the bad joke tomatoes*
LOL :-D:crying:
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
September 15, 2010 at 2:06 pm
brian_winzeler (9/15/2010)
Ahh that fixed it. My whole point was to be pointed in the right direction (which this most certainly did!). I will definitely keep that in mind for future problems.Thanks very much to both you guys for helping!
Understood. Do understand that it's sometimes REAL hard to point someone in the right direction without writing code especially when it's not known if the original poster will understand a suggestion something like "do a cross join between the two table to replace the nested cursor in a set based fashion". 😉
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 15 posts - 1 through 14 (of 14 total)
You must be logged in to reply to this topic. Login to reply