Lots of CTE and dynamic query

  • I have tried optimizing the query maximum, but still any inputs will help

     

     
    declare @firmID Varchar(50)

    declare @query as nvarchar(max)


    SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
    SET @query = '
    WITH CTE_Firm_Option_Field
    AS (
    SELECT
    FOF.Field_label
    , FOF.Field_used
    , FOF.Field_id
    , FOF.Field_type
    FROM dbo.Firm_Option_Field FOF WITH(NOLOCK)
    WHERE firm_id='+@firmID+Char(10) +Char(13) +'
    ),
    CTE_Firm_Roles
    AS(
    SELECT role_name , role_id
    FROM DBO.firm_roles with(nolock)
    WHERE firm_id='+@firmID+Char(10) +Char(13) +'
    ),
    CTE_FirmSearch_GridColumn
    AS(
    SELECT SortID
    , ColumnID
    FROM DBO.FirmSearch_GridColumn with(nolock)
    WHERE FirmID='+@firmID+Char(10) +Char(13) +'
    )'

    SET @query=@query +' select ColumnID,GroupName,ColumnName,ColumnHeader,ColumnMouseOver,IsDefault,IsSearchColumn,IsViewColumn,IsRasCal,SortID,ParentID,ColumnUniqueName,
    ColumnWidth,selected,ColumnType,IsMandatoryColumn
    from (select ColumnID ,GroupName, (case

    when ColumnUniqueName like ''CField_%'' then (select Field_label from CTE_Firm_Option_Field where Field_used=''Y''
    and Field_id=CAST( REPLACE(ColumnUniqueName, ''CField_'', '''') AS INT))

    when ColumnUniqueName like ''Role_%'' then r.role_name

    when ColumnUniqueName like ''Budget_Role_%'' then ''Budget For ''+ r.role_name

    when ColumnUniqueName like ''Adjusted_Role_%'' then ''Adjusted Budget For ''+r.role_name

    when ColumnUniqueName like ''Actual_Role_%'' then ''Actual For ''+r.role_name

    when ColumnUniqueName like ''Remaining_Role_%'' then ''Remaining For ''+r.role_name

    else CAST( ColumnName as VARCHAR(MAX)) end ) as ColumnName,'

    SET @query=@query +'(case
    when ColumnUniqueName like ''CField_%'' then f.Field_label

    when ColumnUniqueName like ''Role_%'' then r.role_name

    when ColumnUniqueName like ''Budget_Role_%'' then r.role_name+'' Bud''

    when ColumnUniqueName like ''Adjusted_Role_%'' then r.role_name +'' Adj Bud''

    when ColumnUniqueName like ''Actual_Role_%'' then r.role_name +'' Act''

    when ColumnUniqueName like ''Remaining_Role_%'' then r.role_name+'' Rem''

    else CAST( ColumnHeader as VARCHAR(MAX)) end ) as ColumnHeader,'

    SET @query=@query +'(case

    when ColumnUniqueName like ''CField_%'' then f.Field_label

    when ColumnUniqueName like ''Role_%'' then r.role_name

    when ColumnUniqueName like ''Budget_Role_%'' then ''Budget For ''+r.role_name

    when ColumnUniqueName like ''Adjusted_Role_%'' then ''Adjusted Budget For ''+r.role_name

    when ColumnUniqueName like ''Actual_Role_%'' then ''Actual For ''+r.role_name

    when ColumnUniqueName like ''Remaining_Role_%'' then ''Remaining For ''+r.role_name

    else CAST(ColumnMouseOver as VARCHAR(MAX)) end ) as ColumnMouseOver,'

    SET @query=@query +'IsDefault,IsSearchColumn,IsViewColumn,
    glp.IsRASCal as IsRasCal
    ,(case when (select Count(1) from CTE_FirmSearch_GridColumn where ColumnID= glp.ColumnID)>0
    then (select CAST( SortID AS SMALLINT ) from CTE_FirmSearch_GridColumn where ColumnID= glp.ColumnID)
    when ColumnID in (1,3,4,5,6,7) then glp.SortID
    else CAST( 0 as SMALLINT) end) as SortID ,
    ParentID,ColumnUniqueName,ColumnWidth,

    (case when (select Count(1) from CTE_FirmSearch_GridColumn where ColumnID= glp.ColumnID)>0 then 1
    when ColumnID in (1,3,4,5,6,7) then 1
    else 0 end) as selected,

    (CASE WHEN ColumnUniqueName LIKE ''CField%'' THEN f.Field_type
    ELSE CAST ( ''Text'' as VARCHAR(MAX)) END) ColumnType,
    (case when ColumnID in (1,3,4,5,6,7) then 1 else 0 end) IsMandatoryColumn
    from GridColumn_lp as glp with(nolock)
    left join CTE_Firm_Roles r
    on r.role_name <> '''' AND
    r.role_id = CASE
    WHEN ColumnUniqueName like ''Budget_Role_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''Budget_Role_'', '''') as INT)
    WHEN ColumnUniqueName like ''Role_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''Role_'', '''') as INT)
    WHEN ColumnUniqueName like ''Adjusted_Role_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''Adjusted_Role_'', '''') as INT)
    WHEN ColumnUniqueName like ''Actual_Role_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''Actual_Role_'', '''') as INT)
    WHEN ColumnUniqueName like ''Remaining_Role_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''Remaining_Role_'', '''') as INT)
    END
    left join CTE_Firm_Option_Field f
    ON f.Field_label <> '''' AND
    f.Field_id = CASE
    WHEN ColumnUniqueName like ''CField_%'' THEN CAST ( REPLACE(ColumnUniqueName, ''CField_'', '''') as INT)
    END
    where IsSearchColumn = 1) a where ColumnName not like ''%<Role%'' and ColumnName <>'''' '

    IF((SELECT COUNT(1) FROM DBO.firm_master WITH(NOLOCK) WHERE firm_id=CAST( @firmID as INT) AND (ras='Y' OR Cal_Schedule='Y')) < 1 )
    BEGIN
    SET @query=@query +' AND IsRasCal = 0 '
    END

    IF((SELECT Count(1) FROM DBO.firm_master with(nolock) where firm_id=CAST( @firmID as INT) and portal_service='Y') < 1)
    BEGIN

    --SET @query=@query +' AND ColumnName <> ''Portal'' '
    SET @query = @query +' AND ColumnUniqueName <> ''isPortal'' '
    END

    SET @query=@query +' order by ColumnName '
    --select @query
    exec sp_executesql @query
  • Might be a dumb question, but does that need to be dynamic SQL?

    My thoughts here are to take your last 2 IF statements and have them assign a value to a BIT variable.  Next, take all of the dynamic SQL above the last 2 IF statements and turn it into regular TSQL.  And then with your last 2 bits of the dynamic SQL, you would change the first one to be:

    AND (IsRasCal = 0 OR @FirmMasterCountCalScheduleFilter = 0)
    AND (ColumnUniqueName <> 'isPortal' OR @FirmMasterCountPortalServiceFilter = 0)

    Mind you I am not a big fan of those variable names, so I'd pick something that makes more sense.

    You also appear to be very liberal with your "NOLOCKS"... you are aware of the risks with them, right?

    But apart from those suggestions, I think I'd need to see more info like an execution plan or some DDL and sample data to be helpful.  Could be indexes could help or you may have all of the appropriate indexes...

    The above is all just my opinion on what you should do. 
    As with all advice you find on a random internet forum - you shouldn't blindly follow it.  Always test on a test server to see if there is negative side effects before making changes to live!
    I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.

  • I would like to echo Brian's comments.    Improving SQL code depends on an understanding of the data, the execution plan, the table and index structures, etc.     Given enough information, the volunteers here can give you a lot of assistance.   But there is very little anyone can do with just a piece of code and a request for "any inputs."

    By the way, in addition to the NOLOCK, hint you are also specifying a READ UNCOMMITTED.    You should know that means you may get the wrong results, which cannot be repeated.     People always want their code to be fast, but correct results are more important than speed.     NOLOCK and READ UNCOMMITTED aren't best practices.

     

    __________________________________________________

    Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
    Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills

  • yea please ignore the nolock hints. its what developer do and we always omit when we commit.

    Attaching Execution plan for one of case

    • This reply was modified 3 years, 1 month ago by  khushbu.
    Attachments:
    You must be logged in to view attached files.
  • As stated, we really need a query plan.  But this is always true: instead of "(select count(...) ...) > 0" you can just do an EXISTS check.  You don't really need to get a full count, you just need to determine if a row exists or not.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • khushbu wrote:

    yea please ignore the nolock hints. its what developer do and we always omit when we commit.

    Attaching Execution plan for one of case

    If you keep this line 'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;' in the code, then it doesn't matter if you remove the NOLOCK hints...you are still doing the same thing.

    If you have enabled RCSI - then you are deliberately changing that and avoiding the potential benefits of RCSI.  Instead of the above - you should look at using SNAPSHOT isolation (if you don't have RCSI enabled).  But - do your research and testing...there are some drawbacks to either RCSI or SNAPSHOT isolation.

    Jeffrey Williams
    “We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

    ― Charles R. Swindoll

    How to post questions to get better answers faster
    Managing Transaction Logs

  • Removing NOLOCK / UNCOMMITTED could only slow down the code, it could never speed it up.  Since OP is looking for a performance gain, removing NOLOCK is not the top priority.

    In the exec plan, if I'm reading it correctly, the row counts seem so low I don't see how it could be that slow.  Did you run it on a smaller test set of data?

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • Why are you using dynamic SQL? Why are you doing everything with metadata? Why was Dr. Codd so wrong about this AND you are so smart? You might want to look at what the term “field” means AND SQL; it refers to parts of the column, such AS the year month AND day fields within a date. Double quotes are used. To enclose nonstandard identifiers; square brackets are a local convention used by Microsoft. This has nothing to do with string values.

    One of my little checks I do is_ to look for commas in front of lines of code. We used to do this 30 years ago because we were using punch cards. A leading comma lets you rearrange the deck without having to punch a new card! This should’ve stopped in the 1970s when we got video editors.

    Why do you write with assembly language flags (column names like “is_<foobar>”)? When we created these standards, we intended for SQL to be a language based on predicates. We test to see what this current status of the schema is and then take action based on that instead of setting flags like we used to with punch cards and magnetic tapes.

    I played with your code and try to make some sense of it, but since you didn’t follow the formal rules about posting DDL or doing decent sample data, I couldn’t help you. Please look at my credentials and how much I know about SQL; I’m not just being mean to you but you are dangerously ignorant.

    Please post DDL and follow ANSI/ISO standards when asking for help. 

  • jcelko212 32090 wrote:

    ... Double quotes are used.

    Where?  There are some pairs of single quotes that are needed in the dynamic SQL (which as others point out, might be changed).  But no double quotes.

    ... Please look at my credentials and how much I know about SQL; I’m not just being mean to you but you are dangerously ignorant.

    Do you think you will impress this user with your tone?  It sounds like politicians that regularly insult the electorate.

  • Was just reviewing the execution plan, and my thoughts are (experts feel free to tell me I'm wrong here):

    1 - the "SELECT(COUNT()...)" statements are checking if the result is less than 1.  Equality comparisons are generally faster than range or inequality, and since a count can never be less than 0, doing an equality comparison (ie =0) should give a slight performance boost.  That being said, as ScottPletcher said, an exists check should be even more efficient.  Doing an "IF NOT EXISTS (SELECT 1 FROM ... )" should get you better performance than the COUNT approach.

    2 - Looking at the plan, you have a few warnings, but nothing that looks like it should be causing drastic performance issues.  There is a SCAN that could probably be SEEK with better indexes, but at the same time, it appears to be on small tables.  BUT since you are asking for performance tuning advice here, getting that scan to seek instead would be a performance boost.

    3 - you would probably get a slight performance boost by storing @firmID as an INT and not doing the CAST on it in your IF statements.  CAST/CONVERT will slow things down slightly, so if you can reduce the number of datatype changes you need, you should get a slight performance boost.

    4 - It looks like your statistics may be out of date.  Not a lot of rows being estimated wrong - estimated 122, actual 121 in some cases, some are exactly the same, and others still are quite a bit different - estimated 122, actual 7, estimated 105, actual 18.

    5 - Depending on where this data is being used, you MAY benefit by sorting the data in the application side.  Ordering data is a slow-ish operation, and if the order isn't important OR it MAY be reordered on the application side, don't order it in SQL.

    Looking at the execution plan, if I am reading it right, it completed in nearly 0 seconds. The actual XML with largest time: <QueryTimeStats CpuTime="4" ElapsedTime="7" /> which I am pretty sure is saying the Elapsed Time is 7 ms.  The other 2 lines with the Query Time Stats field show 0 for CPU time and Elapsed time.  How much faster do you need it to run?

    The above is all just my opinion on what you should do. 
    As with all advice you find on a random internet forum - you shouldn't blindly follow it.  Always test on a test server to see if there is negative side effects before making changes to live!
    I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.

Viewing 10 posts - 1 through 9 (of 9 total)

You must be logged in to reply to this topic. Login to reply