0

I have been looking at this statement for the last day but I've been unable to work out the best way to improve it. So, I thought I would reach out to the wider community for some advise.

SELECT [FLD6]
FROM (
 SELECT TOP (25) [EVENTS].[EV_SEQ] AS [FLD6]
 FROM [EVENTS] WITH (NOLOCK)
  LEFT OUTER JOIN [TASKS] AS [Pe2da3fbc7e676e2b21201605598f3] WITH (NOLOCK) ON [EVENTS].[EV_FKEY_TA_SEQ] = [Pe2da3fbc7e676e2b21201605598f3].[TA_SEQ]
  LEFT OUTER JOIN [TASKS] AS [Pd361fcc2673fdd885d288ab24f6f5] WITH (NOLOCK) ON [EVENTS].[EV_FKEY_TA_SEQ] = [Pd361fcc2673fdd885d288ab24f6f5].[TA_SEQ]
 WHERE (
   (
    [EVENTS].[EV_SEQ] > 0
    OR [EVENTS].[EV_SEQ] IS NULL
    )
   )
  AND (
   [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] = 0
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] IS NULL
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] = ''
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] IN (
    SELECT [ContractGroupsContract].[CTR_SEQ]
    FROM [ContractGroupsContract]
    INNER JOIN [AccountContractGroups] ON [AccountContractGroups].[ContractGroupId] = [ContractGroupsContract].[ContractGroupId]
    WHERE [AccountContractGroups].[AccountId] = 2536
     AND [AccountContractGroups].[Deleted] = 0
     AND [ContractGroupsContract].[Deleted] = 0
    )
   )
 ORDER BY [EVENTS].[EV_SEQ] ASC
 ) [EVENTS]

Here is the Execution Plan: https://www.brentozar.com/pastetheplan/?id=H1zF0Dpqi

I have made sure that all statistics are up to date and all indexes are not massively fragmented. I also don't know why the application generates two LEFT JOINS on the TASKS table, that's something that I'm liaising with the Development team about.

The query originally took around 8 minutes to execute, until I added an index on the EVENTS table, key value EV_SEQ, EV_FKEY_TA_SEQ. Now it completes in around 5 minutes, but it's still not acceptable.

The database itself has LEGACY_CARDINALITY_ESTIMATOR switched on. I disabled this option on the database and it doesn't seem to have effected the performance at all, so I have rolled it back for now.

SQL Edition: SQL Server 2016 Standard

Index Definitions in Use

Events = [2 KEYS] EV_SEQ {int 4}, EV_FKEY_TA_SEQ {int 4}

Tasks = [3 KEYS] TA_SEQ {int 4}, TA_FKEY_CTR_SEQ {int 4}, TA_DATE {datetime 8}

ContractGroupsContract = [3 KEYS] CTR_SEQ {int 4}, ContractGroupId {bigint 8}, Deleted {tinyint 1}

AccountContractGroups = [4 KEYS] AccountId {bigint 8}, Deleted {tinyint 1}, ContractGroupId {bigint 8}, AccountContractGroupId {bigint 8}

Any thoughts on what could be done here to improve this further?

Thanks!

1
  • Why do you have 2 joins to the same table? This could be valid use but it seems that you are only using one of the joined instances of the table in the rest of the query Jan 13, 2023 at 14:15

3 Answers 3

1

To me it seems the TOP 25 clause is confusing the SQL Server engine. Note that the estimates in the whole execution plan are relatively close to 25.

One way to make the query quicker is to split the query into two using temp table. Something like this should finish in seconds:

SELECT [EVENTS].[EV_SEQ] AS [FLD6]
INTO #t
 FROM [EVENTS] WITH (NOLOCK)
  LEFT OUTER JOIN [TASKS] AS [Pe2da3fbc7e676e2b21201605598f3] WITH (NOLOCK) ON [EVENTS].[EV_FKEY_TA_SEQ] = [Pe2da3fbc7e676e2b21201605598f3].[TA_SEQ]
  LEFT OUTER JOIN [TASKS] AS [Pd361fcc2673fdd885d288ab24f6f5] WITH (NOLOCK) ON [EVENTS].[EV_FKEY_TA_SEQ] = [Pd361fcc2673fdd885d288ab24f6f5].[TA_SEQ]
 WHERE (
   (
    [EVENTS].[EV_SEQ] > 0
    OR [EVENTS].[EV_SEQ] IS NULL
    )
   )
  AND (
   [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] = 0
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] IS NULL
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] = ''
   OR [Pd361fcc2673fdd885d288ab24f6f5].[TA_FKEY_CTR_SEQ] IN (
    SELECT [ContractGroupsContract].[CTR_SEQ]
    FROM [ContractGroupsContract]
    INNER JOIN [AccountContractGroups] ON [AccountContractGroups].[ContractGroupId] = [ContractGroupsContract].[ContractGroupId]
    WHERE [AccountContractGroups].[AccountId] = 2536
     AND [AccountContractGroups].[Deleted] = 0
     AND [ContractGroupsContract].[Deleted] = 0
    )
   )

