views:

3548

answers:

9

I hope this is a simple enough question for any SQL people out there...

We have a table which hold system configuration data, and this is tied to a history table via triggers so we can track who changed what, and when.

I have a requirement to add another value in to this table, but it is one that will change frequently from code, and has a requirement that we don't track it's history (we don't want to clog the table with many thousands of updates per day.

At present, our trigger is a little like this...

CREATE TRIGGER 
    [dbo].[SystemParameterInsertUpdate]
ON 
    [dbo].[SystemParameter]
FOR INSERT, UPDATE 
AS
  BEGIN
    SET NOCOUNT ON
      INSERT INTO SystemParameterHistory 
      (
        Attribute,
        ParameterValue,
        ParameterDescription,
        ChangeDate
      )
    SELECT
      Attribute,
      ParameterValue,
      ParameterDescription,
      ChangeDate
    FROM Inserted AS I
END

I'd like to be able to add some logic to stop it creating the record if an Attribute colum value is prefixed with a specific string (e.g. "NoHist_")

Given that I have almost no experience working with triggers, I was wondering how it would be best to implement this... I have tried a where clause like the following

where I.Attribute NOT LIKE 'NoHist_%'

but it doesn't seem to work. The value is still copied over into the history table.

Any help you could offer would be appreciated.

+3  A: 

Maybe this:

CREATE TRIGGER 
    [dbo].[SystemParameterInsertUpdate]
ON 
    [dbo].[SystemParameter]
FOR INSERT, UPDATE 
AS
  BEGIN
    SET NOCOUNT ON

      If (SELECT Attribute FROM INSERTED) LIKE 'NoHist_%'
      Begin
          Return
      End

      INSERT INTO SystemParameterHistory 
      (
        Attribute,
        ParameterValue,
        ParameterDescription,
        ChangeDate
      )
    SELECT
      Attribute,
      ParameterValue,
      ParameterDescription,
      ChangeDate
    FROM Inserted AS I
END
Joel Coehoorn
Perfect. Thank you so much.
ZombieSheep
with this trigger you're only handling one row. what happens if someone inserts/updates 2 or more rows at once? data will be wrong.
Mladen Prajdic
_ is also a wildcard, so LIKE 'NoHist_%' will also match 'NoHistX'
Cade Roux
+2  A: 

How about this?

CREATE TRIGGER 
[dbo].[SystemParameterInsertUpdate]
ON 
[dbo].[SystemParameter]
FOR INSERT, UPDATE 
AS
BEGIN
SET NOCOUNT ON
  IF (LEFT((SELECT Attribute FROM INSERTED), 7) <> 'NoHist_') 
  BEGIN
      INSERT INTO SystemParameterHistory 
      (
        Attribute,
        ParameterValue,
        ParameterDescription,
        ChangeDate
      )
    SELECT
      Attribute,
      ParameterValue,
      ParameterDescription,
      ChangeDate
   FROM Inserted AS I
END
END
Matty
I like LEFT() better than my own LIKE() suggestion for this.
Joel Coehoorn
I don't know enough about the what's going on internally with SQL server to know which approach is better from a performance perspective. I actually like yours a bit better because you exit immediately rather than wrapping the entire INSERT in a conditional statement.
Matty
This one will also fail if more than one record is being inserted or updated.
Cade Roux
Good point Cade. For this to work properly it would have to iterate over the insertions/updates. This would add unnecessary overhead which could be avoided using your code snippet.
Matty
A: 

Guys - Please educate me here... Why would LEFT() be preferable to LIKE in this scenario? I know I've accepted the answer, but I'd like to know for my own education.

Thanks :)

ZombieSheep
LEFT is sometimes not prefereable to LIKE when the optimizer has a harder time looking into functions. It's always getting smarter, but in the past it could do a lot more with a few LIKE patterns than it could with a bunch of substring and string operations.
Cade Roux
+2  A: 

Your where clause should have worked. I am at a loss as to why it didn't. Let me show you how I would have figured out the problem with the where clause as it might help you for the future.

When I create triggers, I start at the query window by creating a temp table called #inserted (and or #deleted) with all the columns of the table. Then I popultae it with typical values (Always multiple records and I try to hit the test cases in the values)

