March 22, 2016 at 3:30 pm
Hi, I have been out of the MSSQL world for about 5 years so trying to catch up. I found this wonderful code to help me with formatting HTML output from SP_SEND_DBMAIL. I need to add a WHERE statement to process the email to the person via the PARM being passed from the prior CURSOR. The emails generate but do not yield the correct result. The email address and subject value do work but not the data in the body.
USE [DRB]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[HTMLSendEmailReminder] AS
DECLARE @CC_List nvarchar(512)
DECLARE @AdminEmail nvarchar(128)
DECLARE @aaa_Admin_List nvarchar(512)
DECLARE @AcctMgrEmail nvarchar(128)
DECLARE @AcctMgrID integer
DECLARE @To_List nvarchar(128)
DECLARE @Query_String nvarchar(max)
DECLARE @MessageText nvarchar(max)
DECLARE @ProfileName nvarchar(128)
DECLARE @TABCHAR VARCHAR(1)
--20160321-NewforHTML
DECLARE @bodyMsg nvarchar(max)
DECLARE @SubjectText nvarchar(max)
DECLARE @tableHTML nvarchar(max)
DECLARE @Table NVARCHAR(MAX) = N''
SET @TABCHAR = CHAR(9)
SET @ProfileName = 'UXXXADMIN'
SET @CC_List = ''
SET @AdminEmail = ''
SET @aaa_Admin_List = ''
SET @AcctMgrID = ''
SET @AcctMgrEmail = ''
--SET @AcctMgr = ''
SET @To_List = ''
SET @Query_String = ''
SET @MessageText = 'THIS IS A TEST EMAIL;
The following design numbers have been submitted on the dates shown and have not been approved or rejected.
Please do not reply to this message. It has been automatically generated.
'
SET @SubjectText = '**DO NOT REPLY TO THIS MESSAGE** Past Due Dispositions!'
-- Build cc: list of all aaa_Admins. Save in @CC_List
DECLARE aaa_Admin_List CURSOR LOCAL FAST_FORWARD READ_ONLY FOR --works fine only 1 email for all
SELECT DISTINCT Admin_Email_Addr
FROM dbo.aaa_Admins_CMA
OPEN aaa_Admin_List
FETCH NEXT FROM aaa_Admin_List INTO @AdminEmail
WHILE @@FETCH_STATUS = 0
BEGIN
SET @CC_List = @CC_List + @AdminEmail + ';'
FETCH NEXT FROM aaa_Admin_List INTO @AdminEmail
END
CLOSE aaa_Admin_List
DEALLOCATE aaa_Admin_List
SET @CC_list = @CC_List -- add hardcoded here
-- Build cursor of account managers with open designs. Send each manager's respective list to each.
DECLARE EmailList CURSOR LOCAL FAST_FORWARD READ_ONLY FOR
--20160318-see HelpDesk-152383
SELECT DISTINCT dbo.Account_Mgrs_CMA.Account_Mgrs_ID as AM_ID_ORIG,
--dbo.Design_CMA.aaa_division,
--dbo.Design_CMA.rep_id,
dbo.Account_Mgrs_CMA.AM_Email_Addr AS EmailAdd
FROM dbo.Design_CMA
INNER JOIN dbo.Reps_CMA
ON dbo.DESIGN_CMA.Rep_ID = dbo.Reps_CMA.Rep_ID
INNER JOIN dbo.Account_Mgrs_CMA
--20160317-HD152383-modified HERE to accomodate extra column of data in Reps for AcctMgr_Electro
ON (CASE
WHEN dbo.DESIGN_CMA.aaa_Division <> 'EC'
THEN dbo.Reps_CMA.Account_Mgrs_ID_Electro
ELSE dbo.Reps_CMA.Account_Mgrs_ID
END) = dbo.Account_Mgrs_CMA.Account_Mgrs_ID
WHERE(dbo.DESIGN_CMA.Status = N'New')
AND (dbo.DESIGN_CMA.App_Rej_Date IS NULL)
/*AND (dbo.DESIGN_CMA.Design_ID = 43)*/
AND (dbo.DESIGN_CMA.Create_Date < GETDATE() - 5) --this will send only if the create date is < date-5days
ORDER BY 1
--
/* HERE IS OUTPUT FROM ABOVE CURSOR TO BE USED IN NEXT SET
AM_ID_ORIG EmailAdd
SAMPLE DATA WHERE ALL AM_ID to be matched to emails above
Design_ID Rep_ID Division Create_Date Status Design_Cust_Name JOINRESULT_AM_ID
4844 87 EC 2016-02-24 New hij 21
4847 87 EC 2016-02-24 New klm 21
4848 74 EC 2016-02-29 New acc 31
4849 74 EC 2016-02-29 New add 31
4850 74 EC 2016-02-29 New aee 31
4805 79 EC 2016-01-29 New nop 31
4821 79 EC 2016-02-11 New qrs 31
4826 79 EC 2016-02-11 New tuv 31
4829 79 EC 2016-02-11 New wxy 31
4837 79 EC 2016-02-22 New aab 31
4853 79 EC 2016-03-02 New abb 31
4858 37 NA 2016-03-11 New aff 37
4852 37 EC 2016-03-02 New ahh 37
4860 37 EC 2016-03-01 New aii 37
4823 15 NA 2016-02-11 New abc 38
4861 15 NA 2016-03-01 New def 38
4825 15 EC 2016-02-11 New agg 38
*/
OPEN EmailList
FETCH NEXT FROM EmailList INTO @AcctMgrID, @AcctMgrEmail
WHILE @@FETCH_STATUS = 0
BEGIN
--SET @Query_String = ' --do I need in order for WHERE to work?
SELECT DISTINCT @Table = @Table +
'<tr>' +
'<td>' + CONVERT(varchar, @AcctMgrID) + '</td>' + -- AS @AcctMgrIDPARM
'<td>' + @AcctMgrEmail + '</td>' + --AcctMgrEmailPARM
'<td>' + CAST(dbo.Account_Mgrs_CMA.Account_Mgrs_ID AS VARCHAR(10)) + '</td>' +
'<td>' + dbo.Account_Mgrs_CMA.AM_Email_Addr + '</td>' +
'<td>' + dbo.Account_Mgrs_CMA.AM_Fname + '</td>' +
'<td>' + dbo.Account_Mgrs_CMA.AM_Lname + '</td>' +
'<td>' + CAST(dbo.Design_CMA.Design_ID AS VARCHAR(15)) + '</td>' + -- AS Design_ID,
'<td>' + CONVERT(VARCHAR(10),dbo.Design_CMA.Create_Date, 101) + '</td>' + -- AS Create_Date,
'<td>' + CAST(dbo.Design_CMA.aaa_Division AS VARCHAR(15)) + '</td>' + -- AS Division,
'<td>' + CAST(dbo.Reps_CMA.Rep_Name AS CHAR(55)) + '</td>' + -- AS Rep_Name,
'<td>' + CAST(dbo.Design_CMA.Design_Cust_Name AS CHAR(60)) + '</td>' + -- AS Design_Customer_Name
'</tr>'
FROM dbo.Design_CMA
INNER JOIN dbo.Reps_CMA
ON dbo.Design_CMA.Rep_ID = dbo.Reps_CMA.Rep_ID
INNER JOIN dbo.Account_Mgrs_CMA
ON (CASE
WHEN dbo.DESIGN_CMA.aaa_Division <> 'EC'
THEN dbo.Reps_CMA.Account_Mgrs_ID_Electro
ELSE dbo.Reps_CMA.Account_Mgrs_ID
END) = dbo.Account_Mgrs_CMA.Account_Mgrs_ID
WHERE (dbo.Design_CMA.Status = N'New')
AND (dbo.Design_CMA.App_Rej_Date IS NULL)
AND (dbo.Design_CMA.Create_Date < GETDATE() - 5)
AND dbo.Account_Mgrs_CMA.Account_Mgrs_ID =[highlight="#49ff00"] CONVERT(varchar, @AcctMgrID)[/highlight]
SET @tableHTML =
N'<table border="1"
align="left"
cellpadding="5"
cellspacing="0"
style="color:black;
text-align:left;"
>' +
N'<tr>
<th>AcctMgrIDPARM</th>
<th>AcctMgrEMailPARM</th>
<th>AM_ID</th>
<th>AM_Email_Addr</th>
<th>AM_FName</th>
<th>AM_LNamaem</th>
<th>DesignID</th>
<th>CreateDate</th>
<th>Division</th>
<th>RepName</th>
<th>DesignCustomerName</th>
</tr>' + @Table + N'</table>'
--
exec msdb..sp_send_dbmail @recipients = @AcctMgrEmail, --this is sending fine; body is not correct
@copy_recipients = @CC_list,
--@Subject = @SubjectText,
@Subject = @AcctMgrID, --this works fine in subject; body does not
@body = @tableHTML,
@body_format = 'HTML',
@query_result_separator = @TABCHAR,
@execute_query_database = 'DRB',
@query = @Query_String,
@PROFILE_NAME = @ProfileName
FETCH NEXT FROM EmailList INTO @AcctMgrID, @AcctMgrEmail
END
CLOSE EmailList
DEALLOCATE EmailList
GO
March 23, 2016 at 4:37 am
You say that the body of the mails is incorrect, but not how it is incorrect. Is it empty? Correct data in wrong order? Cut off? Something else?
For troubleshooting, I suggest replacing the call to sp_send_dbmail with a straight PRINT or SELECT of the body.
I don't know why the CONVERT in the query is emphasised, but I do wonder what the data type of the Account_Mgrs_ID column is. You seem to be doing more data type conversions than I would like.
Without knowing what exactly is wrong in the body I cannot tell you what you need to change, but I have to place bets my money is on the "SELECT @Var = @Var + Something FROM ..." construction. Though popular for string concatenation, it is explicitly advised against by Microsoft and many experts, because it does not always work as you might expect. And in your case, with the added DISTINCT, I think that this is a very likely scenario.
I recommend rewriting that part of your query to use the FOR XML PATH method of string concatenation.
March 23, 2016 at 7:43 am
It would probably be helpful if we knew the entire process here. It is a big red flag to me that you state this is being run inside of another cursor. That means you have nested cursors at least two levels deep. Often with things like emails a cursor is required but multiple levels of nesting is generally not a good approach. We can help but as Hugo said, we need some details here.
_______________________________________________________________
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/
March 23, 2016 at 8:32 am
Without the extra detail that the others have requested, your problem is down to guesswork on our part.
That said, this line rings some alarms bells with me:
SELECT DISTINCT @Table = @Table +
It's run inside the cursor, and I can't see anything that resets the contents of @Table after each mail is sent, so that variable is just going to keep on growing with each row in the cursor.
March 23, 2016 at 1:36 pm
Hugo Kornelis (3/23/2016)
You say that the body of the mails is incorrect, but not how it is incorrect. Is it empty? Correct data in wrong order? Cut off? Something else?For troubleshooting, I suggest replacing the call to sp_send_dbmail with a straight PRINT or SELECT of the body.
I don't know why the CONVERT in the query is emphasised, but I do wonder what the data type of the Account_Mgrs_ID column is. You seem to be doing more data type conversions than I would like.
Without knowing what exactly is wrong in the body I cannot tell you what you need to change, but I have to place bets my money is on the "SELECT @Var = @Var + Something FROM ..." construction. Though popular for string concatenation, it is explicitly advised against by Microsoft and many experts, because it does not always work as you might expect. And in your case, with the added DISTINCT, I think that this is a very likely scenario.
I recommend rewriting that part of your query to use the FOR XML PATH method of string concatenation.
Thank you Hugo for your prompt reply. I will research and try your idea of using a straight PRINT or SELECT and the possibly the FOR XML PATH as well. I did mention that I did not write any of it. I just inherited this week after starting a new job and was told to fix the data issues and formatting of the output which has been waiting since the author left in January. I found the HTML code from another blog post and tried to retrofit. I didn't specify the output issues with this script because I want to know HOW to get the WHERE to work using the values provided by CURSOR#2(EmailList) . I thought that by providing the 2 data dumps within the script comments from both CURSOR outputs were enough. I now know to reflect it explicitly in my future posts. Oh and I found that the AcctMgrID field is INT (it's not my design); Today I have found the original nonHTML code using the same functions and that seems to work great, other than sloppy output. I can provide that script if you think it would help? I think Ian's post below has definitely pointed out the obvious that I missed, but unfortunately, still does not match up the WHERE results as expected from Sample data comment. I will add my comments to his thread shortly as well as explicitly state the expected output. Thank you again.
March 23, 2016 at 1:43 pm
Sean Lange (3/23/2016)
It would probably be helpful if we knew the entire process here. It is a big red flag to me that you state this is being run inside of another cursor. That means you have nested cursors at least two levels deep. Often with things like emails a cursor is required but multiple levels of nesting is generally not a good approach. We can help but as Hugo said, we need some details here.
Hello and thank you for your reply. I'm not sure what you need that I haven't already stated. I never stated that there was something being run inside of another cursor. I stated that I am using data provided by a prior cursor here..."...I need to add a WHERE statement to process the email to the person via the PARM being passed from the prior CURSOR...." In my script, I provided both sets of data dumps not knowing I'd have to explicitly state them as well as the output. I will adjust for future posts and will clean this one up after I fix my issue.
March 23, 2016 at 1:50 pm
Ian Scarlett (3/23/2016)
Without the extra detail that the others have requested, your problem is down to guesswork on our part.That said, this line rings some alarms bells with me:
SELECT DISTINCT @Table = @Table +It's run inside the cursor, and I can't see anything that resets the contents of @Table after each mail is sent, so that variable is just going to keep on growing with each row in the cursor.
Thank you for your reply and suggested idea Ian. I obviously missed that for sure as well. As I just responded to the others, I thought I explained myself well enough, and provided data examples within the script, but I now know that I need to explicitly show from this point forward. I have tried your idea and it does now properly match up the AMID/Email provided by the cursor output, but it only provides one within the body where my sample data for AMID 31 is expecting one email for all 9 values. I will provide a better explanation of my desires shortly. Thank you again.
March 23, 2016 at 2:25 pm
SheFixesThings (3/23/2016)
Sean Lange (3/23/2016)
It would probably be helpful if we knew the entire process here. It is a big red flag to me that you state this is being run inside of another cursor. That means you have nested cursors at least two levels deep. Often with things like emails a cursor is required but multiple levels of nesting is generally not a good approach. We can help but as Hugo said, we need some details here.Hello and thank you for your reply. I'm not sure what you need that I haven't already stated. I never stated that there was something being run inside of another cursor. I stated that I am using data provided by a prior cursor here..."...I need to add a WHERE statement to process the email to the person via the PARM being passed from the prior CURSOR...." In my script, I provided both sets of data dumps not knowing I'd have to explicitly state them as well as the output. I will adjust for future posts and will clean this one up after I fix my issue.
I have read and reread your original post and this reply and the only logical conclusion I can come to is that you have a cursor that is calling the code you posted. The code you posted also has a cursor. This is a pretty clear indicator of nested cursors. I was hinting that perhaps we could take one step back and find way to avoid these nested cursors.
If you are simply trying to debug the existing code then what Hugo suggested is about the only way you can proceed. It is nearly impossible for us to guess as we won't be able to recreate the problem from what has been posted so far. It is hard to figure out what you mean about needing to add a where clause. it seems you already have one highlighted in bright green in your original post.
_______________________________________________________________
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/
March 23, 2016 at 2:41 pm
SheFixesThings (3/23/2016)
Oh and I found that the AcctMgrID field is INT
Then two obvious fixes are to replace SET @AcctMgrID = '' with SET @AcctMgrID = 0, and to remove the explicit convert in the problem query. Unlikely to fix your issues, but definitely a good idea to do anyway.
For the rest, people here can probably help you, but we need to be able to reproduce the problem. In order to do that, you need to post a repro script, containing:
* All tables used in the code, as CREATE TABLE statements. Please include all constraints and indexes, but leave out any non-key columns that are irrelevant for this issue.
* A small set of well-chosen sample data, as INSERT statements. Do not post thousands of rows. If you have confidential data, replace it with dummy data.
* The code you are currently using, and the results you get
* The results you need.
For the first three items, please test the code before posting - create an empty database on your test server, paste all the code, hit execute and verify that it runs as expected. Then copy and paste that code to the forum.
March 23, 2016 at 2:57 pm
Sean Lange (3/23/2016)
SheFixesThings (3/23/2016)
Sean Lange (3/23/2016)
It would probably be helpful if we knew the entire process here. It is a big red flag to me that you state this is being run inside of another cursor. That means you have nested cursors at least two levels deep. Often with things like emails a cursor is required but multiple levels of nesting is generally not a good approach. We can help but as Hugo said, we need some details here.Hello and thank you for your reply. I'm not sure what you need that I haven't already stated. I never stated that there was something being run inside of another cursor. I stated that I am using data provided by a prior cursor here..."...I need to add a WHERE statement to process the email to the person via the PARM being passed from the prior CURSOR...." In my script, I provided both sets of data dumps not knowing I'd have to explicitly state them as well as the output. I will adjust for future posts and will clean this one up after I fix my issue.
I have read and reread your original post and this reply and the only logical conclusion I can come to is that you have a cursor that is calling the code you posted. The code you posted also has a cursor. This is a pretty clear indicator of nested cursors. I was hinting that perhaps we could take one step back and find way to avoid these nested cursors.
If you are simply trying to debug the existing code then what Hugo suggested is about the only way you can proceed. It is nearly impossible for us to guess as we won't be able to recreate the problem from what has been posted so far. It is hard to figure out what you mean about needing to add a where clause. it seems you already have one highlighted in bright green in your original post.
I stated that I will provide more information shortly. I do have other work that I need to get done. And no, they are still not nested cursors. There is one outer cursor: CTS_ADMIN_LIST.
And the second, the one I'm stuck on is within EmailList, which contains only 1 cursor that is processing the data found, and applying within a WHILE loop.
Here is an example of a nested Cursor that I found online:
[highlight="#ccccff"]CURSOR1A
DECLARE curAllTables CURSOR STATIC LOCAL FOR[/highlight]
SELECT QUOTENAME(TABLE_SCHEMA) + '.' + QUOTENAME(TABLE_NAME) AS ST
FROM INFORMATION_SCHEMA.TABLES
WHERE TABLE_TYPE = 'BASE TABLE'
AND OBJECTPROPERTY(OBJECT_ID(QUOTENAME(TABLE_SCHEMA) + '.'
+ QUOTENAME(TABLE_NAME)), 'IsMSShipped') != 1
ORDER BY ST
OPEN curAllTables
FETCH NEXT FROM curAllTables
INTO @SchemaTableName
WHILE (@@FETCH_STATUS = 0) -- Outer cursor loop
BEGIN
PRINT @SchemaTableName
SET @SchemaTableColumn = ''
[highlight="#ccccff"]CURSOR 1B
DECLARE curAllColumns CURSOR FOR -- Nested cursor
[/highlight] SELECT QUOTENAME(COLUMN_NAME)
FROM INFORMATION_SCHEMA.COLUMNS
WHERE TABLE_NAME = PARSENAME(@SchemaTableName,1)
AND TABLE_SCHEMA = PARSENAME(@SchemaTableName,2)
AND DATA_TYPE IN ('varchar','nvarchar','char','nchar','xml')
ORDER BY ORDINAL_POSITION
My code is not this. I have nonHTML code that does work in this same fashion that I need to prepare to post, but I have a data issue on something that needs my attention. I just wanted to clarify that it is again, not a nested cursor. if you see a nested cursor, would you be so kind to point it out?
March 23, 2016 at 3:00 pm
What parameter are you passing to the procedure?
Like Sean asked, it would be helpful if you provided the entire stored procedure. The bigger picture comes in with the calling procedure - the one you say is passing data to this procedure.
There's probably a way to eliminate some of these cursors, but without seeing the whole picture, it's impossible to help. Eliminating the aaa_Admin_List cursor is a simple one to build your @CC_List string. My guess is that there are others.
Another guess is that this has evolved over time and updated by a number of people. I say this because you have variables that you never use, such as @To_List. It looks like it was built up but never cleaned up.
A big-picture type of recommendation would be to create a temp table and insert the data you need for the body and recipients without the HTML, which would make it simpler to debug and maintain. Once you have that built correctly, you can cursor through your temp table and send your email.
Viewing 11 posts - 1 through 10 (of 10 total)
You must be logged in to reply to this topic. Login to reply