Critique this SQL stored procedure
below is a massive stored procedure that a contract developer wrote for me, I feel like I am picking on the developer but it is just terrible. What are the main issues you can see with it?
CREATE PROCEDURE [dbo].[usp_SHS_XXXX]
(
@request_identifier varchar(255),
@category_guids varchar(4000),
@url varchar(500),
@section_id int,
@NoOfDaysToRank int = 45,
@SaleRankWeight decimal(18,2) = 1.0,
@QuantityRankWeight decimal(18,2) = 1.0,
@NewItemWeight decimal(18,2) = 0.2
)
AS
BEGIN
SET NOCOUNT ON
declare @tblCatAttrFilteredPhysicalItems table
(
[request_identifier] [uniqueidentifier] NULL,
[item_guid] [uniqueidentifier] NULL,
[item_cd] [varchar](25) NULL,
[master_guid] [uniqueidentifier] NULL,
[item_type_id] [int] NULL,
[item_description_title] [varchar](100) NULL,
[item_description_short] [varchar](255) NULL,
[item_description] [varchar](3000) NULL,
[item_retail_price] [decimal](18, 2) NULL,
[item_sale_price] [decimal](18, 2) NULL,
[item_backorderable] [int] NULL,
[item_discontinued] [int] NULL,
[item_available] [int] NULL,
[item_catalog_guid] [uniqueidentifier] NULL,
[item_image_counter] [int] NULL,
[item_color] [varchar](255) NULL,
[item_gender] [varchar](255) NULL,
[item_age_group] [varchar](255) NULL,
[item_price_updated] [varchar](25) NULL,
[item_licensor] [varchar](255) NULL,
[item_manufacturer] [varchar](255) NULL,
[item_primary_license] [varchar](255) NULL,
[item_series] [varchar](255) NULL,
[item_size] [varchar](255) NULL,
[item_associated_hero] [varchar](255) NULL,
[item_tshirt_attributes] [varchar](255) NULL,
[item_date_counted] [varchar](25) NULL,
[item_hide_from_search] [varchar](255) NULL,
[item_holiday] [varchar](255) NULL,
[item_material] [varchar](255) NULL,
[item_newphoto_needed] [varchar](255) NULL,
[item_sleeve_type] [varchar](255) NULL,
[item_softness] [varchar](255) NULL,
[item_created] [datetime] NULL,
[item_approved] [int] NULL,
[item_quantity_on_hand] [int] NULL,
[item_quantity_on_hold] [int] NULL,
[item_quantity_on_order] [int] NULL,
[item_weight] [decimal](18,2) NULL,
[is_item_new] [varchar](5) NULL,
[category_guid] [varchar](255) NULL,
[category_name] [varchar](50) NULL,
[item_quantity] [int],
[item_price_adjustment_value] [decimal](16,6),
[item_control_link] [nvarchar](500),
[item_promotion] [nvarchar](255),
[item_price_adjustment] [decimal](16,6)
)
declare
@attribute_cd varchar(255),
@attribute_value varchar(6000),
@tmpAttrValue varchar(255),
@first_section_id int;
declare @primary_license varchar(255);
select @primary_license = primaryLicense from pageData
where url = @url;
declare curSecAttr cursor for
select attribute_cd,attribute_value from shs_page_section_attributes
where url = @url and section_id = @section_id;
declare @show_only_new char(5);
select @show_only_new = ISNULL(show_only_new, 'N')
from shs_page_sections
where url = @url
AND section_id = @section_id;
insert into @tblCatAttrFilteredPhysicalItems
select
@request_identifier,i.*
from mf_item_detail i
inner join mf_item_detail mi
on i.master_guid = mi.item_guid
inner join mf_item_categories ic
on mi.item_guid = ic.item_guid
where ic.category_guid in (select item from dbo.udfSplit(@category_guids,'#'))
and mi.item_image_counter > 0
and mi.item_discontinued = 0
and mi.item_type_id = 3
and i.item_quantity > 0
UNION
select
@request_identifier,i.*
from mf_item_detail i
inner join mf_item_categories ic
on i.item_guid = ic.item_guid
where ic.category_guid in (select item from dbo.udfSplit(@category_guids,'#'))
and i.item_image_counter > 0
and i.item_discontinued = 0
and i.item_type_id = 1
and i.master_guid IS NULL
and i.item_quantity > 0;
select * into #tmpCatAttrFilteredItems from @tblCatAttrFilteredPhysicalItems;
update #tmpCatAttrFilteredItems
set item_color = mi.item_color,
item_gender = mi.item_gender,
item_holiday = mi.item_holiday,
item_age_group = mi.item_age_group,
item_licensor = mi.item_licensor,
item_manufacturer = mi.item_manufacturer,
item_material = mi.item_material,
item_primary_license = mi.item_primary_license,
item_series = mi.item_series,
item_sleeve_type = mi.item_sleeve_type,
item_softness = mi.item_softness,
item_tshirt_attributes = mi.item_tshirt_attributes,
item_promotion = mi.item_promotion
from mf_item_detail mi
where mi.item_guid = #tmpCatAttrFilteredItems.master_guid
and #tmpCatAttrFilteredItems.item_type_id = 1
and #tmpCatAttrFilteredItems.master_guid is not null
and request_identifier = @request_identifier;
select * into #tblTmpResultSet from #tmpCatAttrFilteredItems where 1 = 2;
open curSecAttr;
fetch curSecAttr into @attribute_cd,@attribute_value;
while(@@FETCH_STATUS = 0)
begin
if(@attribute_cd = 'AssociatedHero')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_associated_hero,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_associated_hero,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Color')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_color,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_color,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier; ;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Gender')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_gender,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_gender,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Holiday')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_holiday,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_holiday,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Promotion')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_promotion,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_promotion,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'IntendedAgeGroup')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_age_group,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_age_group,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Licensor')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_licensor,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_licensor,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Manufacturer')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_manufacturer,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_manufacturer,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Material')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_material,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_material,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'PrimaryLicense')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_primary_license,','))
and request_identifier = @request_identifier;
开发者_开发问答 fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_primary_license,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Series')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_series,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_series,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Size')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_size,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_size,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'SleeveType')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_sleeve_type,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_sleeve_type,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'Softness')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_softness,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_softness,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
if(@attribute_cd = 'T-ShirtAttributes')
begin
if (charIndex(',',@attribute_value) > 0)
begin
declare cur_AttrValue cursor for
select item from dbo.udfSplit(@attribute_value,',');
open cur_AttrValue;
fetch cur_AttrValue into @tmpAttrValue;
while (@@FETCH_STATUS = 0)
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @tmpAttrValue in (select item from dbo.udfSplit(item_tshirt_attributes,','))
and request_identifier = @request_identifier;
fetch cur_AttrValue into @tmpAttrValue;
end
close cur_AttrValue;
deallocate cur_AttrValue;
end
else
begin
insert into #tblTmpResultSet
select * from #tmpCatAttrFilteredItems
where @attribute_value in (select item from dbo.udfSplit(item_tshirt_attributes,','))
and request_identifier = @request_identifier;
end
delete from #tmpCatAttrFilteredItems where request_identifier = @request_identifier;
insert into #tmpCatAttrFilteredItems
select * from #tblTmpResultSet where request_identifier = @request_identifier;
delete from #tblTmpResultSet where request_identifier = @request_identifier;
end
fetch curSecAttr into @attribute_cd,@attribute_value;
end
close curSecAttr;
deallocate curSecAttr;
declare @tblFilteredItems table
(
request_identifier uniqueidentifier,
item_guid varchar(255),
item_cd varchar(25),
master_guid varchar(255),
item_quantity int,
item_created datetime,
item_approved int,
sale_rank decimal(18,2) NULL,
qoh_rank decimal(18,2) NULL,
new_item_flag int NULL,
item_rank decimal(18,2) NULL,
is_item_new varchar(5)
);
declare @tblFilteredOrderItems table
(
request_identifier uniqueidentifier,
item_guid varchar(255),
item_cd varchar(25),
master_guid varchar(255),
item_quantity int,
order_item_quantity int,
order_item_quantity_fulfilled int,
item_created datetime,
item_approved int,
sale_rank decimal(18,2) NULL,
qoh_rank decimal(18,2) NULL,
new_item_flag int NULL,
item_rank decimal(18,2) NULL,
is_item_new varchar(5)
);
declare @tblFinalResultItems table
(
request_identifier uniqueidentifier,
item_guid varchar(255),
item_cd varchar(25),
master_guid varchar(255),
item_quantity int,
order_item_quantity int,
order_item_quantity_fulfilled int,
item_created datetime,
item_approved int,
sale_rank numeric(18,5) NULL,
qoh_rank numeric(18,5) NULL,
new_item_flag int NULL,
item_rank numeric(18,5) NULL,
is_item_new varchar(5)
);
declare
@TotNoOfItems int,
@MaxOrderQuantity int,
@MaxQtyOnHand int;
insert into @tblFilteredOrderItems
(request_identifier,item_guid,item_cd,master_guid,item_quantity,order_item_quantity,order_item_quantity_fulfilled,item_created,item_approved,is_item_new)
select
distinct @request_identifier,
oi.item_guid,
min(i.item_cd) as item_cd,
i.master_guid as master_guid,
sum(i.item_quantity) as item_quantity,
sum(oi.order_item_quantity) as order_item_quantity,
sum(oi.order_item_quantity_fulfilled) as order_item_quantity_fulfilled,
MIN(i.item_created) as item_created,
avg(i.item_approved) as item_approved,
min(i.is_item_new) as is_item_new
from mf_order_items oi
inner join #tmpCatAttrFilteredItems i
on i.item_guid = oi.item_guid
inner join mf_orders o
on o.order_guid = oi.order_guid
where o.order_date between getdate()-@NoOfDaysToRank and getdate()
and i.master_guid is NULL
group by oi.item_guid,i.master_guid
UNION
select
distinct @request_identifier,
mi.item_guid,
min(mi.item_cd) as item_cd,
mi.master_guid as master_guid,
sum(i.item_quantity) as item_quantity,
sum(oi.order_item_quantity) as order_item_quantity,
sum(oi.order_item_quantity_fulfilled) as order_item_quantity_fulfilled,
MIN(mi.item_created) as item_created,
min(mi.item_approved) as item_approved,
min(mi.is_item_new) as is_item_new
from mf_order_items oi
inner join #tmpCatAttrFilteredItems i
on i.item_guid = oi.item_guid
inner join mf_item_detail mi
on mi.item_guid = i.master_guid
inner join mf_orders o
on o.order_guid = oi.order_guid
where o.order_date between getdate()-@NoOfDaysToRank and getdate()
and i.master_guid is NOT NULL
group by oi.item_guid,mi.item_guid,mi.master_guid;
insert into @tblFilteredItems
(request_identifier,item_guid,item_cd,master_guid,item_quantity,item_created,item_approved,is_item_new)
select
distinct @request_identifier,
item_guid,
item_cd as
Well, just read it very quickly, but 2 things come immediately to mind:
- Break it down into smaller procedures! This will make maintanence a lot easier.
- It uses cursors. Maybe there's a way to solve the same things without cursors. IMHO, cursors should be avoided if possible (particularly because cursos go against the set-based approach of SQL Server and because they are damn slow).
In addition to the existing answers, I'd like to add something that I'm surprised no-one has pointed out yet:
- There are no comments.
It is too long.
As a general principal, T-SQL is a declarative language i.e. you tell it what you want, and SQL Server (I assume) works out how to get the results.
My main criticism is not "Length", "Cursors" (per se, although they are #1 symptom of the problem) or "Slowness" (don't have a data set so can't say).
The procedure is written imperatively - it is telling SQL exactly what to do, in which order, to get the desired results much as you would write a program in C# etc. I see cursors, and I think: this programmer has not fully grasped the set-based nature of SQL. This causes at least two problems:
Readability - this is not just a function of length, but also of style. If it was broken down functionally (views, CTEs) it would be clearer how the data sets were being built up and combined towards the final result. This is helped in no way by the inexcusable lack of comments.
Performance will be harder to tune because you are not allowing the SQL optimizer to work its magic - look at row counts, join orders etc. and rearrange the method to get you the results in the most efficient way. You're giving the engine no choice in how it performs the work. That's not to say it gets it right 100% of the time, and there isn't need for specific tuning, caching, hinting etc. but it's easier to start from a functional, declarative version and tune from there.
Its much too long -- I've got bored while reading it :(.
Formatting makes it hard to read (but this depends on my personal view and experience).
There are no comments, which makes it really hard to read and understand.
I cannot find the source, but large plans of execution (and definitely this huge stored procedure could have a big execution plan) are not cached, which means that each execution causes new recompilation and could lead to performance issue.
Here:
declare curSecAttr cursor for select attribute_cd,attribute_value from shs_page_section_attributes where url = @url and section_id = @section_id;cursor is not LOCAL -- this could lead to errors if two SP will run at the same time -- details are described at [MSDN: Scope of Transact-SQL Cursor Names](http://msdn.microsoft.com/en-us/library/ms189238(lightweight).aspx).
Here:
declare @show_only_new char(5); select @show_only_new = ISNULL(show_only_new, 'N')value of
@show_only_newwill be "N____", where_is space, if value of columnshow_only_newis null. In the past I saw a few comparations with VARCHAR variable which introduce subtle bug in code.SP creates a lot of local and temporary tables with no indexes (which is possible for temporary table). This could impact on performance.
Seems that values of attribute
@attribute_cdare hard coded. This is not good practice...Seems that there are a lot of duplicated code, which should be refactor (i.e. split into more smaller stored procedures).
There are probably a lot of other issues and flows hidden inside this long piece of code, which is the next bad thing about it...
Do you want us to agree with you that it's terrible?
If it does what you wanted, then I don't see what the issue is.
I worry about your contractor's client not knowing enough SQL to generate a stored procedure himself, or buying the cheapest contractor he could find and then whining when he got what he paid for.
tl;dr
seriously - What's your main gripe? Does it not perform as per the requirements? Does it need to be documented better?
All those IF blocks look repetitive. I suspect that could all be rewritten as a single statement, maybe without the cursors. But if performance isn't an issue, and it works, I don't see anything glaringly wrong with this. As written, it is very long, but makes use of table variables, so it would be awkward to break it into smaller procedures
The procedure name is appalling, what does it do?
There are too many parameters, as a guideline 7 parameters is good limit.
There are no comments.
The use of cursors probably suggested that the author does really understand database programming.
There is no error handling.
It might make sense to split it into several smaller stored procedure, although I suspect getting rid of the cursors and a few comments would make it much easier to understand.
Based on a quick review, the first thing I would question is the massive set of IF blocks. They all look similar and should be consolidated. Also, just generally speaking, it is ssssssssoooooooooooo long.
I also don't like using If statements that are case sensitive. Too easy to mess things up just because you forgot to capitalize the first letter in some setting.
A critique?
It is ugly and unmaintainable. It is too long. It is poorly structured. It looks like it was banged out quickly in one session. It looks like it would run very slow. It looks like it will break extremely easily.
Check the database design, it seems that it's not in 3rd Normal Form. Some attributes are multivalued and on top of that they are stored comma-separated. Not good! This makes the procedure suck a bit more.
Furthermore, there are about 13 paragraphs (one for each attribute) that are 90% structurally identical except for the first parameter of udfSplit. Refactor them and the procedure size (and complexity) will decrease dramatically.
Your company seems to be hiring the wrong people. One who writes such long, uncommented, cursory stored procs. Who in turn are managed by people who post the procs on the internet to find out what is wrong with it. Seriously.
加载中,请稍侯......
精彩评论