Insert Trigger Issue

I have form where I enter name, address, city, phone and date of birth, image. So once I save the data through the form, I have trigger in the sql which will insert 2 more columns as Expiry Date, Auto Expiry date. The trigger will look for the month of the dob(date of birth) and from current year will add 5 years  into it. For Example
Date of Birth: 1978-08-27
Current Year = 2018
Expiry Date = 'Aug 2023'
Auto Expiry Date = 2023-08-27

The issue is the data gets inserted but it doesn't update the front end field BUT DATE GETS INSERTED IN THE BACKEND. I am attaching the logic, snapshot.

I have 2 triggers
1. Insert - Issue
2. Update - Update tigger is working fine


Please let me know, If want to chat or talk over the phone or have WebEx/ Remote  session.

Regards
Sumit
Insert-Trigger.txt
Update-Trigger.txt
Trigger-Issue.docx
sam sainiAsked:
Who is Participating?
 
Mark WillsConnect With a Mentor Topic AdvisorCommented:
@sumit,

Can appreciate that your form refresh error was due to a hidden ID, but, still urge you to consider updating / replacing those triggers.

Converting from Oracle (if I saw that annotation correctly) does do things differently with triggers, so isnt really 'best practices' for SQL Server.

Your "instead of insert" trigger along with the update trigger. Both use cursors for essentially the same work. They could be combined into the one trigger using set based operations on INSERTED.

In an ideal world, just to have those couple of columns updated, use COMPUTED columns and avoid all the hassles of triggers entirely

To give you an idea, if you are SQL2012 or more recent this is the type of thing we can do with your new dates
declare        @new$DATEOFBIRTH datetime2(0) = '1978-08-27', 
               @new$EXPIRYDATE varchar(8), 
               @new$AUTOEXPDATE datetime2(0)

SET @new$AUTOEXPDATE = DATETIME2FROMPARTS(year(sysdatetime())+5, datepart(m,@new$dateofbirth),datepart(d,@new$dateofbirth),0,0,0,0,0)
SET @new$EXPIRYDATE = format(@new$AUTOEXPDATE,'MMM yyyy')

select @new$EXPIRYDATE as EXPIRYDATE, @new$AUTOEXPDATE as AUTOEXPDATE 

Open in new window

Which is very easy to incorporate as COMPUTED columns, or into a trigger.

And as a trigger, all you really need is
    update b SET AUTOEXPDATE = (what ever calcs) --DATETIME2FROMPARTS(year(sysdatetime())+5, datepart(m,i.dateofbirth),datepart(d,i.dateofbirth),0,0,0,0,0)
                ,EXPIRYDATE = (what ever calcs)  --format(DATEFROMPARTS(year(sysdatetime())+5, datepart(m,i.dateofbirth),1),'MMM yyyy')
    from BRAMPTONTRANSITSENIORIDCARDS b
	inner join inserted i on b.rowid = i.rowid

Open in new window

As computed columns, you would need to drop the current two before you can add the new two - alter column wont be permitted to make the changes.

By all means select your own answer in #a42486768 as BEST (and any other assisted), but please consider reworking your triggers or computed columns. Maybe post another question ?

Kind Regards,
Mark Wills
0
 
Brian CroweDatabase AdministratorCommented:
First you need to decide where you want this business logic to reside.
  • application
  • API
  • database

In my opinion, this sort of business logic should reside in an API layer between your application and the database but I am assuming that this does not exist in your current environment.  You could choose to just add 5 years in the application and write that value to the database but that would require an application update if you ever needed to modify that value.

In your case I would funnel the insert/update through a stored procedure and please burn those cursor-using triggers in a fire.  Triggers and Cursors should be used sparingly if at all...combining them is an abomination and completely unnecessary in this case.  Use the stored procedure to return the resulting record to the application with the expiry dates.

If this sounds viable to you please respond and I can help you with the stored procedure.
0
 
sam sainiAuthor Commented:
Hi Brian,

I am looking what you you are suggesting as I want to have all updates through database..as currently my update trigger works only..not the insert ..


Regards
Sumit
0
Free Tool: Path Explorer

