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?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

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
sam sainiAuthor Commented:
What would be your fee..
0
Protecting & Securing Your Critical Data

Considering 93 percent of companies file for bankruptcy within 12 months of a disaster that blocked access to their data for 10 days or more, planning for the worst is just smart business. Learn how Acronis Backup integrates security at every stage

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
Mark WillsTopic 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

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
sam sainiAuthor Commented:
Completed.. thanks u all for providing the helpful comments
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Microsoft SQL Server

From novice to tech pro — start learning today.