1

Do I have an issue with my syntax? I created this trigger as sometimes the sales personnel forget to input the sale date (I Know it should be automatically captured which is an update in progress) but we are still seeing a few rows in the table with a null value for saledate Is this accurate syntax to update the field saledate with GetDate() when saledate is null? And I want to run this on Insert Or Update

create trigger updatefield
on salerecords
for insert, update
as
begin
  update salerecords
  set saledate = getdate()
  where saledate Is null
end
  • 6
    Just a question, have you considered setting default value for this column instead of trigger? – Antoine Hernandez Mar 3 '16 at 21:17
2

One problem with your trigger is that it is potentially updating all rows in the table, even if only one row was updated or inserted. If someone accidentally sets the values to NULL, they'll then get updated to the current time, the next time the trigger runs. Does this make sense for your application? I would think it makes more sense to leave past values alone (deal with those separately, though it seems doubtful any still exist), and only bother updating rows that are currently impacted directly by the trigger. You do that by joining to the inserted pseudo-table:

UPDATE sr SET saledate = GETDATE()
  FROM dbo.salerecords AS sr
  INNER JOIN inserted AS i
  ON sr.key_column = i.key_column -- you'll need to update this
  WHERE sr.saledate IS NULL;

That said, I agree with @SoleDBAGuy, who suggested a default constraint. Here is how you could implement that:

ALTER TABLE dbo.salerecords
  ADD CONSTRAINT df_saledate DEFAULT (GETDATE()) FOR saledate;
GO
ALTER TABLE dbo.salerecords
  ALTER COLUMN df_saledate datetime NOT NULL;

And then drop the trigger. Note that the second command may fail; if you have null values in there already, it's hard to justify just giving them "right now" as their sale date, when they could potentially be years old. So you can look at those individually, or just apply some date to them all, like:

UPDATE dbo.salerecords SET saledate = GETDATE();
-- or
UPDATE dbo.salerecords SET saledate = '19000101'; -- beware magic number

Finally, please always use the schema prefix.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.