SELECT TOP 25 [FLD6]
FROM #t
ORDER BY FLD6 ASC
3
  • Just ran some tests using the above code, and completed in around 2 minutes. Also tested by removing the TOP 25...ORDER BY sections and it then took 9 minutes!
    – Tom
    Jan 12, 2023 at 14:47
  • @Tom You could check the new execution plan, if there are any loop joins left. If yes, you could try adding OPTION(HASH JOIN) just to see, if this has any effect on execution time.
    – rois
    Jan 12, 2023 at 15:35
  • I've done some more testing, with both 'HASH' and 'MERGE' joins, query time drops to 3 minutes now, however this introduces an expensive sort operator that spills to TempDB. The plan also goes Parallel too. Here is the new plan for reference; brentozar.com/pastetheplan/?id=H1Ut250qi
    – Tom
    Jan 13, 2023 at 8:59
1

I think the issue is the general complexity in the way the code is written, especially with the predicates themselves. Predicates that use a lot of OR conditions are known to be problematic at times, for most database systems.

Ultimately you should clean your data, as most of the OR clauses you have appear to account for the same thing, a lack of an actual value: 0, NULL, and an empty-string. But in the meantime you generally can use TempTables and the UNION clause to break up and simplify the code, like so:

-- Materialize the ContractGroupContract CTRs to use for filtering later on
DROP TABLE IF EXISTS #ContractGroupContractCTRs;
SELECT [ContractGroupsContract].[CTR_SEQ]
INTO #ContractGroupContractCTRs
FROM [ContractGroupsContract]
INNER JOIN [AccountContractGroups] 
    ON [AccountContractGroups].[ContractGroupId] = [ContractGroupsContract].[ContractGroupId]
WHERE [AccountContractGroups].[AccountId] = 2536
    AND [AccountContractGroups].[Deleted] = 0
    AND [ContractGroupsContract].[Deleted] = 0;

-- Materialize the relevant Tasks
DROP TABLE IF EXISTS #TASKS;
SELECT [TA_SEQ]
INTO #Tasks
FROM [TASKS]
WHERE ISNULL([TA_FKEY_CTR_SEQ], 0) = 0

UNION

SELECT [TA_SEQ]
FROM [TASKS]
WHERE [TA_FKEY_CTR_SEQ] = ''

UNION

SELECT [TASKS].[TA_SEQ]
FROM [TASKS]
INNER JOIN #ContractGroupContractCTRs
    ON [TASKS].[TA_FKEY_CTR_SEQ] = #ContractGroupContractCTRs.[CTR_SEQ];

-- Final Results
SELECT TOP (25) [EVENTS].[EV_SEQ] AS [FLD6]
FROM [EVENTS]
LEFT JOIN #Tasks
    ON #Events.[EV_FKEY_TA_SEQ] = #Tasks.[TA_SEQ]
WHERE ISNULL([EVENTS].[EV_SEQ], 1) > 0
ORDER BY [EVENTS].[EV_SEQ];