An intuitive utility to help find the CSS path to UI elements on a webpage. These paths are used frequently in a variety of front-end development and QA automation tasks.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

 
sam sainiAuthor Commented:
What would be your fee..
0
 
Mark WillsTopic AdvisorCommented:
I dont mind the use of triggers...

But, have to agree with Brian. Using an INSTEAD OF INSERT is simply asking for trouble, and using CURSOR to transverse Inserted is not needed.

In fact, you could simply set up default values, or make them computed / calculated columns in SQL

So no real need to do all that work just so you can calculate :

 SET @new$AUTOEXPDATE = dateadd(m, 60, CONVERT(datetime2, ISNULL(CONVERT(varchar(5), sysdatetime(), 111), '') + ISNULL(CONVERT(varchar(5), @new$DATEOFBIRTH, 101), ''), 111))

                        --SET @new$EXPIRYDATE = ssma_oracle.to_char_date(dateadd(m, 60, CONVERT(datetime2, ISNULL(CONVERT(varchar(5), sysdatetime(), 111), '') + ISNULL(CONVERT(varchar(5), @new$DATEOFBIRTH, 101), ''), 111)), 'Mon YYYY')
					SET @new$EXPIRYDATE = 	LEFT(CONVERT(varchar(3),dateadd(m, 60, CONVERT(datetime2, ISNULL(CONVERT(varchar(5), sysdatetime(), 111), '') + ISNULL(CONVERT(varchar(5), @new$DATEOFBIRTH, 101), ''), 111)),0),3) + ' ' 
+CONVERT(varchar(4),DATEPART(YEAR,dateadd(m, 60, CONVERT(datetime2, ISNULL(CONVERT(varchar(5), sysdatetime(), 111), '') + ISNULL(CONVERT(varchar(5), @new$DATEOFBIRTH, 101), ''), 111))))

Open in new window


Will need to decipher the above, but think there are much better ways to achieve than the current triggers.
0
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
You don't need a Trigger for that. This problem can be easily solved with computed columns.
Just run the following code and you won't need to do anything else besides dropping your trigger:
ALTER TABLE BRAMPTONTRANSITSENIORIDCARDS ADD
	EXPIRYDATE AS LEFT(DATENAME(month,DATEOFBIRTH),3) +  ' ' + CAST(YEAR(GETDATE())+5 AS CHAR(4)),
	AUTOEXPDATE AS DATEFROMPARTS(YEAR(GETDATE())+5, MONTH(DATEOFBIRTH), DAY(DATEOFBIRTH))

Open in new window

What's good with computed columns is that they are virtual so no disk space needed. And once you create those columns, you don't need to do nothing for existing rows as they will be immediately updated.
1
 
Mark WillsTopic AdvisorCommented:
Agree wholeheartedly - even suggested it :)

But would need to first drop, or ALTER the columns.... Can not ADD the columns, they already exist in the table....
0
 
sam sainiAuthor Commented:
Hello Vitor, The issue is not for inserting the records but its reading the field back once its inserted and that's causing the issue. I tried the code which you provided and gets inserted but reading back gives in error which my trigger was also giving it..
0
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
What do you mean with "reading back"?
Those columns are computed, which mean they will be calculated every time you run a SELECT.
0
 
sam sainiAuthor Commented:
The data which I am inserting through application..I had earlier attached the file but I will add it shortly..the attachment
0
 
sam sainiAuthor Commented:
here u go .  i had enclosed the file..
Trigger-Issue.docx
0
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
My doubt was about what means "reading back". It's populating the form with an existing record?
If so, how are you connecting the form field with the table field?
0
 
sam sainiAuthor Commented:
the issue i had resolved it. The issue was the rowid column when it was inserting, i was pointing to a field which i had hidden so once i was trying to save it was not allowing to read back as looking for the row id which does not exist..
Thank you all for the inputs/suggestions.

Regards
Sumit..
0
 
Vitor MontalvãoMSSQL Senior EngineerCommented:
Good.
Please close this question by accepting the comment or comments that helped you out.
Cheers.
0
 
sam sainiAuthor Commented:
Completed.. thanks u all for providing the helpful comments
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.