Link to home
Get AccessLog in
Avatar of spen_lang
spen_lang

asked on

MS SQL Archive Table - Restore

Hi,

I have a table named t_Devices which has an ID column as an Identity column. The ID column is used to as the device ID and primary key. When a user deletes a device a trigger runs to insert the record into a archive table t_DevicesArchive.

What I now need to do is create a stored procedure to restore the archive record from t_DevicesArchive to t_Devices. I have created an procedure named p_DeviceArchive_Restore but the user must have the ALTER permission on the table, which I do not want to set.

Is there a better way to do this, but I need to have an identity column for the Device ID? Or should I just execute the stored procedure as the owner...

Please see the code below:

Device Table:
CREATE TABLE [dbo].[tbl_Devices](
	[Device_ID] [int] IDENTITY(1,1) NOT NULL,
	[Device_Name] [varchar](30) NOT NULL,
	[Device_LocationID] [int] NOT NULL,
	[Device_Location2] [varchar](30) NULL,
	[Device_MACAddress] [varchar](20) NULL,
	[Device_IPAddress] [varchar](15) NULL,
	[Device_Active] [bit] NOT NULL,
	[Device_CreateDateTime] [datetime] NOT NULL,
	[Device_ModifiedDateTime] [datetime] NULL,
	[Device_Manufacturer] [varchar](50) NULL,
	[Device_Model] [varchar](50) NULL,
	[Device_OS] [varchar](50) NULL,
 CONSTRAINT [PK_tbl_Devices] PRIMARY KEY CLUSTERED )

Open in new window



Device Archive Table:
CREATE TABLE [dbo].[t_DevicesArchive](
	[Device_ID] [int] NOT NULL,
	[Device_Name] [varchar](30) NOT NULL,
	[Device_LocationID] [int] NOT NULL,
	[Device_Location2] [varchar](30) NULL,
	[Device_MACAddress] [varchar](20) NULL,
	[Device_IPAddress] [varchar](15) NULL,
	[Device_Active] [bit] NOT NULL,
	[Device_CreateDateTime] [datetime] NOT NULL,
	[Device_ModifiedDateTime] [datetime] NULL,
	[Device_Manufacturer] [varchar](50) NULL,
	[Device_Model] [varchar](50) NULL,
	[Device_OS] [varchar](50) NULL
) ON [PRIMARY]

Open in new window


Delete trigger:
CREATE TRIGGER [dbo].[tr_Devices_Delete]
ON [dbo].[t_Devices]
AFTER DELETE AS

BEGIN

	INSERT INTO dbo.t_DevicesArchive
	SELECT * FROM Deleted

Open in new window


Restore procedure:
CREATE PROCEDURE p_DevicesArchive_Restore
(
	@Device_ID INT
)

AS

BEGIN
	
	SET IDENTITY_INSERT dbo.t_Devices ON;
	
	INSERT INTO dbo.t_Devices
		(
			Device_ID,
			Device_Name,
			Device_LocationID,
			Device_Location2,
			Device_MACAddress,
			Device_IPAddress,	
			Device_Active,
			Device_CreateDateTime,
			Device_ModifiedDateTime,
			Device_Manufacturer,
			Device_Model,
			Device_OS	
		)
	SELECT
		Device_ID,
		Device_Name,
		Device_LocationID,
		Device_Location2,
		Device_MACAddress,
		Device_IPAddress,	
		Device_Active,
		Device_CreateDateTime,
		Device_ModifiedDateTime,
		Device_Manufacturer,
		Device_Model,
		Device_OS
	FROM
		dbo.t_DevicesArchive
	WHERE
		Device_ID = @Device_ID;
		
	SET IDENTITY_INSERT dbo.t_Devices OFF;
	
	SELECT @@ERROR 
	
END

Open in new window

Avatar of ste5an
ste5an
Flag of Germany image

Use EXECUTE AS OWNER.

Caveat: Your model is flawed. Consider deleting a device, restoring it, deleting it. This will lead to two rows in your history table. Thus a subsequent restore will fail.
Avatar of spen_lang
spen_lang

ASKER

Thanks ste5an I noticed this just after I posted the question...

I have updated the stored procedure to delete the record from the archive if the restore succeeds as the record is now current and not archive.

So I presume the SET IDENTITY_INSERT dbo.t_Devices ON; is the best method to use and set the line WITH EXECUTE AS OWNER

ALTER PROCEDURE [dbo].[p_DevicesArchive_Restore]
(
	@Device_ID INT
)
WITH EXECUTE AS OWNER
AS

