views:

272

answers:

3

I have the following trigger, which causes an error when it runs:

CREATE TRIGGER ...
ON ...
FOR INSERT, UPDATE
AS   

IF UPDATE(STATUS)
BEGIN

    DECLARE @newPrice VARCHAR(50)
    DECLARE @FILENAME VARCHAR(50)
    DECLARE @server VARCHAR(50)
    DECLARE @provider VARCHAR(50)
    DECLARE @datasrc VARCHAR(50)
    DECLARE @location VARCHAR(50)
    DECLARE @provstr VARCHAR(50)
    DECLARE @catalog VARCHAR(50)
    DECLARE @DBNAME VARCHAR(50)

    SET @server=xx
    SET @provider=xx
    SET @datasrc=xx
    SET @provstr='DRIVER={SQL Server};SERVER=xxxxxxxx;UID=xx;PWD=xx;'
    SET @DBNAME='[xx]'

    SET @newPrice = (SELECT STATUS FROM Inserted)
    SET @FILENAME = (SELECT INPUT_XML_FILE_NAME FROM Inserted)

    IF @newPrice = 'FAIL'     
    BEGIN
        EXEC master.dbo.sp_addlinkedserver
            @server, '', @provider, @datasrc, @provstr

        EXEC master.dbo.sp_addlinkedsrvlogin @server, 'true'

        INSERT INTO [@server].[@DBNAME].[dbo].[maildetails]
        (
            'to', 'cc', 'from', 'subject', 'body', 'status',
            'Attachment', 'APPLICATION', 'ID', 'Timestamp', 'AttachmentName'
        )
        VALUES
        (
            'P23741', '', '', 'XMLFAILED', @FILENAME, '4',
            '', '8', '', GETDATE(), ''
        )

        EXEC sp_dropserver @server
    END

END

The error is:

Msg 15002, Level 16, State 1, Procedure sp_MSaddserver_internal, Line 28 The procedure 'sys.sp_addlinkedserver' cannot be executed within a transaction. Msg 15002, Level 16, State 1, Procedure sp_addlinkedsrvlogin, Line 17 The procedure 'sys.sp_addlinkedsrvlogin' cannot be executed within a transaction. Msg 15002, Level 16, State 1, Procedure sp_dropserver, Line 12 The procedure 'sys.sp_dropserver' cannot be executed within a transaction.

How can I prevent this error from occurring?

+1  A: 

As the error clearly states, you can't add a linked server within a transaction. Triggers run as an implicit transaction so you can ROLLBACK if the trigger performs any validation and that validation fails.

I really can't imagine why you'd want to do this anyway. With a few very rare exceptions which I won't bother to mention here, a linked server is not ideal as a temporary fixture. Just add the linked server once, permanently, then you won't have this issue. You'll also be able to treat its management and security as an administrative function rather than hard-coding it in a script somewhere.

Aaronaught
Permanent linked servers are server-level objects. For those of us that share servers with other development teams, this is one more coordination point with those dev teams that can be avoided. If you use automated build tools, like NAnt, to coordinate deployment of DDL changes across enviroments, you want as much stuff as possible at the database level or below.
entaroadun
@entaroadun: The flip side is that `OPENROWSET` requires both special permissions on the source server and saved credentials to the destination. This is probably not a problem for development, but many teams might not want it in production.
Aaronaught
A: 

Use OPENROWSET if you want to reach a remote server dynamically. I do it all the time.

entaroadun
+1  A: 

Your trigger is bad in many respects. First, you can't add a linked server in a trigger. You should add it once administratively and not worry about it again.

Next and extremely important, even with the linked server gone, your trigger will only work if one row is inserted and updated and will not work properly if multiple row inserts/updates occur. This is the first, most basic, rule of trigger design, never assume only one row will be processed. Anytime I see a values clause used or the value from inserted or deleted set to a variable I know the trigger is bad and needs to be rewritten. Now this appears to send out an email, do you really want to send out 1907898 emails if that many records were updated or only one? If you only want one, you need a way to identify all the ids affected. What do you really want to have happen if someone needs to update a whole bunch of prices and does it through a set-based update statment rather than manually going through the user interface one at a time for 10,000 prices? And don't say that ony one record ever will get updated or inserted. Sooner or later someone will need to do a batch insert or update and your trigger will silently cause the wrong thing to happen. You won't even know it failed because it won't error out it simply won't do what you need it to do. This is how nightmare, impossible to fix, data integrity issues occcur.

Another thing, the way this is written the db won't change, so you no longer need a variable witht the linked server stuff removed. Otherwise as wrttten you would have to change to dynamic sql to get this to work properly and that's a poor idea in a trigger (Or anywhere else usually) and since it is not going to vary there is no reason at all to use it.

A set-based solution (Assuming you want one record for every item inserted or updated and assuming you set-up a permanent linked server):

INSERT INTO myserver].mydatabase.[dbo].[maildetails] 
        ( 
            'to', 'cc', 'from', 'subject', 'body', 'status', 
            'Attachment', 'APPLICATION', 'ID', 'Timestamp', 'AttachmentName' 
SELECT   'P23741', '', '', 'XMLFAILED', INPUT_XML_FILE_NAME , '4', 
            '', '8', '', GETDATE(), '' 
FROM inserted
WHERE status = 'Fail'

My final caution to you is that even this will fail if the linked server goes down for any reason. This means while it is down, no records can be added or changed in the table. Consider this very carefully before putting this in a trigger.

HLGEM