Without having your data in front of me to test with, it's difficult to know the accuracy of the re-write above. But let me know if the results are different and we can work through fixing the re-written query accordingly. Also here are some things to be aware of:

  1. I did not include the LEFT OUTER JOIN to [TASKS] AS [Pe2da3fbc7e676e2b21201605598f3] because like ypercube mentioned, it appears to be meaningless, since you don't use any fields from it and it's an outer join. The only use case I could see it being there is for purposefully exploding out the cardinality of the Events table (if the relationship is at least one-to-many), so that you get the same Event multiple times in your TOP 25. If that's the case, then you probably don't need the other more filtered down join to Tasks which is a subset of all Tasks that you're already joining to.

  2. In the same vein, since you're not actually SELECTing any of the fields from [TASKS] AS [Pd361fcc2673fdd885d288ab24f6f5] in your final SELECT (and because it's an outer join) this doesn't seem to make any functional difference to your query as well, unless again the goal is to purposefully explode out the cardinality of the Events table from the join. But again, if that's the case, it seems unlikely you need to do so by joining to this version of Tasks which is a subset of the first join instance of Tasks, as mentioned in my previous point. Something seems logically off with your query as a whole. It almost seems like all you need is SELECT TOP (25) [EVENTS].[EV_SEQ] AS [FLD6] FROM [EVENTS] ORDER BY [EVENTS].[EV_SEQ] ASC to logically accomplish your goals.

  3. Don't use the NOLOCK hint, it's essentially an anti-pattern at this point and has risks with data integrity.

0

Preface: I'm no SQL Server expert and some syntax can be wrong. I'm speaking from a general SQL perspective.

1. Simplify the query

Try to get rid of the unnecessary, just to be sure it isn't confusing the SQL engine. In particular try to remove the duplicate LEFT JOIN and execute the query this way to check the plan.

SELECT TOP (25) [EVENTS].[EV_SEQ] AS [FLD6]
 FROM [EVENTS] WITH (NOLOCK)
  LEFT OUTER JOIN [TASKS] WITH (NOLOCK) ON [EVENTS].[EV_FKEY_TA_SEQ] = [TASKS].[TA_SEQ]
 WHERE (
    [EVENTS].[EV_SEQ] > 0
    OR [EVENTS].[EV_SEQ] IS NULL
 ) AND (
   [TASKS].[TA_FKEY_CTR_SEQ] = 0
   OR [TASKS].[TA_FKEY_CTR_SEQ] IS NULL
   OR [TASKS].[TA_FKEY_CTR_SEQ] = ''
   OR [TASKS].[TA_FKEY_CTR_SEQ] IN (
    SELECT [ContractGroupsContract].[CTR_SEQ]
    FROM [ContractGroupsContract]
    INNER JOIN [AccountContractGroups] ON [AccountContractGroups].[ContractGroupId] = [ContractGroupsContract].[ContractGroupId]
    WHERE [AccountContractGroups].[AccountId] = 2536
     AND [AccountContractGroups].[Deleted] = 0
     AND [ContractGroupsContract].[Deleted] = 0
    )
   )
 ORDER BY [EVENTS].[EV_SEQ] ASC

2. Check the JOIN

Are you sure you need a LEFT JOIN? Do you want to include EVENTS not associated with any TASK? The LEFT JOIN combined with OR [TASKS].[TA_FKEY_CTR_SEQ] IS NULL will consider in the result set all EVENTS not linked to any TASKS. This means that SQL server has to join more than 4 millions of events to the respective task, before filtering out those that doesn't satisfy the filter.

3. Create new indexes.

While there are millions of events and tasks, apparently there are very few that satisfy the AccountContractGroups and ContractGroupsContract conditions, but since you only have indexes to seek from Events, to Tasks, to ContractGroupsContract, to AccountContractGroups, so for every Event the SQL engine has to follow this links just to find, at the end, that AccountContractGroups is not a suitable one.

It could be more effective to start from AccountContractGroups, navigate to ContractGroupsContract and then to the Tasks and Events suitable. To follow this path SQL server needs to have additional indexes like these:

AccountContractGroups = AccountId, Deleted, ContractGroupId
ContractGroupsContract = ContractGroupId, CTR_SEQ, Deleted
Tasks = TA_FKEY_CTR_SEQ, TA_SEQ
Events = EV_FKEY_TA_SEQ, EV_SEQ

4. Add more conditions

If none of this helps, you can use some knowledge from your data to restrict the number or rows processed by the query.

While SQL server is navigating your EVENTS table in EV_SEQ order, it seems it has to check more than 4 millions records before finding 25 that satisfy your conditions. Could it be that there are a lot of records with EV_SEQ <= 0? or [Deleted] <> 0?

Or maybe all the records that satisfy the conditions are recent inserts and you can add some date range or other condition to restrict the number of events to be searched.

Your Answer

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

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