4

Am trying to optimize a procedure. There are 3 different update queries present in the procedure. 

update #ResultSet
set MajorSector = case 
        when charindex('  ', Sector) > 2 then rtrim(ltrim(substring(Sector, 0, charindex('  ', Sector)))) 
            else ltrim(rtrim(sector)) 
        end

update #ResultSet
set MajorSector = substring(MajorSector, 5, len(MajorSector)-4)
where left(MajorSector,4) in ('(00)','(01)','(02)','(03)','(04)','(05)','(06)','(07)','(08)','(09)')

update #ResultSet
set MajorSector = substring(MajorSector, 4, len(MajorSector)-3)
where left(MajorSector,3) in ('(A)','(B)','(C)','(D)','(E)','(F)','(G)','(H)','(I)','(J)','(K)','(L)','(M)','(N)','(O)','(P)','(Q)','(R)','(S)','(T)','(U)','(V)','(W)','(X)','(Y)','(Z)')

To complete all three update queries it takes less than 10 seconds.

Execution plan for all three update queries.

https://www.brentozar.com/pastetheplan/?id=r11BLfq7b

What I planned is to change the three different update queries into one single update query, so that the I/O can be reduced. 

;WITH ResultSet
     AS (SELECT CASE
                  WHEN LEFT(temp_MajorSector, 4) IN ( '(00)', '(01)', '(02)', '(03)', '(04)', '(05)', '(06)', '(07)', '(08)', '(09)' ) 
                      THEN Substring(temp_MajorSector, 5, Len(temp_MajorSector) - 4)
                  WHEN LEFT(temp_MajorSector, 3) IN ( '(A)', '(B)', '(C)', '(D)','(E)', '(F)', '(G)', '(H)','(I)', '(J)', '(K)', '(L)','(M)', '(N)', '(O)', '(P)','(Q)', '(R)', '(S)', '(T)','(U)', '(V)', '(W)', '(X)','(Y)', '(Z)' ) 
                      THEN Substring(temp_MajorSector, 4, Len(temp_MajorSector) - 3)
                  ELSE temp_MajorSector
                END AS temp_MajorSector,
                MajorSector
         FROM   (SELECT temp_MajorSector = CASE
                                             WHEN Charindex('  ', Sector) > 2 THEN Rtrim(Ltrim(Substring(Sector, 0, Charindex('  ', Sector))))
                                             ELSE Ltrim(Rtrim(sector))
                                           END,
                        MajorSector
                 FROM   #ResultSet)a)
UPDATE ResultSet
SET    MajorSector = temp_MajorSector

But this takes around 1 minute to complete. I checked the execution plan, it is identical as first update query.

Execution plan for above query:

https://www.brentozar.com/pastetheplan/?id=SJvttz9QW

Can somebody explain why it is slow?

Dummy data for testing:

If object_id('tempdb..#ResultSet') is not null
drop table #ResultSet


;WITH lv0 AS (SELECT 0 g UNION ALL SELECT 0)
    ,lv1 AS (SELECT 0 g FROM lv0 a CROSS JOIN lv0 b) -- 4
    ,lv2 AS (SELECT 0 g FROM lv1 a CROSS JOIN lv1 b) -- 16
    ,lv3 AS (SELECT 0 g FROM lv2 a CROSS JOIN lv2 b) -- 256
    ,lv4 AS (SELECT 0 g FROM lv3 a CROSS JOIN lv3 b) -- 65,536
    ,lv5 AS (SELECT 0 g FROM lv4 a CROSS JOIN lv4 b) -- 4,294,967,296
    ,Tally (n) AS (SELECT ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) FROM lv5)
SELECT CONVERT(varchar(255), NEWID()) as Sector,cast('' as varchar(1000)) as MajorSector
into #ResultSet
FROM Tally
where  n <= 242906 -- my original table record count
ORDER BY n;

Note : Since this is not my original data the timings I have mentioned above could be little different. Still the single update query is much slower than the first three.

I tried executing the queries more than 10 times to make sure external factors should not affect the performance. All 10 times first three updates ran much faster than the last single update.

Improve this question

3 Answers 3

Reset to default
8

The first single update reads and writes every row from the table. The second and third then re-read and re-write a sub-set of those rows. Look at the Actual Number of Rows. When the three statements are combined into one, the optimizer figures that if it has to read everything to satisfy the first change then it can piggy-back off that for the second and third change.

Have a look at the XML version of the query plans, specifically the <ComputeScalar> operators and <ScalarOperator ScalarString=""> parts. In the original plan you'll see each is relatively simple and maps very closely to the SQL. For the all-in-one plan it's a monster. This is the optimizer re-writing the SQL into a logically equivalent form. A plan works1 by passing each row through the operators once. The optimizer's doing all the work it has to do to satisfy all three changes as that row passes through one time.

I'd expect the second query to be faster because the data is only read and written once whereas it is touched three times in the first.

As the second query has no predicates (no WHERE clause) the optimizer has no choice but read every single row and process it. I'm surprised the second form takes longer than the first. Are both starting from clean buffers? Is there other work happening on the system? Since it's reading and writing to a temp table the IO is happening in tempdb. Is there file growth or somesuch happening?

By one measure you have achieved your desired outcome. You say you want to make changes "so that the IO can be reduced." The all-in-one does less IO than the three separate statements do in total. I suspect what you really want, however, is reduced elapsed time, and this is obviously not happening.

1 more or less, lots of details omitted.

I ran your routine to generate test data then ran the three single-update statements and the all-in-one statement. Although there are some differences (no clustered index, no parallelism) I get more-or-less the same results. Specifically, the plans are about the same shape and the three individual queries complete in about two seconds and the one big query in about thirty to thirty five seconds.

I set

set nocount off;
set statistics io on;
set statistics time on;

With the plan in cache and the data in memory I get:

SQL Server parse and compile time: 
   CPU time = 0 ms, elapsed time = 0 ms.
Table '#ResultSet...'. Scan count 1, logical reads 125223, physical reads 0

 SQL Server Execution Times:
   CPU time = 1422 ms,  elapsed time = 1417 ms.

(242906 row(s) affected)

Table '#ResultSet...'. Scan count 1, logical reads 125223, physical reads 0

 SQL Server Execution Times:
   CPU time = 344 ms,  elapsed time = 337 ms.

(0 row(s) affected)

Table '#ResultSet...'. Scan count 1, logical reads 125223, physical reads 0

 SQL Server Execution Times:
   CPU time = 734 ms,  elapsed time = 747 ms.

(0 row(s) affected)

I've removed some bits that aren't relevant. Since physical reads is zero for all three the table fits in memory. logical reads is the same for all three which makes sense. As there are no indexes the only approach is to scan every row of the table. The second and third query affect zero rows because I'd run them a few times already. CPU time and elapsed work out as 2500ms.

For the bigger query it is

SQL Server parse and compile time: 
   CPU time = 0 ms, elapsed time = 0 ms.
Table '#ResultSet...'. Scan count 1, logical reads 125223

 SQL Server Execution Times:
   CPU time = 33093 ms,  elapsed time = 33137 ms.

(242906 row(s) affected)

The same number of pages are read, the same number of rows are updated. The huge differences is the CPU time. This is reflected in casual observation of Task Manager which shows 30% utilisation for the duration of query execution. The question is, why does it take so much?

The individual queries separately have simple calculations and two of the statements have predicates that greatly reduce the number of rows touched. The optimizer has good heuristics for processing these and finds a quick plan. The all-in-one query applies the monster Compute Scalar against every single row. My suggestion is that, for whatever reason, the optimizer cannot unravel the logic into a plan that's quick to run and ends up using a lot of CPU. The optimizer has to work with what its given, which in the second case is complex, nested SQL. Perhaps by refactoring the SQL the optimizer will follow different heuristics and achieve a better outcome? Perhaps some (filtered) indexes or (filtered) statistics will convince it to write a different plan. Maybe persisted computed columns would help it along. Perhaps you just need to give the optimizer what it needs and your first attempt really is the best that can be achieved and you need to find a way to run those three in parallel. Sorry I can't be more scientific.

Improve this answer
0
7
+50

Please be more careful with your test data in the future. The query plans indicate that you have a clustered index on your table but your temp table does not have a clustered index. That can make a big difference in some cases. On my machine the three UPDATE approach runs in 3 seconds and the single UPDATE approach runs in 5 seconds. Not close to the difference that you see, but it still seems somewhat counterintuitive. Shouldn't the single UPDATE be faster?

As Michael Green pointed out in his answer, the problem here is with the compute scalar operator. The query optimizer isn't very good at estimating costs for compute scalars. The query plans for the first update of the set of three and the second solo update may look identical, but there's a big difference in how much work the compute scalar does. We can actually take the code and make a few changes to turn it into a valid SELECT query. The query is huge and the full code is here. Below is a greatly abbreviated version:

SELECT
  (CONVERT(varchar(1000), CASE
    WHEN SUBSTRING(CASE
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END, (1), (4)) = '(09)' OR
...
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END, (1), (4)) = '(00)' THEN SUBSTRING(CASE
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END, (5), LEN(CASE
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END) - (4))
    ELSE CASE
        WHEN SUBSTRING(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END, (1), (3)) = '(Z)' OR
...
          END, (1), (3)) = '(B)' OR
          SUBSTRING(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END, (1), (3)) = '(A)' THEN SUBSTRING(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END, (4), LEN(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END) - (3))
        ELSE CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END
      END
  END, 0))
