Reputation: 359
MS SQL Server: Trigger fails because stored procedure fails
I have an app which sends up to 20 device barcodes data into SQL-Server table "device usage".
In an AFTER INSERT trigger of this table I have a stored procedure which transfers the barcodes from columns BC1, BC2, BC3, etc. into another table BC usage.
Table BC usage is set to IGNORE_DUP_KEY = ON as it may happen that BCs are scanned twice.
If a duplicate BC is scanned then also the insert in the first table "device usage" fails!
WHY?
I do not understand why the initial insert into table "device usage" fails although the IGNORE_DUP_KEY = ON in the table BC usage which is only used in the stored procedure.
ALTER TRIGGER [dbo].[UpdatePersonanenDaten]
ON [dbo].[ScanIT_tblGeraeteerfassung]
AFTER INSERT
AS
BEGIN
DECLARE @Personalnummer varchar(10)
DECLARE @Mitarbeiter varchar(100)
DECLARE @InOut char(1)=''
DECLARE @ID int
DECLARE @StartID int
SELECT
@Personalnummer = a.[Personalnummer],
@Mitarbeiter = a.[DeviceUser]
FROM
[dbo].[ScanIT_tblDevices] a
INNER JOIN
inserted i ON a.[DeviceID] = i.[DeviceID]
SELECT @ID = (SELECT ID FROM inserted)
BEGIN
UPDATE [dbo].[ScanIT_tblGeraeteerfassung]
SET [Mitarbeiter] = @Mitarbeiter,
[Personalnummer] = @Personalnummer,
[InOut] = CASE
WHEN t.[AufAbbau] = 'Aufbau (Start)'
THEN 'S'
ELSE 'E'
END
FROM ScanIT_tblGeraeteerfassung t
INNER JOIN inserted i ON t.ID = i.ID
END
-- IN THIS SP THE INSERT INTO tbl "BC USAGE" HAPPENS and the Dup Key error let the trigger fail!!
EXEC [dbo].[ScanIT_spGeraeteEinsatz1] @ID
Thanks for any help
Michael
Edit:
this is the stored procedure ScanIT_spGeraeteEinsatz1 I tried to simplyfy the descsription of the whole job before, but in real the BC sent to the server table is followed by the counter reading of the device. So it needs to lookup the last counter reading of the record for this BC before. S stands for Start, E stands for End of the time the device is used. It also may happed, that this device was not in use before at customers side, then we take the counter from the stocktaking to set the initial counter at Start.
And now, if someone scans this device BC twice it has to be blocked, because the FIRST occurrence of this BC is mandatory the Start and the next occurrence has to be the End. Even if someone adds the BC in a Start record by mistake although the device has already been scanned before in another record.
I hope I could explain the job that you understand the whole process.
ALTER PROCEDURE [dbo].[ScanIT_spGeraeteEinsatz1]
@GeraeteerfassungID int
AS
BEGIN
BEGIN TRANSACTION; --neu
SAVE TRANSACTION MySavePoint;
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
declare @tablename as varchar(255) -- tbl, aus der ausglesen wird: 'ScanIT_tblGeraeteerfassung'
declare @temptable table (BarcodeName varchar(100)) -- im @temptable werden die Feldnamen Geraetebarcode1, Zaehlerstand1, ... eingelesen
declare @sqlDynamicString nvarchar(400)
declare @sqlDynamicString2 nvarchar(400)
declare @NumberBC int, @Counter int
declare @temptable2 table (Barcode varchar(10), Zaehlerstand varchar(20))
declare @strCounter varchar(2)
declare @Geraetebarcode int
declare @ID int
declare @L_ID int
declare @L_InOut char(1)
declare @StartID int
declare @GeraeteerfassungIDOUT int
--declare @Einsatzvon datetime
Declare @StartZaehlerstand varchar(25)
Declare @StartDatum datetime
declare @BC_AnzahlVerwendungen int=0
DECLARE @LastDS TABLE
([ID] [int],
[GeraeteerfassungIDOUT] [int] ,
[GeraeteerfassungIDIN] [int] ,
[Projektnummer] [varchar](20) ,
[Geraetebarcode] [varchar](100) ,
[ZaehlerstandOUT] [varchar](50) ,
[ZaehlerstandIN] [varchar](50) ,
[Einsatzvon] [datetime] ,
[Einsatzbis] [datetime] ,
[InOut] [char](1) ,
[PaarID] [int] )
set @tablename = 'ScanIT_tblGeraeteerfassung'
set @Counter=1
-- Einsatzvon-Datum/Zeit un d Projektnummer bleiben für alle Datensätze gleich
--set @Projektnummer=(Select Arbeitsauftrag from [dbo].[ScanIT_tblGeraeteerfassung] where [ID]=@GeraeteerfassungsID);
--set @Einsatzvon=(Select Sendtime from [dbo].[ScanIT_tblGeraeteerfassung] where [ID]=@GeraeteerfassungsID);
--set @L_InOut=(select [InOut] from [dbo].[ScanIT_tblGeraeteerfassung] where [ID]=@GeraeteerfassungsID))
--select @Projektnummer=Arbeitsauftrag, @Einsatzvon=Sendtime, @L_InOut=InOut from [dbo].[ScanIT_tblGeraeteerfassung] where [ID]=@GeraeteerfassungsID
--Insert into @temptable (BarcodeName)
--wie viele Gerätefelder gibt es in dem SCAN-IT Formular bzw. in der Einlesetabelle?
set @NumberBC=(SELECT count(c.name) FROM sys.columns c WHERE c.object_id = OBJECT_ID(@tablename) and c.name like 'Geraetebarcode%' ) ;
-- hier beginnt der Loop NEU
while(@Counter <= @NumberBC)
begin
set @strCounter=convert(varchar(2),@Counter)
set @sqlDynamicString2 = 'SET @GeraeteBC=(SELECT Geraetebarcode' + @strCounter + ' FROM [dbo].[ScanIT_tblGeraeteerfassung] WHERE ID=' + convert(nvarchar(8),@GeraeteerfassungsID) + ' AND Len(Geraetebarcode' + @strCounter + ')>0)'
EXECUTE sp_executesql @sqlDynamicString2, N'@GeraeteBC nvarchar(8) OUTPUT',@GeraeteBC=@Geraetebarcode OUTPUT
--zuerst muß einmal geschaut werden, ob dieses Gerät überhaupt schon mal im Einsatz war, daher wird die [dbo].[ScanIT_tblGeraeteeinsatz] ausgezählt
--bei ERSTMALIGER Verwendung ist die Anzahl 0
Set @BC_AnzahlVerwendungen=(Select Count(*) from [dbo].[ScanIT_tblGeraeteeinsatz] where [Geraetebarcode]=@Geraetebarcode)
print @Geraetebarcode
print @BC_AnzahlVerwendungen
-- es wird vorerst der Zählerstand, die Sendtime und die ID für in/out eingegeben, weil erst danach bewertet wird, ob das ein IN oder OUT Datensatz ist
set @sqlDynamicString=concat('SELECT ID, ID ,', @Projektnummer, ', Geraetebarcode' + @strCounter, ', Zaehlerstand' + @strCounter, ', Zaehlerstand' + @strCounter, ', Sendtime',', Sendtime',', InOut',
' FROM [dbo].[ScanIT_tblGeraeteerfassung] WHERE ID= ', @GeraeteerfassungsID, ' AND Len(Geraetebarcode' + @strCounter, ')>0' )
print @sqlDynamicString
BEGIN TRY --neu
Insert into [dbo].[ScanIT_tblGeraeteeinsatz] ([GeraeteerfassungIDOUT],
[GeraeteerfassungIDIN],
[Projektnummer],
[Geraetebarcode],
[ZaehlerstandOUT],
[ZaehlerstandIN],
[Einsatzvon],
[Einsatzbis],
[InOut])
exec (@sqlDynamicString)
COMMIT TRANSACTION
END TRY
BEGIN CATCH
IF @@TRANCOUNT > 0
BEGIN
ROLLBACK TRANSACTION MySavePoint; -- rollback to MySavePoint
END
END CATCH
--die ID des eben eingegebenen Datensatzes
set @ID=SCOPE_IDENTITY()
-- nun wird geschaut, ob der Barcode schon einmal vokam
-- Gerät war noch NIE im Einsatz, d.h. den Startzählerwert aus der [dbo].[Lager_tblInventarArtikel] suchen
if @BC_AnzahlVerwendungen = 0
begin
Update a set --a.InOut= 'S',
a.[ZaehlerstandOUT]=coalesce(a.[ZaehlerstandOUT],b.[Anfangsstand]), --zuerste wird geschaut, ob ein Zählerstand angliefert wird, wenn nicht wird im Lagerstand geschaut
a.[ZaehlerstandIN]='',
a.[Einsatzbis]=NULL,
a.[GeraeteerfassungIDIN]= NULL,
a.[PaarID]=a.ID
from [dbo].[ScanIT_tblGeraeteeinsatz] a
left join [dbo].[Lager_tblInventarArtikel] b on a.[Geraetebarcode]=b.[Barcode]
where a.ID=@ID
end
--Gerät war schon im Einsatz, daher suchen aus [dbo].[ScanIT_tblGeraeteeinsatz]
if @BC_AnzahlVerwendungen > 0
begin
--print @inout
--ID des Startdatensatzes suchen
print @Projektnummer
print @Einsatzvon
print @L_InOut
--set @L_InOut=(select [InOut] from @LastDS)
--print @L_InOut
if @L_InOut='S'
--zuerst den zuvor gelieferten Datensatz suchen, um den Anfangstand zu setzen
begin
--Befüllen der Table Variablen mit dem vorletzten Datensatz
INSERT INTO @LastDS
([ID] ,
[GeraeteerfassungIDOUT] ,
[GeraeteerfassungIDIN] ,
[Projektnummer] ,
[Geraetebarcode] ,
[ZaehlerstandOUT] ,
[ZaehlerstandIN] ,
[Einsatzvon] ,
[Einsatzbis] ,
[InOut] ,
[PaarID] )
SELECT Top 1 [ID] ,
[GeraeteerfassungIDOUT] ,
[GeraeteerfassungIDIN] ,
[Projektnummer] ,
[Geraetebarcode] ,
[ZaehlerstandOUT] ,
[ZaehlerstandIN] ,
[Einsatzvon] ,
[Einsatzbis] ,
[InOut] ,
[PaarID]
from [dbo].[ScanIT_tblGeraeteeinsatz]
where [Geraetebarcode]=@Geraetebarcode and ID < @ID order by ID desc
end
begin
Update a set
a.[GeraeteerfassungIDIN]=NULL --weil GeraeteerfassungIDOUT existiert, braucht man sie nicht setzen
,a.[ZaehlerstandOut]= coalesce(a.[ZaehlerstandOut],x.[ZaehlerstandOUT]) --wenn ZaehlerstandOut abgelesen wurde, braucht man ihn nicht setzen, wenn nicht, dann aus letztem DS verwenden
,a.[ZaehlerstandIN]=''
-- ,a.[Einsatzvon]=x.[Einsatzvon] --weil Einsatzvon existiert, braucht man sie nicht setzen
,a.[Einsatzbis]= null
,a.[PaarID]=a.ID
from [dbo].[ScanIT_tblGeraeteeinsatz] a
left join @LastDS x on a.[Geraetebarcode]=x.[Geraetebarcode]
where a.[ID]=@ID
end
if @L_InOut='E'
--hier muß man nach dem letzten DS suchen, weil es einen S geben muß!!
--da diese Nummer schon verwendet wurde, kann man nach dem S Datensatz zu diesem Gerät suchen
begin
--Befüllen der Table Variablen mit dem vorletzten Datensatz
INSERT INTO @LastDS
([ID] ,
[GeraeteerfassungIDOUT] ,
[GeraeteerfassungIDIN] ,
[Projektnummer] ,
[Geraetebarcode] ,
[ZaehlerstandOUT] ,
[ZaehlerstandIN] ,
[Einsatzvon] ,
[Einsatzbis] ,
[InOut] ,
[PaarID] )
SELECT Top 1 [ID] ,
[GeraeteerfassungIDOUT] ,
[GeraeteerfassungIDIN] ,
[Projektnummer] ,
[Geraetebarcode] ,
[ZaehlerstandOUT] ,
[ZaehlerstandIN] ,
[Einsatzvon] ,
[Einsatzbis] ,
[InOut] ,
[PaarID]
from [dbo].[ScanIT_tblGeraeteeinsatz]
where [Geraetebarcode]=@Geraetebarcode and ID < @ID order by ID desc
end
select * from @LastDS
begin
Update a set
a.[GeraeteerfassungIDIN]=x.[GeraeteerfassungIDOUT]
,a.[ZaehlerstandOut]= x.[ZaehlerstandOut]
--,a.[ZaehlerstandIN]= NULL --steht schon drinnen
,a.[Einsatzvon]=x.[Einsatzvon]
--,a.[Einsatzbis]= NULL --steht schon drinnen
,a.[PaarID]=x.ID
,a.[Verbrauch]= coalesce(cast(a.[ZaehlerstandIN] as float),0)-coalesce(cast(x.[ZaehlerstandOUT] as float),0)
,a.[Standzeit]= datediff(d, coalesce(x.[Einsatzvon],0), @Einsatzvon)+1
from [dbo].[ScanIT_tblGeraeteeinsatz] a
left join @LastDS x on a.[Geraetebarcode]=x.[Geraetebarcode]
where a.[ID]=@ID
end
end
-- zuletzt @Counter hochzählen, dann loop
set @Geraetebarcode=null
set @BC_AnzahlVerwendungen=null
delete from @LastDS --tablevariable leeren
set @Counter=@Counter+1
end
-- Select @StartZaehlerstand, @StartDatum, @StartID,@InOut, @ID, @Counter
END
Upvotes: 0
Views: 418
Answers (1)
Reputation: 1836
TL;DR: the obvious question is: why are you even trying to insert duplicate key values ? You should address the root of the problem instead of looking for workarounds that do not solve anything and create more problems.
If is not hard to check whether a record already exists in a table and skip insertion. To put it differently, you should strive for quality of the data at the source, not clean up and sort duplicates after.
Short answer: there is no easy fix for your problem, because you have a broad range of issues and we dot not have a copy of your data and environment to provide in-depth advice. Long answer: see below for some suggestions.
Without having a full insight to your application I nonetheless have the impression that the approach is bloated and unnecessary. Transferring records to another table is duplication, waste of data storage and unreliable if your trigger is not well-coded.
Why not build a view (or a stored procedure) that points to the original table ? I don't even think you should try to fix this code, simplify your code instead. Reconsider your data model and the flow of operations.
Your procedures are already complicated enough, you even have dynamic SQL that as far as I can tell is not justified. Very seldom is it justified, because it adds a layer of complication and can be a security risk.
When you make things more complicated than they should be, the code becomes hard to read, less reliable and difficult to debug. There are so many hooplas to perform basic operations that should be relatively straightforward.
We don't have a copy of your data so it is difficult to reproduce your issue. If you posted your table structure with indexes and constraints, and some sample data maybe it would possible to go further.
Another problem is that you do not have comprehensive error handling in your code. That's something you should start doing from now on to make your code more reliable and more robust. Then, when an error occurs, you could catch the contents of certain variables and gain more information on the offending data. Have a look here for example: How to implement error handling in SQL Server. Just 10 lines of code would make a difference.
You have some transactions but this is not sufficient. They are not used properly and do not cover the whole procedures.
Using a trigger is often a bad solution, especially when you already have a stored procedure to insert/update the data. You should keep the logic concentrated in one place if possible. Too often, triggers are misunderstood and miscoded under wrong assumptions.
If you are still reading this, now here is some potentially important information that may affect you. I think this is what @smor alluded to but I will reinforce the point.
One little-know aspect of insert triggers (at least in MSSQL) is that when you insert multiple rows in one batch, the trigger is invoked once for the whole operation, and not once per row (emphasis is mine):
because an INSERT trigger can be fired by an INSERT INTO (table_name) SELECT statement, the insertion of many rows may cause a single trigger invocation.
Source: Create DML Triggers to Handle Multiple Rows of Data
So that could be one of your problems too (this kind of problem often goes unnoticed for a very long time...).
Unfortunately your code is hard to read but there are things you can do that would cost little and bring some benefit:
- use meaningful names for your variable and object names to avoid confusion and improve readability.
- get rid of dynamic SQL - this alone will make the code a bit easier to follow and understand
- improve the shape of your code - tabulation is important
- add error handling ASAP - there could be many errors happening, that you don't notice and compromise the integrity of your data
- remove unused stuff from your code
For instance in stored procedure ScanIT_spGeraeteEinsatz1 you define a in-memory table:
declare @temptable table (BarcodeName varchar(100))
but apparently you are not actually using it. @temptable2 is not used either or not any more. That only brings confusion.
In a nutshell, now is the time to scale back your code. Think hard about your needs and simplify. Don't try to solve problems when you can avoid them. Improve your code and document yourself, even if it takes time. Bad code is expensive in the long run, because it is hard to maintain and creates more work when you have to address the bugs and deficiencies.
Upvotes: 1
Related Questions
- Error-raising procedure and trigger in SQL Server
- SQL Server trigger on a procedure failing
- Trigger will not execute stored procedure
- SQL Server : trigger stops working
- Stored procedure called from trigger does not run
- SQL Trigger Error Code
- SQL Trigger Silently Fails
- Trigger Failing when calling Stored Procedure
- Stored procedure and trigger
- SQL 2005 Trigger Not Firing Reliably