February 13, 2017 at 1:39 pm
bmg002 - Monday, February 13, 2017 1:23 PMkomal145 - Monday, February 13, 2017 1:19 PMkomal145 - Monday, February 13, 2017 1:15 PMbmg002 - Monday, February 13, 2017 1:05 PMkomal145 - Monday, February 13, 2017 1:00 PMbmg002 - Monday, February 13, 2017 12:59 PMkomal145 - Monday, February 13, 2017 12:54 PMbmg002 - Monday, February 13, 2017 12:49 PMkomal145 - Monday, February 13, 2017 12:43 PMbmg002 - Monday, February 13, 2017 10:26 AMThere is also some weirdness with your code. Your:SELECT @code
FROM inserted
line is strange. You don't need a FROM when you are just selecting the variable.But I agree, a Foreign Key Constraint is a much safer way to go and then use a try-catch block in your stored procedure that does the insert.
And as John pointed out, the code:
SELECT @Code = Code from TABLEA AS [M]
INNER JOIN INSERTED I ON M.Code = I.CODE
will fail if inserted contains more than 1 row AND more than 1 code matches.A FK would be much faster for your inserts as well and you could get stuck in an infinite loop depending on your settings.
BUT if the code you provided is what you want, change this line:
INSERT INTO TABLEB (Code)
toINSERT INTO TABLEB (@Code)But to me, this looks like a recipe for disaster.
ALTER TRIGGER Trg_Code ON TABLEB
INSTEAD OF INSERT AS
BEGINDECLARE @CompetitorCode nVarchar(250)
SELECT @CompetitorCode = M.Code From TABLEA AS [M]
INNER JOIN INSERTED I ON M.Code = I.CompetitorType_Code
BEGIN TRY
IF (@Code is Not NULL)
INSERT INTO TABLEB(CompetitorType_Code)
Select @CompetitorCode
END TRY
BEGIN CATCH
IF (@CompetitorCode is NULL)BEGIN
RAISERROR (N'The provided CompetitorTypeCode does not exist in TableB',16, 1);
ROLLBACK TRANSACTION;
RETURN
END
END CATCH
ENDI added try catch block and it does not do what it has to do. Can you please check the code?
I noticed my previous comment was wrong about the INSERT INTO TABLEB(@code).
but your code line:IF (@Code is Not NULL)
Is the problem. It should be @CompetitorCode.But I am still concerned about infinite loops. Do you have "Allow Triggers to Fire Others" set to true? If so, you might get hit by infinite trigger loops.
This inserts the value but not raising any error if the code is incorrect.
OK. So you are making progress.
The raiserror will report an error in the SQL Log. You sure there is no errors apeparing in the SQL Log after raiseerror?yes. No errors appearing.
Try putting the raiseerror after your rollback. Not sure if that'll make a difference, but I'm wondering if it does.
if it makes no difference, can you verify that the competitor code is null (ie print it out)?
I tired ,to print out the @competitorcode before try block( it prints) inside catch....i added print @competitorcode doe snot print anyything
It doesnot print for error ..for correct code it prints code.
The catch block is not workingI just realized why. I am dumb.
It is because the TRY is succeeding. No error is being thrown inside the TRY so the TRY succeeds and the catch never gets called.
Toss your raiseerror inside your TRY as:BEGIN TRY
IF (@Code is Not NULL)
BEGIN
INSERT INTO TABLEB(CompetitorType_Code)
Select @CompetitorCode
END
IF (@CompetitorCode is NULL)
RAISERROR (N'The provided CompetitorTypeCode does not exist in TableB',16, 1);
END TRY
BEGIN CATCH
ROLLBACK TRANSACTION;
RETURN
END CATCH
Don't think you need the TRY CATCH block though... it feels a little messy to me. But I bet that'll give you what you want.
I get this error instead of "custom error" ---"The transaction ended in the trigger. The batch has been aborted."
February 13, 2017 at 1:44 pm
komal145 - Monday, February 13, 2017 1:39 PMbmg002 - Monday, February 13, 2017 1:23 PMkomal145 - Monday, February 13, 2017 1:19 PMkomal145 - Monday, February 13, 2017 1:15 PMbmg002 - Monday, February 13, 2017 1:05 PMkomal145 - Monday, February 13, 2017 1:00 PMbmg002 - Monday, February 13, 2017 12:59 PMkomal145 - Monday, February 13, 2017 12:54 PMbmg002 - Monday, February 13, 2017 12:49 PMkomal145 - Monday, February 13, 2017 12:43 PMbmg002 - Monday, February 13, 2017 10:26 AMThere is also some weirdness with your code. Your:SELECT @code
FROM inserted
line is strange. You don't need a FROM when you are just selecting the variable.But I agree, a Foreign Key Constraint is a much safer way to go and then use a try-catch block in your stored procedure that does the insert.
And as John pointed out, the code:
SELECT @Code = Code from TABLEA AS [M]
INNER JOIN INSERTED I ON M.Code = I.CODE
will fail if inserted contains more than 1 row AND more than 1 code matches.A FK would be much faster for your inserts as well and you could get stuck in an infinite loop depending on your settings.
BUT if the code you provided is what you want, change this line:
INSERT INTO TABLEB (Code)
toINSERT INTO TABLEB (@Code)But to me, this looks like a recipe for disaster.
ALTER TRIGGER Trg_Code ON TABLEB
INSTEAD OF INSERT AS
BEGINDECLARE @CompetitorCode nVarchar(250)
SELECT @CompetitorCode = M.Code From TABLEA AS [M]
INNER JOIN INSERTED I ON M.Code = I.CompetitorType_Code
BEGIN TRY
IF (@Code is Not NULL)
INSERT INTO TABLEB(CompetitorType_Code)
Select @CompetitorCode
END TRY
BEGIN CATCH
IF (@CompetitorCode is NULL)BEGIN
RAISERROR (N'The provided CompetitorTypeCode does not exist in TableB',16, 1);
ROLLBACK TRANSACTION;
RETURN
END
END CATCH
ENDI added try catch block and it does not do what it has to do. Can you please check the code?
I noticed my previous comment was wrong about the INSERT INTO TABLEB(@code).
but your code line:IF (@Code is Not NULL)
Is the problem. It should be @CompetitorCode.But I am still concerned about infinite loops. Do you have "Allow Triggers to Fire Others" set to true? If so, you might get hit by infinite trigger loops.
This inserts the value but not raising any error if the code is incorrect.
OK. So you are making progress.
The raiserror will report an error in the SQL Log. You sure there is no errors apeparing in the SQL Log after raiseerror?yes. No errors appearing.
Try putting the raiseerror after your rollback. Not sure if that'll make a difference, but I'm wondering if it does.
if it makes no difference, can you verify that the competitor code is null (ie print it out)?
I tired ,to print out the @competitorcode before try block( it prints) inside catch....i added print @competitorcode doe snot print anyything
It doesnot print for error ..for correct code it prints code.
The catch block is not workingI just realized why. I am dumb.
It is because the TRY is succeeding. No error is being thrown inside the TRY so the TRY succeeds and the catch never gets called.
Toss your raiseerror inside your TRY as:BEGIN TRY
IF (@Code is Not NULL)
BEGIN
INSERT INTO TABLEB(CompetitorType_Code)
Select @CompetitorCode
END
IF (@CompetitorCode is NULL)
RAISERROR (N'The provided CompetitorTypeCode does not exist in TableB',16, 1);
END TRY
BEGIN CATCH
ROLLBACK TRANSACTION;
RETURN
END CATCH
Don't think you need the TRY CATCH block though... it feels a little messy to me. But I bet that'll give you what you want.I get this error instead of "custom error" ---"The transaction ended in the trigger. The batch has been aborted."
BEGIN CATCH
PRINT ERROR_MESSAGE()
ROLLBACK TRANSACTION;
RETURN
END CATCH
This prints the error. But still , this is check for only 1 code , whta if i need to do checkon multiple codes in same trigger , will my try catch repeats for differenct codeS?
February 13, 2017 at 1:45 pm
IF (@CompetitorCode is NULL)
BEGIN
RAISERROR (N'The provided CompetitorTypeCode does not exist in TableB',16, 1);
ROLLBACK TRANSACTION;
RETURN
END The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
February 13, 2017 at 1:52 pm
komal145 - Monday, February 13, 2017 1:44 PMBEGIN CATCH
PRINT ERROR_MESSAGE()
ROLLBACK TRANSACTION;
RETURN
END CATCHThis prints the error. But still , this is check for only 1 code , whta if i need to do checkon multiple codes in same trigger , will my try catch repeats for differenct codeS?
If you want to run through the code for multiple inserted values, you'll need to step through each row in the inserted table. A cursor is one option. A slow and horrible option, but it is an option. Cursors on triggers are just asking for performance problems though.
Or modify your query to handle multiple values from inserted.
like instead of storing it in a temporary variable and then checking if that is not null, you could do an IF EXISTS. The problem is, what should happen if 5 rows were inserted and only 4 of them matched? Should it insert the 4 rows and ignore the 5th, should it fail the whole insert.
If it was me, I would either use a foreign key (as has been suggested multiple times) OR sanitize the data before it even starts that insert so end users via whatever application they are trying to use don't even have the option to insert invalid values into the table.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
February 13, 2017 at 2:56 pm
This should be a foreign key. Plain and simple. Doing this in a trigger is a bad decision for several reasons. It is more complicated to maintain, it far more difficult to get it right and it is going to be slower than a foreign key. Last but not least, triggers have a habit of getting forgotten about which can cause other problems down the road. Do this with a foreign key and you need one statement and there is nothing else required. A trigger is just the wrong tool for this task.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
February 14, 2017 at 12:59 pm
If you're not willing to use a Foreign Key Constraint then insert your data into a temp table and use a stored procedure to do your data validation. I would include data validation in the business logic category. Triggers are no place for business logic. I hate triggers and avoid them at all cost. Sometimes a trigger make sense but in this case I would say avoid it.
Viewing 6 posts - 16 through 20 (of 20 total)
You must be logged in to reply to this topic. Login to reply