开发者

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:

  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.


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

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


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.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