Insert with trigger works with cursor, not set

  • I have a trigger on a table, which computes a string value using a UDF. The table has a self-referencing cascade structure, and complete lookups are expensive, so I put in this extra field to make selects faster. I can't make it a persisted computed field, since it uses a self-reference, so I put a trigger on the table that computes or recomputes the field whenever a record is added or updated. It works okay when I add one record at a time, but when I do a set-based insert, it crashes with the message that I am attempting to insert a duplicate. The computed field has a unique index on it, and the attempt seems to be to insert a null. But a null should not be possible, since the field is simply a concatentaion of cascading texts, and it works when I do the inserts with a cursor,  instead of a set.

     

    Here is the trigger:

    ALTER TRIGGER [dbo].[trTaxonUpOnly]
    ON [dbo].[Taxonomy]
    AFTER INSERT,UPDATE
    AS
    BEGIN
    SET NOCOUNT ON;
    Declare @TaxonAutoID int
    DECLARE CompleteTaxonText_Cursor CURSOR FAST_FORWARD FOR SELECT TaxonAutoID FROM INSERTED Order By TaxonLevelID, TaxonAutoID;

    if exists (Select Ins.TaxonAutoID From inserted Ins Inner Join Taxonomy T On Ins.NextHigherTaxonAutoID = T.TaxonAutoID Where Ins.TaxonLevelID <= T.TaxonLevelID)
    begin
    raiserror ('Taxon parent level >= taxon level - not permitted.', 16, 1)
    rollback transaction
    end

    open CompleteTaxonText_Cursor
    FETCH NEXT FROM CompleteTaxonText_Cursor into @TaxonAutoID

    WHILE @@FETCH_STATUS = 0
    BEGIN
    update T
    Set T.CompleteTaxonText = dbo.fnsTaxonString(CDL.TaxonAutoID)
    from Taxonomy T
    inner join Tax.CurrentAndDownLine(@TaxonAutoID) CDL
    on T.TaxonAutoID = CDL.TaxonAutoID
    where not T.CompleteTaxonText = dbo.fnsTaxonString(CDL.TaxonAutoID)
    FETCH NEXT FROM CompleteTaxonText_Cursor into @TaxonAutoID
    end
    CLOSE CompleteTaxonText_Cursor
    DEALLOCATE CompleteTaxonText_Cursor
    END
  • Not sure that I get this, but if you insert multiple rows at the time, and don't fill something in the CompleteTaxonText column, you have multiple NULL which will validate your index.

    Would this be better as an INSTEAD OF trigger? Wouldn't that circumvent the issue?

    [font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]

  • That's what the trigger is supposed to do - on insert, it fills in the CompleteTaxonText column. And it does, when I insert manually, or loop through the insert recordset with a cursor. It only fails when inserting multiple records at once, from a set-based insert.

  • Yeah, but the trigger is an AFTER trigger. So the trigger does not fail on multi-row inserts. The INSERT statement fails before the trigger is fired. This is why I think you need to write this as an INSTEAD OF trigger.

    [font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]

  • You mean it tries to insert all the rows at once, and only AFTER they are all inserted, does the trigger begin its work?

  • I wonder if maybe it would be more effective to disable the index on this field, do the import and then re-enable the index. Under normal conditions, only one record at a time is ever updated or inserted. These multiple inserts only take place when I (as the developer/DBA) import current data from an old version of the database into a new one. Users never do such things, and I'm only getting stuck on this migration into a new version. Changing the trigger to INSTEAD OF I don't even know how to do - BEFORE INSERT? But disabling and re-enabling an index is simple.

  • You mean it tries to insert all the rows at once, and only AFTER they are all inserted, does the trigger begin its work?

    Yes.

    I can't say whether it is a good idea to disable the index, since I don't know the context. But assumptions like "it is only ..." or "users always update one row at a time", often fails to be wrong over time.

    An INSTEAD OF trigger is a little trickier to code, because you have redo the INSERT or UPDATE operation yourself. It is not a BEFORE trigger - that is, unfortunately, not available in SQL Server.

     

    [font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]

  • Erland Sommarskog wrote:

    You mean it tries to insert all the rows at once, and only AFTER they are all inserted, does the trigger begin its work?

    Yes.

    Ah, thank you. That explains it. I don't have that much experience with triggers. But I'm learning.

     

    I can't say whether it is a good idea to disable the index, since I don't know the context. But assumptions like "it is only ..." or "users always update one row at a time", often fails to be wrong over time.

    No question, that is certainly true enough, but in this case, I think it might be a safe assumption. There is only one application that accesses this database, and I am the sole author of the entire project - application, database, machine configuration, backup procedures - everything. I know what my application does - there is no code to allow multiple inserts, and no reason for anyone to even want to do them in this particular table. It's mostly a reference table - FAR more read activity than anything else, and edits and adds are only possible individually. I'll try the index bit first, since it's simple and I know how to do it.

  • Or re-create the index so that it allows multiple NULLs but no other dups.

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • ScottPletcher wrote:

    Or re-create the index so that it allows multiple NULLs but no other dups.

    Apparently, the morning coffee hasn't kicked in for me yet today... what would the CREATE INDEX statement look like to do that?

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • This is what Scott had in mind:

    CREATE UNIQUE INDEX my_index ON tbl(col) WHERE col IS NOT NULL

    Although, with an INSTEAD OF trigger, the column can be made NOT NULL, which is even prettier.

    [font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]

  • Jeff Moden wrote:

    ScottPletcher wrote:

    Or re-create the index so that it allows multiple NULLs but no other dups.

    Apparently, the morning coffee hasn't kicked in for me yet today... what would the CREATE INDEX statement look like to do that?

    Filtered indexes would work well, I guess.  I haven't personally done that.  Here's how I did it before filtered indexes.  If you run into a critical situation with this, as I did earlier, you figure out how to get around it!

    USE tempdb;

    GO

    CREATE TABLE dbo.some_table (

    id int IDENTITY(1, 1) NOT NULL,

    unique_value_except_nulls int NULL,

    uniquifier_for_nulls AS CASE WHEN unique_value_except_nulls IS NULL THEN id ELSE 0 END,

    CONSTRAINT some_table__UQ_unique_value_except_nulls

    UNIQUE ( unique_value_except_nulls, uniquifier_for_nulls )

    )

    GO

    INSERT INTO dbo.some_table VALUES(1);

    INSERT INTO dbo.some_table VALUES(1); /*OOPS*/

    GO

    INSERT INTO dbo.some_table VALUES(NULL);

    GO 5

    SELECT *

    FROM dbo.some_table

    GO

    DROP TABLE dbo.some_table;

    GO

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • I did the bit of disabling and re-enabling the trigger, but still had problems with the text getting populated correctly. I then discovered a bug in my trigger. The condition clause needs to be:

    where (T.CompleteTaxonText <> dbo.fnsTaxonString(cdl.TaxonAutoID)) or (T.CompleteTaxonText is null)

    The check for null was not in the prior iteration, and naturally, the <> comparison in the first phrase of the where clause didn't handle those. Added the second phrase of null test and all seems to be working properly now.

    Thank you all for the input.

Viewing 13 posts - 1 through 12 (of 12 total)

You must be logged in to reply to this topic. Login to reply