BEGIN
	
	SET IDENTITY_INSERT dbo.t_Devices ON;
	
	INSERT INTO dbo.t_Devices
		(
			Device_ID,
			Device_Name,
			Device_LocationID,
			Device_Location2,
			Device_MACAddress,
			Device_IPAddress,	
			Device_Active,
			Device_CreateDateTime,
			Device_ModifiedDateTime,
			Device_Manufacturer,
			Device_Model,
			Device_OS	
		)
	SELECT
		Device_ID,
		Device_Name,
		Device_LocationID,
		Device_Location2,
		Device_MACAddress,
		Device_IPAddress,	
		Device_Active,
		Device_CreateDateTime,
		Device_ModifiedDateTime,
		Device_Manufacturer,
		Device_Model,
		Device_OS
	FROM
		dbo.t_DevicesArchive
	WHERE
		Device_ID = @Device_ID;
		
	SET IDENTITY_INSERT dbo.tbl_Devices OFF;
	
	IF @@ERROR = 0
		BEGIN
		
			DELETE FROM dbo.t_DevicesArchive
			WHERE
				Device_ID = @Device_ID 
		
		END
	
	SELECT @@ERROR 
	
END

Open in new window

Don't use the IF @@ERROR construct. Use an explicit transaction instead. E.g. something like this

SET ANSI_NULLS ON;
GO
SET QUOTED_IDENTIFIER ON;
GO

CREATE PROCEDURE dbo.p_RestoreDevice ( @DeviceID INT )
AS
    BEGIN TRY                                                         
        SET NOCOUNT ON;
        SET XACT_ABORT ON;

	-- Constants.
        DECLARE @NO_ERROR INT = 0;
        DECLARE @GENERIC_ERROR INT = 55555;

	-- Variables.        
        DECLARE @Result INT = @NO_ERROR;

	-- Use explicit transaction.
        BEGIN TRANSACTION;

        SET IDENTITY_INSERT dbo.t_Devices ON;
	
        INSERT  INTO dbo.t_Devices
                ( Device_ID ,
                  Device_Name ,
                  Device_LocationID ,
                  Device_Location2 ,
                  Device_MACAddress ,
                  Device_IPAddress ,
                  Device_Active ,
                  Device_CreateDateTime ,
                  Device_ModifiedDateTime ,
                  Device_Manufacturer ,
                  Device_Model ,
                  Device_OS	
		        )
                SELECT  Device_ID ,
                        Device_Name ,
                        Device_LocationID ,
                        Device_Location2 ,
                        Device_MACAddress ,
                        Device_IPAddress ,
                        Device_Active ,
                        Device_CreateDateTime ,
                        Device_ModifiedDateTime ,
                        Device_Manufacturer ,
                        Device_Model ,
                        Device_OS
                FROM    dbo.t_DevicesArchive
                WHERE   Device_ID = @Device_ID;
		
        SET IDENTITY_INSERT dbo.tbl_Devices OFF;
	
        DELETE  FROM dbo.t_DevicesArchive
        WHERE   Device_ID = @Device_ID; 
	
	-- Use explicit transaction.
        COMMIT TRANSACTION;
		
        RETURN @Result;
    END TRY  
    BEGIN CATCH
        IF ( @@trancount > 0 )
            ROLLBACK TRANSACTION;
        EXECUTE dbo.p_ThrowError;
        RETURN @GENERIC_ERROR;
    END CATCH;
GO

CREATE PROCEDURE dbo.p_ThrowError
AS
    SET NOCOUNT ON;

    -- Variables.
    DECLARE @ErrorMessage NVARCHAR(2048) = ERROR_MESSAGE();
    DECLARE @ErrorSeverity TINYINT = ERROR_SEVERITY();
    DECLARE @ErrorState TINYINT = ERROR_STATE();
    DECLARE @ErrorNumber INT = ERROR_NUMBER();
    DECLARE @ErrorProcedure sysname = ERROR_PROCEDURE();
    DECLARE @ErrorLine INT = ERROR_LINE();           

    -- Mark error with leading asterisks to distingush between compiliation and run-time errors.
    IF ( @ErrorMessage NOT LIKE '***%' )
        BEGIN 
            SET @ErrorMessage = '*** ' + COALESCE(QUOTENAME(@ErrorProcedure), '<dynamic SQL>') + ', ' + LTRIM(STR(@ErrorLine)) + '. Error #'
                + LTRIM(STR(@ErrorNumber)) + ': ' + @ErrorMessage;
        END;

    SET @ErrorNumber += 50000;
    THROW @ErrorNumber, @ErrorMessage, @ErrorState;
GO

Open in new window

 

See also Erland's article about Error and Transaction Handling in SQL Server.
ASKER CERTIFIED SOLUTION
Avatar of Scott Pletcher
Scott Pletcher
Flag of United States of America image

Link to home
membership
This content is only available to members.
To access this content, you must be a member of Experts Exchange.
Get Access
Good idea, but the transaction is still necessary.

Otherwise it may fail when having the wrong timing. Cause some other thread may delete the inserted row before the procedure executes the delete.

An explicit transaction will hold a X lock until the row is deleted.
Seriously?  You think realistically that some other transaction is going to delete the row I just inserted before I can immediately check that the row exists??  How would it get that identity value to delete??

There's nothing wrong with a transaction here, of course, but I'm not sure that it's realistically required.
No, just kidding..