May 12, 2015 at 12:02 am
Hi,
I have created below stored procedure and our Code review asked me to modify the code. they asked me to use LOOP instead of Multiple IFs. can anybody help me to write that Code. Thanks in Advance
USE [TEST]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
EXEC forecast.PR_CREATE_PROC_STUB '[forecast].[PR_TEST_ABC]'
GO
ALTER PROCEDURE [forecast].[PR_TEST_ABC]
AS
BEGIN
DECLARE
@l_srvr_nm VARCHAR(25),
@l_autosys_id varchar(15),
@l_msg_machine_nm VARCHAR(max),
@l_msg_rows_rejected varchar(max),
@l_msg_run_sts varchar(max),
@l_msg_last_mod_idsid varchar(max),
@l_cnt_machine_nm INT,
@l_cnt_rows_rejected INT,
@l_cnt_run_sts INT,
@l_cnt_last_mod_idsid INT,
@l_sub varchar(200)
SET @l_cnt_machine_nm = 0
SET @l_cnt_rows_rejected = 0
SET @l_cnt_run_sts = 0
SET @l_cnt_last_mod_idsid = 0
SET @l_msg_machine_nm = 'System has encountered the following stg tables entering wrong M/C names in STG_LD_JOB: <br /><br />'
SET @l_msg_rows_rejected = 'System has encountered the following stg tables having rejects in records: <br /><br />'
SET @l_msg_run_sts = 'System has encountered the following stg tables having issues in run status in STG_LD_JOB: <br /><br />'
SET @l_msg_last_mod_idsid = 'System has encountered the following stg tables having wrong last_mod_idsid in STG_LD_JOB: <br /><br />'
SET @l_srvr_nm = CONCAT(@l_srvr_nm,CAST(SERVERPROPERTY('ComputerNamePhysicalNetBIOS') AS VARCHAR(25)))
SET @l_autosys_id = CASE WHEN @l_srvr_nm like '%PRD%'
THEN 'AMR\autosys'
ELSE 'AMR\sys_autosystest'
END
BEGIN TRY
SELECT * FROM #TMP_STG_LD_JOB FROM
(
SELECT
RANK() OVER (PARTITION BY PKG_NM ORDER BY container_strt_tm DESC) AS RNKING
,PKG_NM
,MACHINE_NM
,ROWS_REJECTED
,RUN_STS
,LAST_MOD_IDSID
FROM PRODSQL.DF_STG.forecast.STG_LD_JOB
WHERE PKG_NM like 'STG%' OR PKG_NM like 'forecast.STG%' OR PKG_NM like 'dbo.%'
)RNK
WHERE RNKING = 1
SELECT
@l_msg_machine_nm = CONCAT(@l_msg_machine_nm,SUBSTRING(PKG_NM,0,charindex('.dtsx',PKG_NM)),'<br />'),
@l_cnt_machine_nm = @l_cnt_machine_nm + 1
FROM #TMP_STG_LD_JOB
WHERE MACHINE_NM <> @l_srvr_nm
SELECT
@l_msg_rows_rejected = CONCAT(@l_msg_rows_rejected,SUBSTRING(PKG_NM,0,charindex('.dtsx',PKG_NM)),' : ',
CAST(ROWS_REJECTED AS VARCHAR),'<br />'),
@l_cnt_rows_rejected = @l_cnt_rows_rejected + 1
FROM #TMP_STG_LD_JOB
WHERE ROWS_REJECTED <> 0
SELECT @l_msg_run_sts = CONCAT(@l_msg_run_sts,SUBSTRING(PKG_NM,0,charindex('.dtsx',PKG_NM)),'<br />'),
@l_cnt_run_sts = @l_cnt_run_sts +1
FROM #TMP_STG_LD_JOB
WHERE RUN_STS <> 'COMPLETED'
SELECT @l_msg_last_mod_idsid = CONCAT(@l_msg_last_mod_idsid,SUBSTRING(PKG_NM,0,charindex('.dtsx',PKG_NM)),'<br />'),
@l_cnt_last_mod_idsid = @l_cnt_last_mod_idsid + 1
FROM #TMP_STG_LD_JOB
WHERE LAST_MOD_IDSID <> @l_autosys_id
IF(@l_cnt_machine_nm > 0)
BEGIN
SET @l_msg_machine_nm = CONCAT(@l_msg_machine_nm,'<br />Total of tables having M/C name issue: ',
CAST(@l_cnt_machine_nm AS VARCHAR),'<br />')
SET @l_sub = CONCAT(@l_srvr_nm,': M/C Name issue in STG_LD_JOB')
EXECUTE DF.dbo.PR_SQL_DB_Mail
@from_user_email = 'abc.xyz@pqrst.com',
@to_user_email = '1a.2b@pqrst.com',
@subject = @l_sub,
@message = @l_msg_machine_nm,
@dce_cc_list = '1a.2b@pqrst.com',
@priority = NULL,
@attachment = NULL,
@query_attachment_filename = NULL;
SET @l_sub = ''
END;
IF(@l_cnt_rows_rejected > 0)
BEGIN
SET @l_msg_rows_rejected = CONCAT(@l_msg_rows_rejected,'<br />Total of tables having rejects: ',
CAST(@l_cnt_rows_rejected AS VARCHAR),'<br />')
SET @l_sub = CONCAT(@l_srvr_nm,': Rejects in staging table')
EXECUTE DF.dbo.PR_SQL_DB_Mail
@from_user_email = 'abc.xyz@pqrst.com',
@to_user_email = '1a.2b@pqrst.com',
@subject = @l_sub,
@message = @l_msg_rows_rejected,
@dce_cc_list = '1a.2b@pqrst.com',
@priority = 'HIGH',
@attachment = NULL,
@query_attachment_filename = NULL;
SET @l_sub = ''
END;
IF(@l_cnt_run_sts > 0)
BEGIN
SET @l_msg_run_sts = CONCAT(@l_msg_run_sts,'<br />Total of tables having run status issue: ',
CAST(@l_cnt_run_sts AS VARCHAR) + '<br />')
SET @l_sub = CONCAT(@l_srvr_nm,': run_status is not complete')
EXECUTE DF.dbo.PR_SQL_DB_Mail
@from_user_email = 'abc.xyz@pqrst.com',
@to_user_email = '1a.2b@pqrst.com',
@subject = @l_sub,
@message = @l_msg_run_sts,
@dce_cc_list = '1a.2b@pqrst.com',
@priority = NULL,
@attachment = NULL,
@query_attachment_filename = NULL;
SET @l_sub = ''
END;
IF(@l_cnt_last_mod_idsid > 0)
BEGIN
SET @l_msg_last_mod_idsid = CONCAT(@l_msg_last_mod_idsid,'<br />Total of tables having last mod idsid issue: ',
CAST(@l_cnt_last_mod_idsid AS VARCHAR) + '<br />')
SET @l_sub = CONCAT(@l_srvr_nm,': last_mod_idsid is not correct')
EXECUTE DF.dbo.PR_SQL_DB_Mail
@from_user_email = 'abc.xyz@pqrst.com',
@to_user_email = '1a.2b@pqrst.com',
@subject = @l_sub,
@message = @l_msg_last_mod_idsid,
@dce_cc_list = '1a.2b@pqrst.com',
@priority = NULL,
@attachment = NULL,
@query_attachment_filename = NULL;
SET @l_sub = ''
END
END TRY
BEGIN CATCH
EXEC forecast.PR_CUSTOM_ERRMSG;
END CATCH
END
May 12, 2015 at 2:36 am
What does "use LOOP" mean? Can you please clarify?
-- Gianluca Sartori
May 12, 2015 at 3:15 am
Hi,
If you see SP , there are 4 IF conditions i used, they suggested me to use WHILE LOOP instead of multiple IF conditions. I am not able to Implement the same. TIA
May 12, 2015 at 3:16 am
Why? That doesn't make any sense. It's no something that can be looped over, it won't make the code clearer, faster or anything else.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 12, 2015 at 3:17 am
You may generalise 4 types of problems as a rows of a table variable.
DECLARE @problems TABLE (
id int -- problem type =1,2,3,4
,cnt int
,msg VARCHAR(MAX)
, priority VARCHAR (100)
-- more varying DF.dbo.PR_SQL_DB_Mail parameters
);
INSERT @problems (id, cnt, msg, priority)
VALUES (1, 0,'', NULL)
,(2, 0,'', NULL)
,(3, 0,'', 'high')
,(4, 0,'', NULL);
Then update it using collected #TMP_STG_LD_JOB data (BTW, don't you mean SELECT * INTO #TMP_STG_LD_JOB FROM (... ? )
Then loop @problems (using cursor or just plain @i=1..4) and execute DF.dbo.PR_SQL_DB_Mail for the rows with cnt >0.
May 12, 2015 at 3:17 am
I don't see how a WHILE loop can help you here.
-- Gianluca Sartori
May 12, 2015 at 3:20 am
okay.. i can put it my question like this.. how can i optimise my SP ?
May 12, 2015 at 3:25 am
May be a future big number of problem types is a reason behind the requierment.
May 12, 2015 at 3:30 am
Yes.. you are correct. we are going to integrate this SP with UI.. So want to optimise further .
May 12, 2015 at 3:32 am
friend.vasu (5/12/2015)
okay.. i can put it my question like this.. how can i optimise my SP ?
No idea - http://www.sqlservercentral.com/articles/SQLServerCentral/66909/
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 12, 2015 at 3:43 am
If procedure perfomance is a concern then any WHILE, any extra table impose some overhead you know.
As a simple example
x =1+2+3+4+5
is faster then
x=0
for i=1..5 do {x += i}
🙂
What is exactly the reason behind the requirement?
May 14, 2015 at 4:19 am
You are probably under a bit of pressure here and consequently not communicating your problem as well as you could?
I can't see how a loop can help you here as you are not even testing for the same conditions... unless you're talking about nesting your While loops in which case I can't see how that will be any more elegant than what you currently have.
You may need to provide a little more context around the while.. loop approach...
As far as optimising the procedure is concerned, you'll need to provide a lot more info about the underlying database objects the procedure interacts with such as other procedures, tables and available indexes, execution plans and the like to get any useful help although having not hinted to performance issues as such you may be really wanting to refactor the procedure more than anything else.
Take a step back and try to clarify the requirements as you would to a "new joiner" in your company and you might get better help bearing in mind that your procedure contains business logic no one outside your organisation/team has any knowledge about.
Viewing 13 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply