|
Posted by --CELKO-- on 07/31/06 01:43
Please post DDL, so that people do not have to guess what the keys,
constraints, Declarative Referential Integrity, data types, etc. in
your schema are. Sample data is also a good idea, along with clear
specifications. It is very hard to debug code when you do not let us
see it.
CREATE PROCEDURE Rolex136Sync
-- proc names are "<verb><object>" in ISO 11179
AS
DECLARE
@date VARCHAR(50), -- reserved word, non-temporal data type and
toooooooo long
@ydate VARCHAR (50) -- non-temporal data type and toooooooo long
PRINT CONVERT(CHARr(11), (GETDATE()-1), 100);
-- print is for diagnostics
-- CONVERT() is proprietary
--GETDATE() is proprietary and should be CURRENT_TIMESTAMP
-- 1950's style COBOL strings for dates !!
...
(SELECT CASE ua
WHEN 'unknown' THEN NULL ELSE ua END ) AS ua,
-- why did you put a SELECT in this?
-- did you use CASE instead if NULLIF()?
...
CASE servicename
WHEN 'aktel' THEN 'aktel'
WHEN 'qatar2900' THEN 'star multimedia 2900'
WHEN 'telemovil' THEN 'telemovil'
WHEN 'comcel' THEN 'comcel'
WHEN 'tsttnews' THEN 'tstt'
WHEN 'tsttwap' THEN 'tstt'
WHEN 'tstt_mms' THEN 'tstt mms'
WHEN 'alclickwin6464' THEN 'airtel click win 646'
WHEN 'almmsportal' THEN 'airtel mms'
WHEN 'almmssmsdwn' THEN 'airtel mms sms'
WHEN 'almyalbum646' THEN 'airtel my album'
WHEN 'hindu6397' THEN 'hindu 6397'
ELSE
(SELECT opname FROM datalogs.dbo.operator WHERE phoneseries ..)
END
-- why isn't this an auxiliary look-up table? Why only one operator in
that table?
Your code is basically 1950's COBOL written in SQL and is awful. You
are treating temporal data as insanely loooooonng strings and doing
display work in the database and not the application. You even use
PRINT in production code!
Singular table names is another COBOL thing. That language (and others
that are 40+ years old and non-relational) did record-at-a-time process
so you did bring one Operator into the program each loop. But SQL is
set-oriented and we bring many operators into the program without a
loop.
[Back to original message]
|