January 22, 2016 at 1:31 pm
I have been asked to change an existing trigger to look at the Action Plan column, and if it has changed, update the Action Plan Change Tracking Date/time. (this starts out as NULL) I have not worked on triggers in the past, but the code for this one just feels wrong, first for the insert into the tracking table there is no matching of the mult-column PK between the inserted data and the read data... isn't that a requirement for a trigger to always work correctly? (like has been done in the second part of the trigger)
The trigger does two things: First it writes the record to a change history table so the last changes can then be written back after the table is truncated and new data loaded. Second it updates some fields, in the updated table.
I am going to give a much reduced trigger (not as many columns), to try and make this question easier to answer.
The change I need to make, I have started using the version is V001.007.
ALTER TRIGGER [dbo].[Log_Details] ON [dbo].[ztb_Details] FOR UPDATE
AS
BEGIN
INSERT INTO ztb_Details_Comment_Hist
( Period
, [Plant-Material]
, [Batch_Num]
, [Explanation]
, [Reserve_Value]
, [Last_Update_User]
, [Last_Update_DT]
, [Action_Plan]
, Action_Category_Change_DT ---** V001.007
)
SELECT cast(Period as date) as Period_DM---** V001.004
, [Plant-Material]
, [Batch_Num]
, [Reserve_Pct]
, [Explanation]
, [Tot_Value_Inv_Group] * [Reserve_Pct] as [Reserve_Value]
, ( SELECT ISNULL(FirstName+ ' '+ Lastname, System_User)
FROM dbo. WHERE LoginName = System_User) as [Last_Update_User]
, CURRENT_TIMESTAMP as [Last_Update_DT]
---** V001.005 End
, Action_Plan
---** V001.007 Begin
, Action_Category_Change_DT
--- ---** V001.007 End
FROM inserted
--===============================--
UPDATE A
SET
Last_Update_DT = CURRENT_TIMESTAMP
, Last_Update_User = ( SELECT ISNULL(FirstName+ ' '+ Lastname, System_User)
FROM dbo. WHERE LoginName = System_User)
, Reserve_Value = b.[Tot_Value_Inv_Group] * b.[Reserve_Pct]
---** V001.007 Begin
, Action_Category_Change_DT = b.Action_Category_Change_DT
--- ---** V001.007 End
FROM ztb_Details A
INNER JOIN inserted B
on a.[plant-material] = b.[plant-material]
and a.[batch_num] = b.[batch_num]
and a.[Period] = b.[Period]
END
CREATE TABLE [dbo].[ztb_Details_Comment_Hist](
[id] [int] IDENTITY(1,1) NOT NULL,
[Period] [date](250) NOT NULL,
[Plant-Material] [char](12) NOT NULL,
[Batch_Num] [varchar](20) NOT NULL,
[Explanation] [varchar](1000) NULL,
[Reserve_Value] [float] NULL,
[Last_Update_User] [varchar](200) NULL,
[Last_Update_DT] [datetime] NOT NULL,
[Action_Plan] [varchar](1000) NULL,
[Action_Category_Change_DT] [datetime] NULL,
CONSTRAINT [PK_ztb_Details_Comment_Hist] PRIMARY KEY CLUSTERED
(
[id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
CREATE TABLE [dbo].[ztb_Details](
[Period] [datetime] NOT NULL,
[Material_CD] [char](6) NOT NULL,
[Plant_CD] [char](3) NOT NULL,
[Batch_Num] [varchar](20) NOT NULL,
[Customer] [varchar](100) NOT NULL,
[Reserve_Pct] [float] NULL,
[Tot_Value_Inv_Group] [float] NULL,
[Reserve_Value] [float] NULL,
[Action_Plan] [varchar](1000) NULL,
[Plant-Material] [char](12) NOT NULL,
[Action_Category_Change_DT] [datetime] NULL,
[Loaded_From] [varchar](50) NOT NULL,
CONSTRAINT [PK_ztb_Details_1] PRIMARY KEY CLUSTERED
(
[Period] ASC,
[Material_CD] ASC,
[Plant_CD] ASC,
[Batch_Num] ASC,
[Customer] ASC,
[Loaded_From] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
January 23, 2016 at 5:46 am
After taking a quick look, I can tell you that you are right.
The UPDATE FROM used in the trigger will affect all rows that match an inserted/updated row on the combination of three columns provided. Which works out to exactly the updated rows if that combination is exactly unique in the table - but since the primary key is on a combination of those three plus three additional columns, that is not guaranteed.
So either the primary key is incorrectly defined, or the trigger is wrong.
January 24, 2016 at 1:05 pm
Change this part of the UPDATE query in the trigger
FROM ztb_Details A
INNER JOIN inserted B
on a.[plant-material] = b.[plant-material]
and a.[batch_num] = b.[batch_num]
and a.[Period] = b.[Period]
to use all the columns from the primary key and you'll be sure that only the inserted columns are updated.
[plant-material] is not part of the PK.
i.e.
FROM ztb_Details A
INNER JOIN inserted B
on
a.[Period]=b.[Period]
a.[Material_CD]=b.[Material_CD]
a.[Plant_CD]=b.[Plant_CD]
a.[Batch_Num]=b.[Batch_Num]
a.[Customer]=b.[Customer]
a.[Loaded_From]=b.[Loaded_From]
Igor Micev,
My blog: www.igormicev.com
January 24, 2016 at 3:48 pm
...and what if the intention was to update all rows matched on those three columns?
Before you go changing something just because it "looks wrong", I suggest you check with the business whether they are having the data corruption you think it is causing and whether anyone knows what the trigger was designed to do.
MM
select geometry::STGeomFromWKB(0x0106000000020000000103000000010000000B0000001000000000000840000000000000003DD8CCCCCCCCCC0840000000000000003DD8CCCCCCCCCC08408014AE47E17AFC3F040000000000104000CDCCCCCCCCEC3F9C999999999913408014AE47E17AFC3F9C99999999991340000000000000003D0000000000001440000000000000003D000000000000144000000000000000400400000000001040000000000000F03F100000000000084000000000000000401000000000000840000000000000003D0103000000010000000B000000000000000000143D000000000000003D009E99999999B93F000000000000003D009E99999999B93F8014AE47E17AFC3F400000000000F03F00CDCCCCCCCCEC3FA06666666666FE3F8014AE47E17AFC3FA06666666666FE3F000000000000003D1800000000000040000000000000003D18000000000000400000000000000040400000000000F03F000000000000F03F000000000000143D0000000000000040000000000000143D000000000000003D, 0);January 25, 2016 at 8:36 am
Hugo Kornelis (1/23/2016)
After taking a quick look, I can tell you that you are right.The UPDATE FROM used in the trigger will affect all rows that match an inserted/updated row on the combination of three columns provided. Which works out to exactly the updated rows if that combination is exactly unique in the table - but since the primary key is on a combination of those three plus three additional columns, that is not guaranteed.
So either the primary key is incorrectly defined, or the trigger is wrong.
Yep... the PK is correct, the trigger is wrong.. There is only a few rows that overlap, between the three source files being loaded into this table... but if a user was to update one of those rows..then we would have bad data. (Either that has not happened yet, this table is just over a year old, or no one has noticed it yet.)
Viewing 5 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply