Critique this SQL stored procedure.

Question

Hey guys,

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

Answer 1

It is too long.

Answer 2

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.

Answer 3

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).

Answer 4

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.

Answer 5

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.

Answer 6

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.

Answer 7

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.

Answer 8

tl;dr

seriously - What's your main gripe? Does it not perform as per the requirements? Does it need to be documented better?

Answer 9

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.

Answer 10

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

Answer 11

1. Its much too long -- I've got bored while reading it :(.

2. Formatting makes it hard to read (but this depends on my personal view and experience).

3. There are no comments, which makes it really hard to read and understand.

4. 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.

5. 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 LOCA**L -- 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.

6. Here:

   declare @show_only_new char(5);
   select @show_only_new = ISNULL(show_only_new, 'N')
   

   value of @show_only_new will be "N____", where _ is space, if value of column show_only_new is null. In the past I saw a few comparations with VARCHAR variable which introduce subtle bug in code.

7. SP creates a lot of local and temporary tables with no indexes (which is possible for temporary table). This could impact on performance.

8. Seems that values of attribute @attribute_cd are hard coded. This is not good practice...

9. 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...

Answer 12

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:

1. 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.

2. 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.

Answer 13

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.

Answer 14

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.