PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197
[RESOLVED] Please fact-check my simple trigger-VBForums
Results 1 to 9 of 9

Thread: [RESOLVED] Please fact-check my simple trigger

  1. #1

    Thread Starter
    PowerPoster MMock's Avatar
    Join Date
    Apr 2007
    Location
    Sitting in an empty room trying to forget the past
    Posts
    4,107

    Resolved [RESOLVED] Please fact-check my simple trigger

    I have a small table that has a column called SystemRelease with date/id created and date/id modified.
    I don't need a history, but whenever a user edits the SystemRelease I want to capture who is the user and when they updated it.
    So I believe it would be this:
    Code:
    CREATE TRIGGER  [dbo].[tg_xtblSystemReleaseRecordUpdated] 
       ON  [dbo].[xtblSystemRelease]
       FOR UPDATE
    AS 
    DECLARE @Control int
    SELECT @Control = [Control] FROM Inserted
    
    UPDATE [xtblSystemRelease] SET ModifiedBy = SUSER_SNAME(), ModifiedDate = getdate() WHERE Control = @Control
    
    GO
    I just want to make sure,
    1. It's "FOR UPDATE" not "AFTER UPDATE"
    2. It's updating the right record. How does it know which record to grab from Inserted if it's such a non-specific name as Control?

    Does anything else look wrong? We only have a handful of existing triggers currently in our database, and honestly I am nervous copying one if I don't understand it 100%.
    Thanks.
    There are 10 kinds of people in this world. Those who understand binary, and those who don't.

  2. #2
    PowerPoster Zvoni's Avatar
    Join Date
    Sep 2012
    Location
    To the moon and then left
    Posts
    1,911

    Re: Please fact-check my simple trigger

    Uhuh.

    Is it just me, or does this look like an infinite loop?
    Someone changes something in xtblSystemRelease (successfully!), the trigger activates, and within the trigger the same table is updated again (trigger activates again?).

    As for your question with that "Control"-Thing: Control has to be a column of xtblSystemRelease (PrimaryKey?) otherwise the UPDATE-Statement wouldn't work (Try the UPDATE-Statement manually).

    Syntax:
    https://docs.microsoft.com/en-us/sql...ql-server-2017

    FOR | AFTER
    AFTER specifies that the DML trigger fires only when all operations specified in the triggering SQL statement have launched successfully. All referential cascade actions and constraint checks must also succeed before this trigger fires.
    AFTER is the default when FOR is the only keyword specified.
    So, basically it is an AFTER UPDATE-Statement (twice so, because you're checking the INSERTED-Table)
    One System to rule them all, One IDE to find them,
    One Code to bring them all, and to the Framework bind them,
    in the Land of Redmond, where the Windows lie
    ---------------------------------------------------------------------------------
    People call me crazy because i'm jumping out of perfectly fine airplanes.
    ---------------------------------------------------------------------------------
    For health reasons i try to avoid reading unformatted Code

  3. #3
    PowerPoster Zvoni's Avatar
    Join Date
    Sep 2012
    Location
    To the moon and then left
    Posts
    1,911

    Re: Please fact-check my simple trigger

    *GROWL*
    Couldn't edit my own post (too short?!?!?!?!)


    EDIT: Here is a solution to avoid Recursion:
    https://dev.to/ravenous_baboon/check...update-trigger
    One System to rule them all, One IDE to find them,
    One Code to bring them all, and to the Framework bind them,
    in the Land of Redmond, where the Windows lie
    ---------------------------------------------------------------------------------
    People call me crazy because i'm jumping out of perfectly fine airplanes.
    ---------------------------------------------------------------------------------
    For health reasons i try to avoid reading unformatted Code

  4. #4
    PowerPoster Zvoni's Avatar
    Join Date
    Sep 2012
    Location
    To the moon and then left
    Posts
    1,911

    Re: Please fact-check my simple trigger

    OH, you gotta be kidding me.
    What's wrong with the forum today????

    EDIT2: You do realize, that your trigger is going to fail, if more than one row is UPDATED?
    I'm assuming it's MS SQL-Server, so you would have to implement a FOR EACH ROW-Syntax (which MS SQL doesn't support in that way)
    See here: https://social.msdn.microsoft.com/Fo...=sqlgetstarted
    One System to rule them all, One IDE to find them,
    One Code to bring them all, and to the Framework bind them,
    in the Land of Redmond, where the Windows lie
    ---------------------------------------------------------------------------------
    People call me crazy because i'm jumping out of perfectly fine airplanes.
    ---------------------------------------------------------------------------------
    For health reasons i try to avoid reading unformatted Code

  5. #5

    Thread Starter
    PowerPoster MMock's Avatar
    Join Date
    Apr 2007
    Location
    Sitting in an empty room trying to forget the past
    Posts
    4,107

    Re: Please fact-check my simple trigger

    Okay, thanks. This is why I asked. There are some triggers in our database already so I was using them as an example, but I didn't want to blindly use them without fully understanding.
    Regarding recursion, here is an existing trigger:
    Code:
    CREATE TRIGGER [dbo].[tg_EquipmentRecordUpdated] ON [dbo].[tblEquipment] 
    FOR INSERT, UPDATE 
    AS
    DECLARE @EquipmentControl int
    SELECT @EquipmentControl = EquipmentControl FROM Inserted
    
    UPDATE tblEquipment SET LastModifiedBy = SUSER_SNAME(), LastModifiedOn = getdate() WHERE EquipmentControl = @EquipmentControl
    
    GO
    So you are saying there is a problem with it, because it is updating the same table which an update to caused the trigger in the first place???
    There are 10 kinds of people in this world. Those who understand binary, and those who don't.

  6. #6
    PowerPoster Zvoni's Avatar
    Join Date
    Sep 2012
    Location
    To the moon and then left
    Posts
    1,911

    Re: Please fact-check my simple trigger

    Yes, but i have to revise my argument with the infinite loop.
    Your Trigger gets triggered twice (and not as with my first assumption in an endless loop).

    First Trigger --> EquipmentControl has been inserted/updated (as in your current example) --> Update LastMod-Fields
    Second Trigger --> LastMod-Fields have been updated. But since you're checking against EquipmentControl, which hasn't been changed in this second step, nothing happens in the UPDATE-Part.

    Neverthless, i'd implement some sanity-check
    Something like
    Aircode in red
    Code:
    CREATE TRIGGER [dbo].[tg_EquipmentRecordUpdated] ON [dbo].[tblEquipment] 
    FOR INSERT, UPDATE 
    AS
    BEGIN
    DECLARE @EquipmentControl int
    IF EXISTS(SELECT @EquipmentControl = EquipmentControl FROM Inserted) THEN
    UPDATE tblEquipment SET LastModifiedBy = SUSER_SNAME(), LastModifiedOn = getdate() WHERE EquipmentControl = @EquipmentControl
    END
    
    GO
    That way, only if EquipmentControl has been changed/inserted, it enters into the UPDATE-part to update the LastMod-Fields.
    It will still get triggered twice, but the second pass will be forked by the IF-Clause, and jumps directly to the end without trying to execute the UPDATE-Statement (as in your version right now).

    And you should definitely check for multiple rows being inserted/updated. A trigger gets triggered per Event, not per Row
    Last edited by Zvoni; Feb 27th, 2019 at 08:22 AM.
    One System to rule them all, One IDE to find them,
    One Code to bring them all, and to the Framework bind them,
    in the Land of Redmond, where the Windows lie
    ---------------------------------------------------------------------------------
    People call me crazy because i'm jumping out of perfectly fine airplanes.
    ---------------------------------------------------------------------------------
    For health reasons i try to avoid reading unformatted Code

  7. #7
    Super Moderator FunkyDexter's Avatar
    Join Date
    Apr 2005
    Location
    An obscure body in the SK system. The inhabitants call it Earth
    Posts
    7,345

    Re: Please fact-check my simple trigger

    so you would have to implement a FOR EACH ROW-Syntax
    You don't need to put a loop in, just join to the Inserted table in your update:-

    sql Code:
    1. UPDATE T
    2. SET LastModifiedBy = SUSER_SNAME(),
    3.     LastModifiedOn = getdate()
    4. FROM Inserted I
    5. JOIN tblEquipment T
    6.     ON I.Control = T.Control  --<--I'm assuming Control is the primary key.  If not, do the join on the primary key.
    7.  
    8. --and here's a quick and dirty way of preventing any recursion/double firing etc.
    9. --The trigger will fire twice but only result in an update once:-
    10. WHERE I.LastModifiedBy <> T.LastModifiedBy
    11. AND I.LastModifiedOn <> T.LastModifiedOn
    12. --I'm assuming neither column can contain nulls.  If they can you should handle nulls apropriately
    You can depend upon the Americans to do the right thing. But only after they have exhausted every other possibility - Winston Churchill

    Hadoop actually sounds more like the way they greet each other in Yorkshire - Inferrd

  8. #8

    Thread Starter
    PowerPoster MMock's Avatar
    Join Date
    Apr 2007
    Location
    Sitting in an empty room trying to forget the past
    Posts
    4,107

    Re: Please fact-check my simple trigger

    Awesome! Thank you!
    There are 10 kinds of people in this world. Those who understand binary, and those who don't.

  9. #9
    PowerPoster Zvoni's Avatar
    Join Date
    Sep 2012
    Location
    To the moon and then left
    Posts
    1,911

    Re: Please fact-check my simple trigger

    FD,
    Nice!
    +1
    One System to rule them all, One IDE to find them,
    One Code to bring them all, and to the Framework bind them,
    in the Land of Redmond, where the Windows lie
    ---------------------------------------------------------------------------------
    People call me crazy because i'm jumping out of perfectly fine airplanes.
    ---------------------------------------------------------------------------------
    For health reasons i try to avoid reading unformatted Code

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  



Featured


Click Here to Expand Forum to Full Width