CODEWITHSUNDEEP
sqlsql-serverdatabasetriggers
Johan Hjalmarsson
Johan Hjalmarsson

Reputation: 3493

Trigger doesn't work as expected

I'm having a bit of a trouble with my Trigger. It's supposed to:

Sloppy pseudocode:

IF NOT EXISTS ( --if the value isn't in the history table
    SELECT History.value1 FROM History, INSERTED
    WHERE History.value1 LIKE INSERTED.value1
    AND History.value2 LIKE INSERTED.value2
       ) 
       OR EXISTS ( --or if it has been added over 24h ago
        SELECT History.value1 FROM History, INSERTED
        WHERE History.value1 LIKE INSERTED.value1
        AND History.value2 LIKE INSERTED.value2
        AND DATEDIFF(HOUR,History.time, GETDATE()) > 24
       )     
    BEGIN --Insert it
        INSERT INTO History(value1, value2, counter, time)
        SELECT value1, value2, counter GETDATE() FROM INSERTED
    END
    ELSE
    BEGIN  -- else, increase counter and add new time
        UPDATE History
        SET History.time = GETDATE(), 
                               History.Items = History.Items + INSERTED.Items
        FROM History
        JOIN INSERTED ON History.value1 = INSERTED.value1
        AND History.value2 = INSERTED.value2
            AND DATEDIFF(HOUR, _History.time, GETDATE()) < 24;
    END

example table:

__________________________________________________
| value1 | value2 | counter | time(last updated) |
+------------------------------------------------+
| test1  | test2  |    1    |        < 24h       |
| test3  | test4  |    1    |        > 24h       |
| test3  | test4  |    1    |        < 24h       |
+------------------------------------------------+

input: 

INSERT INTO main_table(value1, value2, counter)
VALUES ('test3', 'test4', 1);

resulting table:

__________________________________________________
| value1 | value2 | counter | time(last updated) |
+------------------------------------------------+
| test1  | test2  |    1    |        < 24h       |
| test3  | test4  |    1    |        > 24h       |
| test3  | test4  |    1    |        < 24h       | <--This counter+time should be updated
| test3  | test4  |    1    |        < 24h       | <--This row shouldn't be added
+------------------------------------------------+

I Understand WHY this happens (because the code finds a history value thats over 24h, disregarding the newer one(s)) but I dont know how to fix it.

Upvotes: 0

Views: 99

Answers (3)

gavin
gavin

Reputation: 11

--Change your criteria statements like this. it will work. Anyway, the trigger you write really not good. Triggers should design to deal with set not records, as one batch insert/update/delete only can trigger once.

IF NOT EXISTS ( --if the value isn't in the history table
SELECT History.value1 FROM History, INSERTED
WHERE History.value1 LIKE INSERTED.value1
AND History.value2 LIKE INSERTED.value2
   ) 
   OR EXISTS ( --or if it has been added over 24h ago
    SELECT INSERTED.value1 
    FROM INSERTED
    cross apply
        (select max(History.[time]) mx_time
        from    History
        where   History.value1 LIKE INSERTED.value1
                AND History.value2 LIKE INSERTED.value2
        ) as t
    WHERE DATEDIFF(HOUR,t.mx_time, GETDATE()) >= 24
   )     
BEGIN --Insert it
    INSERT INTO History(value1, value2, items, time)
    SELECT value1, value2, items, GETDATE() FROM INSERTED
END
ELSE
BEGIN  -- else, increase counter and add new time

    UPDATE History
    SET History.time = GETDATE(), 
                           History.Items = History.Items + INSERTED.Items
    FROM History
    JOIN INSERTED ON History.value1 = INSERTED.value1
    AND History.value2 = INSERTED.value2
        AND DATEDIFF(HOUR, History.time, GETDATE()) < 24;
END

Upvotes: 1

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239646

I think your trigger is still broken if inserted contains a mixture of rows - since your IF/ELSE structure makes a single decision on what action to take.

It would be better to have a MERGE, something like:

;MERGE INTO History h USING INSERTED i
   ON h.Value1 = i.Value1 and h.Value2 = i.Value2 and
      DATEDIFF(HOUR,h.time, GETDATE()) <= 24
WHEN MATCHED THEN
   UPDATE SET time = GETDATE(), Items = h.Items + i.Items
WHEN NOT MATCHED THEN
   INSERT (Value1,Value2,Items,time)
   VALUES (i.Value1,i.Value2,i.Items,GETDATE());

Which should replace your whole trigger body.

Incidentally, DATEDIFF counts the number of transitions across boundaries, rather than computing an exact difference (E.g. DATEDIFF(hour,'00:59','01:01') is 1). If you want to get closer to a 24 hour cutoff which also considers minutes and seconds, a better comparison would be:

h.Time >= DATEADD(day,-1,GETDATE())

Which incidentally would also allow an index that includes History's time column to be used.

Upvotes: 1

Johan Hjalmarsson
Johan Hjalmarsson

Reputation: 3493

OMG I solved it ^^ Been trying to fix it for like 2 hours and just when I post it to stackoverflow I manage to fix it ^^

What I did:

Changed the second "EXIST" test from:

OR EXISTS ( --or if it has been added over 24h ago
        SELECT History.value1 FROM History, INSERTED
        WHERE History.value1 LIKE INSERTED.value1
        AND History.value2 LIKE INSERTED.value2
        AND DATEDIFF(HOUR,History.time, GETDATE()) > 24
       )

to:

OR NOT EXISTS ( --or if it has been added over 24h ago
        SELECT History.value1 FROM History, INSERTED
        WHERE NOT History.value1 LIKE INSERTED.value1
        OR History.value2 LIKE INSERTED.value2
        OR DATEDIFF(HOUR,History.time, GETDATE()) > 24
       )

Upvotes: 0

Related Questions