Page 1 of 2 12 LastLast
Results 1 to 15 of 17
  1. #1
    Join Date
    Jun 2005
    Posts
    79

    Unanswered: Feedback on trigger.

    Hi,

    I have an orderline table that is a child of one of 3 tables (quote, order, project). The orderline table has 3 nullable foreign keys where only 1 can be filled and a orderlinetypeid column that specified which table is the parent.

    I want to make a trigger that when the Quantity field changes or when an orderline is inserted or deleted, the trigger will update the TotalQuantity field for one of the parent tables.

    Please tell me if I can improve something.

    Thanks

    Code:
    CREATE TRIGGER [tOrderLineQuantity] ON OrderLine
    FOR INSERT, UPDATE, DELETE 
    AS
    DECLARE @OrderLineTypeID int
     
    IF UPDATE(Quantity)
    BEGIN 
    IF (SELECT COUNT(*) FROM inserted) > 0 
        BEGIN  
            -- update
            SELECT @OrderLineTypeID = (SELECT OrderLineTypeID FROM inserted)
    
    	IF (@OrderLineTypeID = 1)
    		BEGIN
    			EXEC cpUpdateQuoteTotalQuantity SELECT QuoteID FROM inserted
    		END
    	ELSE IF (@OrderLineTypeID = 2)
    		BEGIN
    			EXEC cpUpdateOrderTotalQuantity SELECT OrderID FROM inserted
    		END
    	ELSE
    		BEGIN
    			EXEC cpUpdateProjectTotalQuantity SELECT ProjectID FROM inserted
    		END
        END 
        ELSE 
        BEGIN 
            -- delete
    	SELECT @OrderLineTypeID = (SELECT OrderLineTypeID FROM deleted)        
    
    	IF (@OrderLineTypeID = 1)
    		BEGIN
    			EXEC cpUpdateQuoteTotalQuantity SELECT QuoteID FROM inserted
    		END
    	ELSE IF (@OrderLineTypeID = 2)
    		BEGIN
    			EXEC cpUpdateOrderTotalQuantity SELECT OrderID FROM inserted
    		END
    	ELSE
    		BEGIN
    			EXEC cpUpdateProjectTotalQuantity SELECT ProjectID FROM inserted
    		END
        END 
    END
    
    
    
    -- one of the procedures I call from the trigger
    CREATE PROCEDURE cp_UpdateQuoteTotalQuantity
    	@QuoteID int
    AS
    BEGIN
        UPDATE Quote
    	   SET TotalQuantity = (SELECT SUM(Quantity) FROM OrderLine WHERE QuoteID = @QuoteID)
    	 WHERE Quote.QuoteID = @QuoteID
    END

  2. #2
    Join Date
    Dec 2007
    Location
    London, UK
    Posts
    741
    I don't think you even tested it did you? Among other problems it will fail with an error whenever more than one row is updated.

    The main problem isn't your trigger, it's your table design. There is no obvious reason to have the TotalQuantity column at all. Drop it unless you have a very good reason not to. Three nullable foreign keys in one table also rings alarm bells for me. You should consider refactoring that as a supertype / subtype design, with separate tables for each of the three cases.

  3. #3
    Join Date
    Jun 2005
    Posts
    79
    Hi,

    Thank you for your input. I have not gotten a change to check my code, I will be doing that later but I just wanted to throw what I want to accomplish. How do I solve the multiple update problem you mentioned? During a multi-row update/insert, the line type and id would be identical, so the update/insert would only contain either quote lines, order lines or project lines.

    As for the table design, it was the best solution to our problem.

  4. #4
    Join Date
    Jun 2003
    Location
    Ohio
    Posts
    12,592
    Provided Answers: 1
    So an OrderLine can be associated with either a quote, an order, or a project. But is may only be associated with one of the three?
    Since you have an orderlinetypeid column, you don't need three foreign keys. You can make due with just one as long as all the keys are the same datatype. If you are using surrogates, that should not be a problem.
    If it's not practically useful, then it's practically useless.

    blindman
    www.chess.com: "sqlblindman"
    www.LobsterShot.blogspot.com

  5. #5
    Join Date
    Jun 2005
    Posts
    79
    The 3 separate keys are a requirement for code generation and must be there.
    How do I solve the multiple row update error?

    Thanks

  6. #6
    Join Date
    Nov 2004
    Posts
    1,427
    Provided Answers: 4
    I'm not pleased with the update scripts. Who can do better?
    Code:
    CREATE TABLE Quote(
    QuoteId		int	IDENTITY (1, 1) NOT NULL,
    bla		CHAR(5)		NOT NULL,
    TotalQuantity	int	NOT NULL default 0,
    CONSTRAINT PK_quote PRIMARY KEY (QuoteId)
    )
    
    CREATE TABLE Order_(
    OrderId		int	IDENTITY (1, 1) NOT NULL,
    bla		CHAR(5)		NOT NULL,
    TotalQuantity	int	NOT NULL default 0,
    CONSTRAINT PK_order PRIMARY KEY (OrderId)
    )
    
    CREATE TABLE Project(
    ProjectId		int	IDENTITY (1, 1) NOT NULL,
    bla		CHAR(5)		NOT NULL,
    TotalQuantity	int	NOT NULL default 0,
    CONSTRAINT PK_project PRIMARY KEY (ProjectId)
    )
    
    CREATE TABLE OrderLine(
    Id			int	IDENTITY (1, 1) NOT NULL,
    OrderLineTypeId		INT	NOT NULL 
    	CONSTRAINT c_OrderLineTypeId CHECK (OrderLineTypeID BETWEEN 1 AND 3),
    QuoteId			int,
    OrderId			int, 
    ProjectId		int, 
    Quantity		int	NOT NULL,
    CONSTRAINT Q_XOR_O_XOR_P CHECK 
    	((OrderLineTypeID = 1 AND QuoteId IS NOT NULL AND OrderId IS NULL AND ProjectId IS NULL) OR
    	(OrderLineTypeID = 2 AND QuoteId IS NULL AND OrderId IS NOT NULL AND ProjectId IS NULL) OR
    	(OrderLineTypeID = 3 AND QuoteId IS NULL AND OrderId IS NULL AND ProjectId IS NOT NULL)),
    CONSTRAINT PK_OrderLine PRIMARY KEY (Id)
    )
    
    
    CREATE TRIGGER [tIUOrderLineQuantity] ON OrderLine
    AFTER INSERT, UPDATE
    AS
     
    --IF UPDATE(Quantity)
    BEGIN 
        -- update
        UPDATE Quote
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM Inserted AS I
    							WHERE OL.QuoteId = I.QuoteId AND
    								Quote.QuoteId = OL.QuoteId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM Inserted 
    			WHERE Quote.QuoteId = Inserted.QuoteId)
        
        UPDATE Order_
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM Inserted AS I
    							WHERE OL.OrderId = I.OrderId AND
    								Order_.OrderId = OL.OrderId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM Inserted 
    			WHERE Order_.OrderId = Inserted.OrderId)
        
        UPDATE Project
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM Inserted AS I
    							WHERE OL.ProjectId = I.ProjectId AND
    								Project.ProjectId = OL.ProjectId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM Inserted 
    			WHERE Project.ProjectId = Inserted.ProjectId)
        
    END
    
    
    CREATE TRIGGER [tDOrderLineQuantity] ON OrderLine
    AFTER DELETE 
    AS
    BEGIN
        UPDATE Quote
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM deleted AS D
    							WHERE OL.QuoteId = D.QuoteId AND
    								Quote.QuoteId = OL.QuoteId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM deleted 
    			WHERE Quote.QuoteId = deleted.QuoteId)
        
        UPDATE Order_
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM deleted AS D
    							WHERE OL.OrderId = D.OrderId AND
    								Order_.OrderId = OL.OrderId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM deleted 
    			WHERE Order_.OrderId = deleted.OrderId)
        
        UPDATE Project
    	SET TotalQuantity = COALESCE((SELECT SUM(OL.Quantity) 
    					FROM OrderLine AS OL
    					WHERE EXISTS (SELECT 1 
    							FROM deleted AS D
    							WHERE OL.ProjectId = D.ProjectId AND
    							Project.ProjectId = OL.ProjectId)
    								), 0)
    	WHERE EXISTS (SELECT 1 
    			FROM deleted 
    			WHERE Project.ProjectId = deleted.ProjectId)
    END
    
    INSERT INTO Quote (bla) values('Hi,')
    INSERT INTO Order_ (bla) values('Hey')
    INSERT INTO Project (bla) values('Hello')
    
    INSERT INTO OrderLine (OrderLineTypeID, QuoteId, Quantity) values(1, 1, 100)
    INSERT INTO OrderLine (OrderLineTypeID, QuoteId, Quantity) values(1, 1, 101)
    
    INSERT INTO OrderLine (OrderLineTypeID, OrderId, Quantity) values(2, 1, 200)
    INSERT INTO OrderLine (OrderLineTypeID, OrderId, Quantity) values(2, 1, 202)
    
    INSERT INTO OrderLine (OrderLineTypeID, ProjectId, Quantity) 
    	SELECT 3, 1, (Quantity * 3) 
    	from OrderLine 
    	WHERE OrderLineTypeID = 1
    select * from Quote;
    select * from Order_;
    select * from Project;
    select * from OrderLine;
    
    UPDATE OrderLine SET Quantity = 99 WHERE id = 1
    select * from Quote;
    select * from Order_;
    select * from Project;
    select * from OrderLine;
    
    UPDATE OrderLine SET Quantity = Quantity * 2
    select * from Quote;
    select * from Order_;
    select * from Project;
    select * from OrderLine;
    
    DELETE OrderLine WHERE id < 3
    select * from Quote;
    select * from Order_;
    select * from Project;
    select * from OrderLine;
    
    
    DROP TRIGGER tIUOrderLineQuantity
    DROP TRIGGER tDOrderLineQuantity
    
    DROP TABLE OrderLine
    DROP TABLE Quote
    DROP TABLE Order_
    DROP TABLE Project
    With kind regards . . . . . SQL Server 2000/2005/2012
    Wim

    Grabel's Law: 2 is not equal to 3 -- not even for very large values of 2.
    Pat Phelan's Law: 2 very definitely CAN equal 3 -- in at least two programming languages

  7. #7
    Join Date
    Jun 2003
    Location
    Ohio
    Posts
    12,592
    Provided Answers: 1
    Quote Originally Posted by PolarBear2k
    The 3 separate keys are a requirement for code generation and must be there.
    How do I solve the multiple row update error?

    Thanks
    God, I love crap like this.
    Tools are supposed to assist in development, not dictate design.
    You are allowing your tools (ORM, most likely), to restrict you to a bad database design.
    It's akin to a home builder deciding to use only nails, because his workers have hammers but no screwdrivers.
    Really stupid and short-sited.
    If it's not practically useful, then it's practically useless.

    blindman
    www.chess.com: "sqlblindman"
    www.LobsterShot.blogspot.com

  8. #8
    Join Date
    Jun 2005
    Posts
    79
    Thank you Wim for that great response.
    I have read somewhere that it is a good idea to keep update code out of triggers and use stored procedures instead, while using triggers to enforce business rules. Do you agree with this opinion?

  9. #9
    Join Date
    Dec 2007
    Location
    London, UK
    Posts
    741
    Quote Originally Posted by PolarBear2k
    I have read somewhere that it is a good idea to keep update code out of triggers and use stored procedures instead, while using triggers to enforce business rules. Do you agree with this opinion?
    I do, yes.

    I also agree with Blindman. If you are using the wrong tools then get different tools, don't botch the model to suit. A data model should serve its users and the business that pays for it, not the convenience of the people who develop it.

  10. #10
    Join Date
    Jun 2005
    Posts
    79
    Quote Originally Posted by blindman
    God, I love crap like this.
    Tools are supposed to assist in development, not dictate design.
    You are allowing your tools (ORM, most likely), to restrict you to a bad database design.
    It's akin to a home builder deciding to use only nails, because his workers have hammers but no screwdrivers.
    Really stupid and short-sited.
    I totally agree with you. Unfortunately, sometimes sacrifices have to be made to reap certain benefits. Code generation creates thousands of lines of code and that has greater benefit for us in the long run than a couple of bad design decisions. Since this is a in-house project that is strictly coupled with the business model and software in use, those bad design decisions are well known and understood.

  11. #11
    Join Date
    Jun 2003
    Location
    Ohio
    Posts
    12,592
    Provided Answers: 1
    Quote Originally Posted by PolarBear2k
    Code generation creates thousands of lines of code and that has greater benefit for us in the long run than a couple of bad design decisions. Since this is a in-house project that is strictly coupled with the business model and software in use, those bad design decisions are well known and understood.
    I've seen it a hundred times, and you are wrong. You are merely trading short-term short-cuts for long-term administrative overhead, performance degradation, and functional limitations on your application.
    As a consultant, 3/4 of the projects I work on are work-arounds on bad database designs.
    If it's not practically useful, then it's practically useless.

    blindman
    www.chess.com: "sqlblindman"
    www.LobsterShot.blogspot.com

  12. #12
    Join Date
    Jun 2005
    Posts
    79
    Don't get me wrong, I may sound like I am justifying the bad design decisions in our situation, but I am really not. I'm just presenting the reasoning in the environment I work in. Most of the time you have to work with what you got. Telling the boss that you need 20 hours to re-design something that works fine (according to them) and an extra few hours after each code generation because its not done properly is not gonna fly. I lost count how many times I've been told when gathering requirements to handle 99.9% of the situations while knowing that that .1% will make the whole thing crash and burn. The problem is obviously the people in power that don't understand certain things regardless of how many times you try to drill it into their heads. The best I can do is, do what I was hired to do in the best way I can and move on hoping that the people I work with next time will be more informed.

    I knew, I was gonna open a can of worms mentioning the reason for the 3 foreign keys. lol

  13. #13
    Join Date
    Nov 2004
    Posts
    1,427
    Provided Answers: 4
    I would never use such a trigger myself. Keeping redundant data (TotalQuantity) in sync with the rest of the system needs a very very very good reason.

    Check if calculating TotalQuantity at runtime is really a show stopper. Perhaps using a view that does just this, can solve your problem in a much more healthy manner. Or perhaps your reporting tool can do the calculation on the client when rendering the report.

    I am studying SQL Server triggers right now. That's why I improved your trigger, not because I think using a trigger here is the best solution. Denormalisation is something I will only do if there is clear evidence that it is the only solution to a performance problem.

    I have read somewhere that it is a good idea to keep update code out of triggers and use stored procedures instead, while using triggers to enforce business rules. Do you agree with this opinion?
    I agree to some extend. I only didn't know how to deal with the "Inserted" and "Deleted" tables together with a SP. Perhaps they can be passed as arguments to the SP ? Studying SP's in SQL-Server is still on my to-do list
    With kind regards . . . . . SQL Server 2000/2005/2012
    Wim

    Grabel's Law: 2 is not equal to 3 -- not even for very large values of 2.
    Pat Phelan's Law: 2 very definitely CAN equal 3 -- in at least two programming languages

  14. #14
    Join Date
    Jun 2005
    Posts
    79
    There is a lot of calculation done on that column and that was the reason why it was there. But, after looking at all the scenarios where it will be used, all of the OrderLines are already loaded so having this column in the parent is not necessary, so I'm gonna remove it.

    I also started to get into triggers and stored procedures to remove the need to do some things manually in code. Now I'm writing triggers mostly to fire stored procedures when certain columns are updated.

    Thanks for your help.

  15. #15
    Join Date
    Jun 2003
    Location
    Ohio
    Posts
    12,592
    Provided Answers: 1
    Quote Originally Posted by PolarBear2k
    I'm just presenting the reasoning in the environment I work in.
    I hear you, Brother.
    If it's not practically useful, then it's practically useless.

    blindman
    www.chess.com: "sqlblindman"
    www.LobsterShot.blogspot.com

Posting Permissions

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