Evolution of a stored proc
-
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
Hmm, I will never complain about Visual Basic again... :laugh:
-
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
Good lord, I'm having flashbacks from my last job. We had nearly 500,000 lines of SQL in stored procedures. And I really don't see the need for the "usp" prefix. Is it really so common to confuse a user stored procedure with a system stored procedure? At least it seems the developers that made this attempt to maintain some sort of style. At my current job, most stored procedures look like they were generated from a designer and no attempt was made to format the code. I feel your pain.
-
Good lord, I'm having flashbacks from my last job. We had nearly 500,000 lines of SQL in stored procedures. And I really don't see the need for the "usp" prefix. Is it really so common to confuse a user stored procedure with a system stored procedure? At least it seems the developers that made this attempt to maintain some sort of style. At my current job, most stored procedures look like they were generated from a designer and no attempt was made to format the code. I feel your pain.
-
Hmm - lots of comments giving the reason for a change as "as suggested by manager". Doesn't inspire confidence that the person making those changes understood why they were done.
Yes, I particularly like the addition:
-- Start Added IF condition on 4/Nov/2009 as suggested by Manager.
IF (@Mh_Mfno = -2)
BEGIN
SET @Scrap_Qty = @Prod_Qty
END
-- Endwhich to me sounds like
"if I press the red button, dump all produce on the garbage heap"
:D P.S.: I wonder if one of the 'M's in
Mh_Mfnostands for 'Manager' ;) -
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
virang_21 wrote:
BEGIN TRAN Branch_One:--DECLARATION OF THE BRANCH LABEL TRUNCATE TABLE MachHistTemp
I stopped reading the code after this. A DDL statement within a transaction?
Φευ! Εδόμεθα υπό ρηννοσχήμων λύκων! (Alas! We're devoured by lamb-guised wolves!)
-
virang_21 wrote:
BEGIN TRAN Branch_One:--DECLARATION OF THE BRANCH LABEL TRUNCATE TABLE MachHistTemp
I stopped reading the code after this. A DDL statement within a transaction?
Φευ! Εδόμεθα υπό ρηννοσχήμων λύκων! (Alas! We're devoured by lamb-guised wolves!)
Actually, truncate _can_ be rolled back on SQL server, so it is not as bad as it would be if it were Oracle code.
-
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
You can refactor it. Also, I don't know for Oracle or other DBs, but there's a debugger with breakpoints, local vars, step into and all such stuff available for MySQL. OTOH, packing business logic into SQL (which I'm under the impression that it happens here) is always a bad idea - unless you really do pack your entire logic in SQL, and have stored procedures which are in fact the equivalent of services, and no or almost no logic in your client app. And yes, I do agree it's horrible the way it is now. Whoever designed the database clearly has no notion of sane naming conventions.
-
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
virang_21 wrote:
I am not saying stored proc are bad
I'm no DB expert, but stored procedures always seem to violate the "do one thing and do it well" principle. Seems to me, DBs should, I don't know, store data. Acting on that data seems to go beyond their purview. About the only case I can think of where an SP might be appropriate is to synthesize read-only data from other stored data.
-
Actually, truncate _can_ be rolled back on SQL server, so it is not as bad as it would be if it were Oracle code.
-
I have few of those in one of the project I am responsible to maintain... and sometimes something goes wrong with the system and I have oh no .. not again moment....I am not saying stored proc are bad but at times it takes you on a wild ride when figuring out what went wrong and itis absolute nightmare to debug...
-- =============================================
-- Author: XXXX
-- Create date: <06/07/2009>
-- Description: -- =============================================
ALTER PROCEDURE [dbo].[uspUpdateMfgOrdNo]
ASBEGIN
DECLARE @Machine_Id varchar(8)
DECLARE @Flt_Locn VARCHAR(8)
DECLARE @Prod_Date datetime
DECLARE @Shift_Id VARCHAR(1)
DECLARE @Item_No varchar(15)
DECLARE @Prod_Qty numeric(9,0)
DECLARE @Tot_Prod_Qty NUMERIC(9,0)
DECLARE @Mh_Mfno varchar(8)
DECLARE @Seq_Id NUMERIC(18,0)
DECLARE @WorkCenter_Id VARCHAR(8)
DECLARE @Facility_Id VARCHAR(8)
DECLARE @Matched_MfNo VARCHAR(8)
DECLARE @Available_Qty NUMERIC(9, 0)
DECLARE @CurrentQty NUMERIC(9, 0)
DECLARE @FilledQty NUMERIC(9, 0)
--DECLARE @UpperLimit_Qty NUMERIC(9, 0)
DECLARE @Cono NUMERIC(9, 0)
DECLARE @Scrap_Qty NUMERIC(9, 0)
DECLARE @RowCount INT
DECLARE @blFlagMultiMO CHAR(1)
DECLARE @MachHistTemp TABLE(Id INT,Machine_Id VARCHAR(8),Flt_Locn VARCHAR(8),Prod_Date DATETIME,
Shift_Id VARCHAR(1),Item_No VARCHAR(15),Prod_Qty NUMERIC(9,0),Mh_Mfno VARCHAR(8),
Seq_Id NUMERIC(18,0))
DECLARE @ROWCNT INT
DECLARE @CNT INTSET @blFlagMultiMO = 'F'
BEGIN TRAN
Branch_One:--DECLARATION OF THE BRANCH LABEL
TRUNCATE TABLE MachHistTemp
The size of the proc is scary enough, but here's the scariest thing in my opinion. Added IF condition on 4/Nov/2009 as suggested by Manager. Changed the Where Clause on 18/November/2009 as asked by Manager Really? What do system analysts do in this company? :doh: