Assigning permissions to new role without RBAR

I'm currently faced with a problem in our SQL Server environments. We need to migrate our existing logins, users and roles to a new login, user and role with the same permissions as the old. I've been tasked with writing a stored procedure that will accept 3 parameters:

• Database Name

• Old Role

• New Role

Using these parameters, I need to query the system views to get the list of all the objects assigned to a Role and then execute some process to assign these permissions to the new role.

I've written a stored procedure that works but unfortunately uses a WHILE loop and a row-by-agonizing-row dynamic sql execution to assign the permissions. I'd like to know if there are any improvements that would be made to the code to maybe avoid the RBAR process or any other potential pitfalls that are apparent in the code.

```
CREATE PROCEDURE dbo.usp_AssignPermissionsToRole
(
@OldRoleName VARCHAR(50),
@NewRoleName VARCHAR(50),
@Database VARCHAR(50)
)
AS
BEGIN
DECLARE @DataSQL NVARCHAR(MAX);
DECLARE @PermissionSQL NVARCHAR(MAX);
DECLARE @Start INT = 1;
DECLARE @Total INT;
DECLARE @PermissionState VARCHAR(60);
DECLARE @PermissionName VARCHAR(500);
DECLARE @SchemaName VARCHAR(50);
DECLARE @ObjectName VARCHAR(500);

IF(OBJECT_ID('tempdb..#OldPermissions') IS NOT NULL)
DROP TABLE #OldPermissions;

-- get the list of all permissions for the OldRole
CREATE TABLE #OldPermissions
(
ID INT IDENTITY,
RoleName VARCHAR(50),
PermissionState VARCHAR(60),
PermissionName VARCHAR(500),
SchemaName VARCHAR(50),
ObjectName VARCHAR(500)
);

SET @DataSQL = 'USE '+QUOTENAME(@Database)+'
INSERT INTO #OldPermissions
(
RoleName,
PermissionState,
PermissionName,
SchemaName,
ObjectName
)

Alright, here is my take on it:

CREATE PROCEDURE dbo.usp_AssignPermissionsToRole
(
    @OldRoleName VARCHAR(50),
    @NewRoleName VARCHAR(50),
    @Database VARCHAR(50)
)
AS
BEGIN
    DECLARE @DataSQL NVARCHAR(MAX);
    DECLARE @PermissionSQL NVARCHAR(MAX) = '';
    DECLARE @Start INT = 1;
    DECLARE @Total INT;
    DECLARE @PermissionState VARCHAR(60);
    DECLARE @PermissionName VARCHAR(500);
    DECLARE @SchemaName VARCHAR(50);
    DECLARE @ObjectName VARCHAR(500);

    -- This check is unnecessary
    --IF(OBJECT_ID('tempdb..#OldPermissions') IS NOT NULL)
    --    DROP TABLE #OldPermissions;

    -- I would check for the existence of the new role here:
    DECLARE @RoleExistsSQL NVARCHAR(MAX);
    DECLARE @ReturnVal INT;
    SET @RoleExistsSQL = 'USE '+QUOTENAME(@Database)+' SELECT @ReturnVal=1 FROM sys.database_principals WHERE [type] = ''R'' AND [name] = '+QUOTENAME(@NewRoleName, '''');
    EXEC sp_executesql @RoleExistsSQL, N'@ReturnVal INT OUTPUT', @ReturnVal OUTPUT;

    IF (@ReturnVal <> 1)
        BEGIN
            PRINT 'Role: ' + @NewRoleName +' not set up in this database.';
            RETURN;
        END
    ELSE
        BEGIN
            -- get the list of all permissions for the OldRole
            CREATE TABLE #OldPermissions
            (
                ID INT IDENTITY,
                RoleName VARCHAR(50),
                PermissionState VARCHAR(60),
                PermissionName VARCHAR(500),
                SchemaName VARCHAR(50),
                ObjectName VARCHAR(500)
            );

            SET @DataSQL = 'USE '+QUOTENAME(@Database)+'
                            INSERT INTO #OldPermissions
                            (
                                RoleName,
                                PermissionState,
                                PermissionName,
                                SchemaName,
                                ObjectName
                            )
                            SELECT 
                                RoleName = r.name,
                                PermissionState = dp.state_desc,
                                dp.permission_name,
                                SchemaName = s.name,
                                ObjectName = o.name
                            FROM sys.database_permissions dp
                            INNER JOIN sys.database_principals r
                                ON dp.grantee_principal_id = r.principal_id
                            INNER JOIN sys.objects o
                                ON dp.major_id = o.[object_id]
                            INNER JOIN sys.schemas s
                                ON o.schema_id = s.schema_id
                            WHERE r.name = '+QUOTENAME(@OldRoleName, '''');

            EXEC sp_executesql @DataSQL; --Changed the EXEC to sp_executesql for consistency

            -- I concatenated all the commands needed on the same string to execute it once
            SELECT @PermissionSQL = @PermissionSQL + PermissionState + ' ' + PermissionName + ' on ' + 
                                    QUOTENAME(SchemaName)+'.'+QUOTENAME(ObjectName)+' to ' + QUOTENAME(@NewRoleName) + '; '
            FROM #OldPermissions;

            SET @PermissionSQL = 'USE  '+QUOTENAME(@Database)+' '+@PermissionSQL;
            EXEC sp_executesql @PermissionSQL;

        END

END

The main changes are checking existence of the new role at the beginning of the sp to avoid unnecessary creating and populating the temp table with the data of the old role. Then I concatenated all the GRANT or DENY permissions on the same string (using an undocumented behaviour I'm afraid) without using a WHILE so you need to execute it only once.

StackExchange Code Review Q#86438, answer score: 4