Then I write my triggers logic and I can test without it actually being in a trigger. In a case like your where clause not doing what was expected, I could easily test by commenting out the insert to see what the select was returning. I would then probably be easily able to see what the problem was. I assure you that where clasues do work in triggers if they are written correctly.

Once I know that the code works properly for all the cases, I global replace #inserted with inserted and add the create trigger code around it and voila, a tested trigger.

AS I said in a comment, I have a concern that the solution you picked will not work properly in a multiple record insert or update. Triggers should always be written to account for that as you cannot predict if and when they will happen (and they do happen eventually to pretty much every table.)

HLGEM
When you say you have a concern, could you elaborate, please? I can't see another comment from you in this thread... If you mean there is a possibility that non-'NoHist_' records are dropped incorrectly, could you proposed a solution?
ZombieSheep
Yes, that's why I think it is worth it to get the where clause to work properly. If you look at the solution you took you will see that it returns if it finds a record with NoHist, therefore I suspect it wouldn't process any records in the same batch that do have a different value. Test to confirm.
HLGEM
Thanks HLGEM, I'll bear that in mind when putting some load on the app.
ZombieSheep
A: 

Using LIKE will give you options for defining what the rest of the string should look like, but if the rule is just starts with 'NoHist_' it doesn't really matter.

Daniel M
+2  A: 

The _ character is also a wildcard, BTW, but I'm not sure why this wasn't working for you:

CREATE TRIGGER 
    [dbo].[SystemParameterInsertUpdate]
ON 
    [dbo].[SystemParameter]
FOR INSERT, UPDATE 
AS
  BEGIN
    SET NOCOUNT ON
      INSERT INTO SystemParameterHistory 
      (
        Attribute,
        ParameterValue,
        ParameterDescription,
        ChangeDate
      )
    SELECT
      I.Attribute,
      I.ParameterValue,
      I.ParameterDescription,
      I.ChangeDate
    FROM Inserted AS I
    WHERE I.Attribute NOT LIKE 'NoHist[_]%'
END
Cade Roux
A: 

OK - as predicted by Cade Roux, this fail spectacularly on multiple updates. I'm going to have to take a new approach to this. Does anyone have any other suggestions, please?

ZombieSheep
A: 

For triggers in general, you need to use a cursor to handle inserts or updates of multiple rows. For example:

DECLARE @Attribute;
DECLARE @ParameterValue;
DECLARE mycursor CURSOR FOR SELECT Attribute, ParameterValue FROM inserted;
OPEN mycursor;
FETCH NEXT FROM mycursor into @Attribute, @ParameterValue;
WHILE @@FETCH_STATUS = 0
BEGIN

If @Attribute LIKE 'NoHist_%'
      Begin
          Return
      End

etc.

FETCH NEXT FROM mycursor into @Attribute, @ParameterValue;
END

Triggers, at least in SQL Server, are a big pain and I avoid using them at all.

A: 
CREATE TRIGGER
    [dbo].[SystemParameterInsertUpdate]
ON 
    [dbo].[SystemParameter]
FOR INSERT, UPDATE 
AS
  BEGIN
    SET NOCOUNT ON 

    DECLARE @StartRow int
    DECLARE @EndRow int
    DECLARE @CurrentRow int

    SET @StartRow = 1
    SET @EndRow = (SELECT count(*) FROM inserted)
    SET @CurrentRow = @StartRow

    WHILE @CurrentRow <= @EndRow BEGIN

        IF (SELECT Attribute FROM (SELECT ROW_NUMBER() OVER (ORDER BY Attribute ASC) AS 'RowNum', Attribute FROM inserted) AS INS WHERE RowNum = @CurrentRow) LIKE 'NoHist_%' BEGIN

            INSERT INTO SystemParameterHistory(
                Attribute,
                ParameterValue,
                ParameterDescription,
                ChangeDate)
            SELECT
                I.Attribute,
                I.ParameterValue,
                I.ParameterDescription,
                I.ChangeDate
            FROM
                (SELECT Attribute, ParameterValue, ParameterDescription, ChangeDate FROM (
                                                                                            SELECT ROW_NUMBER() OVER (ORDER BY Attribute ASC) AS 'RowNum', * 
                                                                                            FROM inserted)
                                                                                    AS I 
            WHERE RowNum = @CurrentRow

        END --END IF

    SET @CurrentRow = @CurrentRow + 1

    END --END WHILE
END --END TRIGGER
QuintessentialSolutions