FROM [#ResultSet];

All of those repeated calculations in the CASE statements aren't good. As part of the compute scalar SQL Server may run the same calculations over and over again. If I run that as just a SELECT it takes around 3 seconds, which is the time for the first set of UPDATE queries.

Putting repeated scalar calculations in an APPLY derived table can often improve readability of queries. In some cases it can dramatically improve performance as well. I took that large query and simplified it a bit by moving repeated expressions to an APPLY derived table. Further simplifications are possible but this should give you the basic idea:

SELECT
  (CONVERT(varchar(1000), CASE
    WHEN a.sub4 IN ('(09)','(08)','(07)','(06)','(05)','(04)','(03)','(02)','(01)','(00)') 
        THEN SUBSTRING(CASE
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN a.r_trim
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END, (5), LEN(CASE
        WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN a.r_trim
        ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
      END) - (4))
    ELSE CASE
        WHEN a.sub3 IN ('(Z)','(Y)','(X)','(W)','(V)','(U)','(T)','(S)','(R)','(Q)','(P)','(O)','(N)','(M)','(L)','(K)','(J)','(I)','(H)','(G)','(F)','(E)','(D)','(C)','(B)','(A)')
        THEN SUBSTRING(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN a.r_trim
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END, (4), LEN(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN a.r_trim
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END) - (3))
        ELSE CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN a.r_trim
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END
      END
  END, 0))
FROM [#ResultSet]
OUTER APPLY 
(
    SELECT SUBSTRING(CASE
            WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
            ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
          END, (1), (4))
    , SUBSTRING(CASE
                WHEN CHARINDEX('  ', [#ResultSet].[Sector]) > (2) THEN RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
                ELSE LTRIM(RTRIM([#ResultSet].[Sector]))
              END, (1), (3))
    , RTRIM(LTRIM(SUBSTRING([#ResultSet].[Sector], (0), CHARINDEX('  ', [#ResultSet].[Sector]))))
) a (sub4, sub3, r_trim);

Now the SELECT query runs in less than 1 second. I used OUTER APPLY so that SQL Server would calculate everything in the APPLY derived table once for each row instead of collapsing it into the compute scalar. The compute scalar is still in the query plan but it does much less work than before:

estimated APPLY

If I plug in that code into the CTE for your UPDATE query I get the following performance numbers:

CPU time = 2125 ms, elapsed time = 2134 ms.

That is a little faster than the original set of three queries:

CPU time = 1734 ms, elapsed time = 1735 ms

CPU time = 187 ms, elapsed time = 197 ms.

CPU time = 343 ms, elapsed time = 368 ms.

It may be possible to optimize the solo query further but I will leave that up to you.

Improve this answer
1

It appears to me that the second and third queries could be rewritten using these formulas:

MajorSector LIKE '(0[0-9])%'

MajorSector LIKE '([A-Z])%'

However, that doesn't much help if MajorSector is not indexed (and in a temporary table, it isn't indexed) or if these constitute every row in the table.

However:

  1. If the term '(0n)' (n = digit) or '(n)' (n = letter) only occurs at the start of the string, then you could operate on all MajorSector:

    REPLACE(REPLACE(REPLACE(MajorSector, '(00)', ''), '(01)', ''), '(02)', '')

    But with a lot more REPLACE's (36).

    However, this will turn '(00)A(01)B(02)C', into 'ABC' - not what is wanted. If that data does not occur, then, consider it.

  2. If every MajorSector starts with '(0n)' or '(n)' to be removed - or contains no ')' at all - then really you just need, 

    SUBSTRING(MajorSector, CHARINDEX(')', MajorSector)+1, maxLength)

    where maxLength is the defined length of MajorSector e.g. varchar(255). If the length parameter in SUBSTRING() is longer than the actual data, SUBSTRING() is content to return the data from the indicated offset up to the end of the string.

    SELECT SUBSTRING('ABCD', 3, 4) -- produces CD

SELECT SUBSTRING('ABCD', 0, 4) -- produces ABC, confusing!
Improve this answer
1
  • 2
    ‘and in a temporary table, it isn't indexed’ – I'd agree that it's reasonable to assume it may not be indexed, but you can't be certain it's not.
    – Andriy M
    Jun 24, 2017 at 13:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.