August 28, 2015 at 6:45 am
I have a temptable with a list of user IDs that I want to drop so I created a script to do a cursor and run through my drop functions. The drops work by themselves and the ver check works with them but when I wrap them in the cursor all i get is an output for each user in the results window in ssms. Any ideas why it's not setting the variable and instead outputting to results? Thanks in advance
DECLARE @ver nvarchar(128);
DECLARE @UserName nvarchar(50);
DECLARE @UserD nvarchar(80);
DECLARE @LoginD nvarchar(80);
-- Initialize the variable.
SET @ver = CAST(serverproperty('ProductVersion') AS nvarchar)
SET @ver = SUBSTRING(@ver, 1, CHARINDEX('.', @ver) - 1)
-- Open Cursor
DECLARE usrcursor CURSOR FOR
SELECT Name from TempTable;
OPEN usrcursor;
FETCH NEXT FROM usrcursor INTO @UserName;
WHILE @@FETCH_STATUS = 0
BEGIN
SET @UserD = 'DROP USER ' + @UserName
SET @LoginD = 'DROP LOGIN ' + @UserName
-- For SQL 2005 and up: change username
IF ( @ver > '10' ) IF EXISTS (SELECT * FROM sys.database_principals WHERE name = N'@UserName')
exec (@UserD)
If Exists (select loginname from master.dbo.syslogins
where name = @UserName)
exec (@LoginD)
GOTO exit_now
-- For SQL 2000: change username
IF ( @ver < '10' ) IF EXISTS (SELECT * FROM dbo.sysusers WHERE name = N'@UserName')
EXEC dbo.sp_revokedbaccess N'@UserName';
exit_now:
FETCH NEXT FROM usrcursor;
END;
CLOSE usrcursor;
DEALLOCATE usrcursor;
GO
August 28, 2015 at 8:13 am
The first thing I notice is that you are excluding SQL 2008. You have tests for > 10, and < 10, but not = 10.
On a separate note, replace your GOTO with an ELSE. They accomplish the same thing in this particular case, but the ELSE is much better programming style.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
August 28, 2015 at 8:24 am
I included some comments on your code.
DECLARE @ver nvarchar(128);
DECLARE @UserName nvarchar(50);
DECLARE @UserD nvarchar(80);
DECLARE @LoginD nvarchar(80);
-- Initialize the variable.
SET @ver = CAST(serverproperty('ProductVersion') AS nvarchar)
SET @ver = SUBSTRING(@ver, 1, CHARINDEX('.', @ver) - 1)
-- Open Cursor
DECLARE usrcursor CURSOR FOR
SELECT Name
from TempTable;
OPEN usrcursor;
FETCH NEXT FROM usrcursor INTO @UserName;
WHILE @@FETCH_STATUS = 0
BEGIN
SET @UserD = 'DROP USER ' + @UserName
SET @LoginD = 'DROP LOGIN ' + @UserName
-- For SQL 2005 and up: change username
IF ( @ver > '10' )
IF EXISTS (SELECT * FROM sys.database_principals WHERE name = N'@UserName') --Unless you have a user called @UserName, this will always return false. Remove the quotes.
exec (@UserD)
If Exists (select loginname from master.dbo.syslogins where name = @UserName) --This isn't tied to the "IF ( @ver > '10' )"
exec (@LoginD)
GOTO exit_now --Neither is this
-- For SQL 2000: change username
IF ( @ver < '10' ) --This will never execute
IF EXISTS (SELECT * FROM dbo.sysusers WHERE name = N'@UserName')
EXEC dbo.sp_revokedbaccess N'@UserName';
exit_now:
FETCH NEXT FROM usrcursor;
END;
CLOSE usrcursor;
DEALLOCATE usrcursor;
GO
August 28, 2015 at 8:39 am
I figured since all versions are actually xx.xx.xxxx they would be greater or less than 10. In this case the GOTO is not an else as it is just a way to skip the next statement since a version greater than 10 was found and the appropriate code was ran. else would be appropriate for then nest IF statement if needed to be use it that way as it is the alternative to the first IF. Certainly feel free to let me know if my interpretation of the logic is incorrect. As is that part of the code functions as desired when not included in the Cursor....
August 28, 2015 at 9:09 am
alemmon2 (8/28/2015)
I figured since all versions are actually xx.xx.xxxx they would be greater or less than 10. In this case the GOTO is not an else as it is just a way to skip the next statement since a version greater than 10 was found and the appropriate code was ran. else would be appropriate for then nest IF statement if needed to be use it that way as it is the alternative to the first IF. Certainly feel free to let me know if my interpretation of the logic is incorrect. As is that part of the code functions as desired when not included in the Cursor....
You're right about the string. It's sometimes hard to remember that you're dealing with strings instead of numbers. I was thinking of it as an INT. That being said, what's the string returned by SQL 2005? If it doesn't have leading zeros, it's going to trigger the wrong code.
As for the GOTO: the two pieces of code are logically connected in that the conditions are mutually exclusive. The version cannot be simultaneously greater than and less than the same value. IF...ELSE was specifically designed to conditionally run different code based on mutually exclusive conditions. That's exactly what you're trying to do.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
August 28, 2015 at 9:57 am
Actually, serverproperty('ProductVersion') returns the version in the form of 'major.minor.build.revision'.
Here are some changes to your code which might follow a better logic.
- I changed the @ver to an integer and changed the logic to assign it.
- I changed the version used to compare as 10 is 2008 and you need 9 to include 2005 and up.
- Defined the blocks of code for the IF which uses an ELSE instead of the GOTO.
- I changed the logins table.
- I removed the string comparisons to leave comparisons against variables.
- I removed the unnecessary variables.
DECLARE @ver int;
DECLARE @UserName nvarchar(128);
-- Initialize the variable.
SET @ver = PARSENAME( CAST( serverproperty('ProductVersion') AS nvarchar(128)), 4);
-- Open Cursor
DECLARE usrcursor CURSOR FOR
SELECT Name
from TempTable;
OPEN usrcursor;
FETCH NEXT FROM usrcursor INTO @UserName;
WHILE @@FETCH_STATUS = 0
BEGIN
-- For SQL 2005 and up:
IF ( @ver >= 9 )
BEGIN
IF EXISTS (SELECT 1 FROM sys.database_principals WHERE name = @UserName)
exec ('DROP USER ' + @UserName);
If Exists (select 1 from master.sys.sql_logins where name = @UserName)
exec ('DROP LOGIN ' + @UserName);
END
ELSE
BEGIN --Not needed in here, but included for clarity.
-- For SQL 2000: change username
IF EXISTS (SELECT 1 FROM dbo.sysusers WHERE name = @UserName)
EXEC dbo.sp_revokedbaccess @UserName;
END
FETCH NEXT FROM usrcursor INTO @UserName;
END;
CLOSE usrcursor;
DEALLOCATE usrcursor;
August 28, 2015 at 10:27 am
DECLARE @sqlVersion smallint;
DECLARE @UserName nvarchar(100);
SELECT @sqlVersion = LEFT(ProductVersion, CHARINDEX('.', ProductVersion) - 1)
FROM (
SELECT CAST(SERVERPROPERTY('ProductVersion') AS varchar(30)) AS ProductVersion
) AS system_value
--get all user/login names that need removed from current db/instance
DECLARE usrcursor CURSOR LOCAL FAST_FORWARD FOR
SELECT Name
FROM TempTable;
OPEN usrcursor;
--remove each user/login individually
WHILE 1 = 1
BEGIN
FETCH NEXT FROM usrcursor INTO @UserName;
IF @@FETCH_STATUS <> 0
BREAK;
--for SQL 2005 up, use sys views
IF @sqlVersion >= 9
BEGIN
IF EXISTS(SELECT 1 FROM sys.database_principals WHERE name = @UserName)
EXEC('DROP USER [' + @UserName + '];')
IF EXISTS(SELECT 1 FROM sys.logins WHERE name = @UserName)
EXEC('DROP LOGIN [' + @UserName + '];')
END --IF
ELSE
BEGIN
--for SQL 2000, must use dbo views
IF EXISTS(SELECT 1 FROM dbo.sysusers WHERE name = @UserName)
EXEC sp_revokedbaccess @UserName
IF EXISTS(SELECT 1 FROM dbo.syslogins WHERE name = @UserName)
EXEC sp_droplogin @UserName
END --ELSE
END --WHILE
DEALLOCATE usrcursor;
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
August 28, 2015 at 12:07 pm
Thanks!
I have it working thanks to the many quick suggestions!
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply