Help creating a SP

  • SQLisAwE5OmE (3/7/2016)


    Sean Lange (3/7/2016)


    You created your trigger on Orders but you posted the ddl for logger_all. Does the Orders table have the same columns??? The bigger issue is that your trigger has a MAJOR flaw. Even though it has been suggested repeatedly in this thread you still are assuming there will only ever be a single row in inserted. You need to handle multiple row operations. In this case it likely means a loop.

    What does this return?

    select Name

    , [Version]

    , [Timestamp]

    from Orders

    Also, you really should avoid using reserved words as column names. Especially reserved words that are other datatypes. It will drive you nuts!!!

    I am confused. I attached 2 different tables(dbo.Orders & dbo.Order_Meet)

    Attaching them again, also including dbo.logger_all create script as well.

    Look at your trigger code. You created the trigger on the Orders table right? Then you have this.

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @Name = i.Name, @Version = .[Version],

    @Timestamp = i.[Timestamp]

    from inserted i

    Name, Version and Timestamp are going to fail here because those three columns do not exist in the Orders table. I can't tell you what is correct because it is not clear what you are trying to do. This really shouldn't be that difficult. If you could explain the requirements it would be a LOT easier to help you.

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Sean Lange (3/7/2016)


    SQLisAwE5OmE (3/7/2016)


    Sean Lange (3/7/2016)


    You created your trigger on Orders but you posted the ddl for logger_all. Does the Orders table have the same columns??? The bigger issue is that your trigger has a MAJOR flaw. Even though it has been suggested repeatedly in this thread you still are assuming there will only ever be a single row in inserted. You need to handle multiple row operations. In this case it likely means a loop.

    What does this return?

    select Name

    , [Version]

    , [Timestamp]

    from Orders

    Also, you really should avoid using reserved words as column names. Especially reserved words that are other datatypes. It will drive you nuts!!!

    I am confused. I attached 2 different tables(dbo.Orders & dbo.Order_Meet)

    Attaching them again, also including dbo.logger_all create script as well.

    Look at your trigger code. You created the trigger on the Orders table right? Then you have this.

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @Name = i.Name, @Version = .[Version],

    @Timestamp = i.[Timestamp]

    from inserted i

    Name, Version and Timestamp are going to fail here because those three columns do not exist in the Orders table. I can't tell you what is correct because it is not clear what you are trying to do. This really shouldn't be that difficult. If you could explain the requirements it would be a LOT easier to help you.

    Okay, Sean....Thanks.

    I made the changes, here is the updated script....this executed without errors.

    I will test it out now and see. I will definitely have more questions. 🙂

    USE [IMIS_TEST]

    GO

    /****** Object: Trigger [dbo].[Orders_Online_Reg] Script Date: 03/07/2016 14:53:14 ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER TRIGGER [dbo].[Orders_Online_Reg]

    ON [dbo].[Orders]

    AFTER insert

    AS

    DECLARE @ORDER_NUMBER varchar(100)

    DECLARE @ID varchar(10)

    DECLARE @Body varchar(500)

    Declare @Subject varchar(104)

    Declare @Name varchar(200)

    Declare @Version varchar(100)

    Declare @Timestamp varchar(100)

    declare @optype tinyint = 0;

    if exists (select * from inserted) set @optype = @optype+1

    if exists (select * from deleted) set @optype = @optype+2

    BEGIN

    set @Subject =

    case @optype

    when 1 then 'New row inserted into Orders table in XXXXXX'

    else 'Nothing Happened'

    end ;

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    if @optype = 1 or @optype = 3

    select @Body = 'New record has been Updated in Orders table in XXXXXX, Following are the details ' + char(13)

    + 'ORDER_NUMBER : ' + ISNULL(@ORDER_NUMBER,'[Missing ConfigSet]') + CHAR(13)

    + 'Name : ' + ISNULL(@Name ,'[Missing Name]') + CHAR(13)

    + 'Version : ' + ISNULL(@Version,'[Missing Version]') + CHAR(13)

    + 'TimeStamp : ' + ISNULL(@Timestamp,'[Missing Timestamp]') + CHAR(13)

    from inserted i ;

    INSERT INTO dbo.logger_all SELECT @Body, @ORDER_NUMBER, @ID, @Subject, @Name, @Timestamp

    EXEC msdb..sp_send_dbmail

    @profile_name = '@defaultprofile',

    @recipients = '@somedomain.com',

    @subject = @subject,

    @body = @body

    END

    Regards,
    SQLisAwe5oMe.

  • Are you just ignoring the repeated comments about triggers needing to support multiple row operations? If you don't do something in your trigger to handle multiple rows in inserted/deleted you are going to end up in a bad situation at some point because your trigger is NOT going to work correctly.

    You also run the risk of inserting NULLs into your logging table. You conditionally set @Body (and a few other variables) but then you insert those value into your logging table regardless. Maybe you only want to insert when @optype = 1 or @optype = 3?

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    This above will fail whenever the application inserted more than one row as part of the operation. The table [Inserted] will return more than one row in these situations.

    Whenever you get data into your table, the job for the trigger should be as simple as something like

    INSERT into MyLoggingTable(colA, colB, colC, colD, updatedOn)

    SELECT colm1, colm3, colm7, colm8, getdate() /* just the columns you need */

    FROM Inserted

    ----------------------------------------------------

  • Hi Sean & Martin,

    Thanks for your feedbacks.

    I am trying to test this trigger and see if I get an email when I register for a class in our test env first.

    Once that is successfully, I will get back to you guys.

    I really appreciate the feedbacks.

    Regards,
    SQLisAwe5oMe.

  • MMartin1 (3/8/2016)


    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    This above will fail whenever the application inserted more than one row as part of the operation. The table [Inserted] will return more than one row in these situations.

    Whenever you get data into your table, the job for the trigger should be as simple as something like

    INSERT into MyLoggingTable(colA, colB, colC, colD, updatedOn)

    SELECT colm1, colm3, colm7, colm8, getdate() /* just the columns you need */

    FROM Inserted

    To add to what Martin is saying here. It will fail but only logically. It will NOT throw an error. It will set your variables values to the last row in the result set of your query. If your inserted table contains 2 rows you don't even know which one of the two you will get. This why I keep harping on making this set based.

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Hi Sean\Martin,

    Here is my revised code

    USE [IMIS_TEST]

    GO

    /****** Object: Trigger [dbo].[Orders_Online_Reg] Script Date: 03/09/2016 09:29:20 ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER TRIGGER [dbo].[Orders_Online_Reg]

    ON [dbo].[Orders]

    AFTER insert

    AS

    DECLARE @ORDER_NUMBER varchar(100)

    DECLARE @ID varchar(10)

    DECLARE @Body varchar(500)

    Declare @Subject varchar(104)

    Declare @Name varchar(200)

    Declare @Timestamp varchar(100)

    declare @optype tinyint = 0;

    if exists (select * from inserted) set @optype = @optype+1

    BEGIN

    set @Subject = 'New online registration occurred'

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    if @optype = 1 or @optype = 3

    select @Body = 'Following are the details: ' + char(13)

    + 'ORDER_NUMBER : ' + @ORDER_NUMBER + CHAR(13)

    + 'ID : ' + @ID + CHAR(13)

    + 'Name : ' + @Name + CHAR(13)

    + 'TimeStamp : ' + @Timestamp

    from inserted i ;

    INSERT INTO dbo.logger_all SELECT @Body, @ORDER_NUMBER, @ID, @Subject, @Name, @Timestamp

    EXEC msdb..sp_send_dbmail

    @profile_name = 'DefaultProfile',

    @recipients = 'SomeDomain.com',

    @subject = @subject,

    @body = @body

    END

    Trigger is working, and I am getting emails.....but need few modifications.

    Below is the body of my email.

    Following are the details ORDER_NUMBER : 116256.00 ID : 12345 Name : Mr. Tony Alexander TimeStamp : Mar 9 2016 12:00AM

    I want the details to be in each line, but currently it shows as one line....I did use + char(13) but still not working

    Regards,
    SQLisAwe5oMe.

  • SQLisAwE5OmE (3/9/2016)


    Hi Sean\Martin,

    Here is my revised code

    USE [IMIS_TEST]

    GO

    /****** Object: Trigger [dbo].[Orders_Online_Reg] Script Date: 03/09/2016 09:29:20 ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER TRIGGER [dbo].[Orders_Online_Reg]

    ON [dbo].[Orders]

    AFTER insert

    AS

    DECLARE @ORDER_NUMBER varchar(100)

    DECLARE @ID varchar(10)

    DECLARE @Body varchar(500)

    Declare @Subject varchar(104)

    Declare @Name varchar(200)

    Declare @Timestamp varchar(100)

    declare @optype tinyint = 0;

    if exists (select * from inserted) set @optype = @optype+1

    BEGIN

    set @Subject = 'New online registration occurred'

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    if @optype = 1 or @optype = 3

    select @Body = 'Following are the details: ' + char(13)

    + 'ORDER_NUMBER : ' + @ORDER_NUMBER + CHAR(13)

    + 'ID : ' + @ID + CHAR(13)

    + 'Name : ' + @Name + CHAR(13)

    + 'TimeStamp : ' + @Timestamp

    from inserted i ;

    INSERT INTO dbo.logger_all SELECT @Body, @ORDER_NUMBER, @ID, @Subject, @Name, @Timestamp

    EXEC msdb..sp_send_dbmail

    @profile_name = 'DefaultProfile',

    @recipients = 'SomeDomain.com',

    @subject = @subject,

    @body = @body

    END

    Trigger is working, and I am getting emails.....but need few modifications.

    Below is the body of my email.

    Following are the details ORDER_NUMBER : 116256.00 ID : 12345 Name : Mr. Tony Alexander TimeStamp : Mar 9 2016 12:00AM

    I want the details to be in each line, but currently it shows as one line....I did use + char(13) but still not working

    I just counted and I have said 6 times that you MUST handle multiple row operations and that you should avoid scalar variables. You have repeatedly ignored me on this. Your trigger is going to fail at some point and it is going to fail badly. I realize this one is only going to skip sending some emails but you will use this as an example for future work. You are building a very poor design here and are refusing to listen, or even acknowledge my repeated recommendations. Try running your "working" trigger with an insert that inserts two rows and see what happens.

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Can you tell me where I need to make the modification, to include multiple inserts.

    Regards,
    SQLisAwe5oMe.

  • When I purchased 2 programs(classes) in the same order, I got 2 separate emails. So, is that mean it's working?

    Regards,
    SQLisAwe5oMe.

  • SQLisAwE5OmE (3/9/2016)


    When I purchased 2 programs(classes) in the same order, I got 2 separate emails. So, is that mean it's working?

    No. That means that your program inserts the rows one at a time. All insert statements are NOT single row. You need to send an email for EACH row in inserted. This means looping. This is one of the few times where you need to loop. Strangely it is usually difficult to get people to not loop. In this case I can't seem to convince you that you need to. 😉

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Sean Lange (3/9/2016)


    SQLisAwE5OmE (3/9/2016)


    When I purchased 2 programs(classes) in the same order, I got 2 separate emails. So, is that mean it's working?

    No. That means that your program inserts the rows one at a time. All insert statements are NOT single row. You need to send an email for EACH row in inserted. This means looping. This is one of the few times where you need to loop. Strangely it is usually difficult to get people to not loop. In this case I can't seem to convince you that you need to. 😉

    You mean I don't need to worry about multiple row inserts?

    Regards,
    SQLisAwe5oMe.

  • I have modified my trigger to do a join with another table to get the Meeting(Class) code since it doesn't appear on dbo.order table, the meeting code is inserted to dbo.order_meet table.

    However, after doing this, when I register for a class, I am getting blank email...can you point out if I my join is correct in the code, or where exactly I am off..

    USE [IMIS_TEST]

    GO

    /****** Object: Trigger [dbo].[Orders_Online_Reg] Script Date: 03/09/2016 09:29:20 ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER TRIGGER [dbo].[Orders_Online_Reg]

    ON [dbo].[Orders]

    AFTER insert

    AS

    DECLARE @ORDER_NUMBER varchar(100)

    DECLARE @ID varchar(10)

    DECLARE @Body varchar(500)

    Declare @Subject varchar(104)

    Declare @Name varchar(200)

    Declare @Timestamp varchar(100)

    Declare @Meeting varchar(15);

    if exists (select * from inserted)

    BEGIN

    set @Subject = 'New online registration occurred'

    SELECT @ORDER_NUMBER = i.ORDER_NUMBER, @Meeting = om.Meeting, @ID = i.ST_ID, @Name = i.FULL_NAME, @Timestamp = i.ORDER_DATE

    from inserted i

    join dbo.order_meet om on i.ORDER_NUMBER=om.ORDER_NUMBER

    select @Body = 'Following are the details: ' + CHAR(13)+CHAR(10) + CHAR(13)+CHAR(10)

    + 'ORDER_NUMBER : ' + @ORDER_NUMBER + CHAR(13)+CHAR(10)

    + 'ID : ' + @ID + CHAR(13)+CHAR(10)

    + 'Name : ' + @Name + CHAR(13)+CHAR(10)

    + 'Meeting : ' + @Meeting + CHAR(13)+CHAR(10)

    + 'TimeStamp : ' + @Timestamp

    from inserted i ;

    INSERT INTO dbo.logger_all SELECT @Body, @ORDER_NUMBER, @ID, @Subject, @Name, @Timestamp

    EXEC msdb..sp_send_dbmail

    @profile_name = 'DefaultProfile',

    @recipients = 'Some@domain.com',

    @subject = @subject,

    @body = @body

    END

    Regards,
    SQLisAwe5oMe.

  • SQLisAwE5OmE (3/9/2016)


    Sean Lange (3/9/2016)


    SQLisAwE5OmE (3/9/2016)


    When I purchased 2 programs(classes) in the same order, I got 2 separate emails. So, is that mean it's working?

    No. That means that your program inserts the rows one at a time. All insert statements are NOT single row. You need to send an email for EACH row in inserted. This means looping. This is one of the few times where you need to loop. Strangely it is usually difficult to get people to not loop. In this case I can't seem to convince you that you need to. 😉

    You mean I don't need to worry about multiple row inserts?

    You DO need to worry about them. That is what I have said now almost 10 times in this thread!!! Just because the app doesn't today does NOT mean it won't in the future. And it is not the only way rows can be inserted into that table. You could very easily write an insert statement yourself right??? This is turning into an episode of "Who's on first?".

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Sean Lange (3/9/2016)


    SQLisAwE5OmE (3/9/2016)


    Sean Lange (3/9/2016)


    SQLisAwE5OmE (3/9/2016)


    When I purchased 2 programs(classes) in the same order, I got 2 separate emails. So, is that mean it's working?

    No. That means that your program inserts the rows one at a time. All insert statements are NOT single row. You need to send an email for EACH row in inserted. This means looping. This is one of the few times where you need to loop. Strangely it is usually difficult to get people to not loop. In this case I can't seem to convince you that you need to. 😉

    You mean I don't need to worry about multiple row inserts?

    You DO need to worry about them. That is what I have said now almost 10 times in this thread!!! Just because the app doesn't today does NOT mean it won't in the future. And it is not the only way rows can be inserted into that table. You could very easily write an insert statement yourself right??? This is turning into an episode of "Who's on first?".

    I only need to worry about online registrations....dbo.order table....column name(Source_Code = WEB)....I need to specify that also in the script.

    Any manual registration, I don't need to get notifications.

    Regards,
    SQLisAwe5oMe.

Viewing 15 posts - 16 through 30 (of 43 total)

You must be logged in to reply to this topic. Login to reply