1

Let me explain my situation. I have 3 tables generated with

CREATE TABLE Partners 
( 
    id INT IDENTITY(1,1),
    name NVARCHAR(50) NOT NULL,
    PRIMARY KEY (id)
);

-- snip ... 
CREATE TABLE Questions 
(
    id INT IDENTITY(1,1),
    section_id INT,
    qtext NVARCHAR(300) NOT NULL,
    PRIMARY KEY (id),
    FOREIGN KEY (section_id) REFERENCES Sections(id) ON DELETE CASCADE
);

-- snip ... 
CREATE TABLE Answers 
(
    id INT IDENTITY (1,1),
    question_id INT,
    partner_id INT,
    val DECIMAL DEFAULT 0.0,
    PRIMARY KEY (id),
    FOREIGN KEY (question_id) REFERENCES Questions(id) ON DELETE CASCADE,
    FOREIGN KEY (partner_id) REFERENCES Partners(id) ON DELETE CASCADE
);

and I'm trying to set up a trigger so that when a new partner is added, default answers get for him get generated for every question.

My attempt at creating that trigger is

-- Create trigger so that adding a partner results in default
-- answers for every survey
-- See https://stackoverflow.com/questions/11852782/t-sql-loop-over-query-results
CREATE TRIGGER DefaultAnswers     
    ON  Partners       
    AFTER INSERT     
AS       
BEGIN    
    CREATE TABLE QuestIds (RowNum INT, Id INT);

    INSERT INTO QuestIds (RowNum, Id)
        SELECT DISTINCT ROW_NUMBER() OVER (ORDER BY Id) as RowNum, Id
        FROM TABLE

    DECLARE @id INT
    DECLARE @totalrows INT = (SELECT COUNT(*) FROM QuestIds)
    DECLARE @currentrow INT = 1

    WHILE @currentrow <  @totalrows  
    BEGIN 
        SET @id = (SELECT Id FROM QuestIds WHERE RowNum = @currentrow)

        EXEC AddAnswerWithoutVal @question_id=@id, @partner_id=INSERTED.id

        SET @currentrow = @currentrow +1
    END 
END

and the errors are

Msg 156, Level 15, State 1, Procedure DefaultAnswers, Line 313
Incorrect syntax near the keyword 'TABLE'.

Msg 102, Level 15, State 1, Procedure DefaultAnswers, Line 323
Incorrect syntax near '.'.

Can you help me figure out the problem? INSERTED, I thought, refers to the row that was just inserted in the table. As for the FROM TABLE, that too was something I tried to rip off of T-SQL loop over query results.

EDIT: I have another question. When I get this working, will the trigger refer to a row that was successfully inserted? I want to make sure this is an atomic operation.

Community
  • 1
  • 1
user5648283
  • 5,913
  • 4
  • 22
  • 32
  • 3
    `inserted` is a table. You would need to use a cursor or other looping construct to use a value from `inserted` for a stored procedure. – Gordon Linoff Feb 01 '16 at 15:32
  • After insert triggers are run only after the row has been successfully inserted. This is the answer to your second question. – Gordon Linoff Feb 01 '16 at 15:41

3 Answers3

4

The use of a stored procedure to insert rows seems like a bad idea. Perhaps it is necessary for some reason, but it would be easier to just express this as a single insert. Something like this:

insert into Answers(question_id, partner_id, val)
    select q.id, i.id, NULL
    from inserted i cross join
         questions q;

This would replace the entire body and there is no need for a temporary table. Creating a table inside a trigger seems like a bad idea.

Gordon Linoff
  • 1,242,037
  • 58
  • 646
  • 786
  • Oops, I forgot to mention that I already have a sproc that adds answers with default values: `CREATE PROCEDURE AddAnswerWithoutVal @question_id INT, @partner_id INT AS INSERT INTO Answers (question_id, partner_id) VALUES (@question_id, @partner_id)` – user5648283 Feb 01 '16 at 15:46
  • I'm getting `Invalid object name 'inserted'. Msg 208, Level 16, State 1, Procedure AddAnswerWithVal, Line 137 Invalid object name 'inserted'.` after trying your suggestion. ANy idea why that may be? – user5648283 Feb 01 '16 at 16:09
  • @user5648283 . . . This code goes inside a trigger, not a stored procedure. As I mention in the answer, a stored procedure to insert values just makes the whole effort a lot more complicated than it needs to be. – Gordon Linoff Feb 02 '16 at 00:42
1

The reason for the syntax error is here:

    INSERT INTO QuestIds (RowNum, Id)
    SELECT DISTINCT ROW_NUMBER() OVER (ORDER BY Id) as RowNum, Id
    FROM TABLE

You need to put the name of the table in the FROM clause, not the keyword TABLE.

But do you really want to be creating this table every time the trigger is fired? What happens the second time it fires and the table already exists?

And while I'm thinking of it, this:

I'm trying to set up a trigger so that when a new partner is added, default answers get for him get generated for every question.

Sounds like a sub-optimal solution in the first place. If default answers are defined for every question, why not just assume that, whenever a partner is missing an answer for a question, his answer is the default answer. Then just don't add any answers for the new partner, until they are specifically added later.

That's the alternative approach I would explore: no trigger at all.

Tab Alleman
  • 31,483
  • 7
  • 36
  • 52
  • I'm open to alternative ways of doing this. As you can probably tell, I'm a n00b to database programming, but I'm trying my best to learn – user5648283 Feb 01 '16 at 15:38
1

OK first, You NEVER and I mean NEVER want to loop in a trigger. Triggers that loop can cause horrible performance problems. You have to assume that multiple records might be inserted or deleted and never assume only one will be. The trigger has to accommodate both.

Yes the inserted table contains the records that were just inserted. You would then process them the same as if they were in any any other table. What you wrote would not work with any ordinary table either. You can't reference a table in an EXEC statement.

The first thing you need to do is rewrite that proc so that it processes based on sets of data not individual records. Again, it is a very bad thing to ever try to loop through records. In fact I would not do this through another stored proc at all but put all the logic in the trigger unless there are other processes that use the proc. But the logic in any case needs to be set -based and not row by row or one records at a time.

HLGEM
  • 94,695
  • 15
  • 113
  • 186