Eliminate False Positive SQL NOLOCK patterns in SQL Server

Problem

In Part 1 of this series, I showed how to identify “NOLOCK in DML” patterns within a single statement using a Visitor pattern. In Part 2, I showed how to find those patterns in procedure and trigger bodies across multiple databases and instances. I still need to show how to eliminate false positives without tedious visual inspection.

Solution

While I can use PowerShell to identify all of the tokens in a statement, I’m going to turn to T-SQL for deeper analysis, where I’m much more comfortable with writing relational logic. So, instead of writing each token to the screen, I’m going to store everything in a set of tables, and let queries handle the process of elimination.

Let’s create a collection database and a set of tables and procedures to handle input:

CREATE DATABASE Utility;
GO
USE Utility;
GO
CREATE TABLE dbo.NolockProcedureList
(
    ModuleID       int IDENTITY(1,1) NOT NULL,
    ServerName     nvarchar(513)     NOT NULL,
    DatabaseName   nvarchar(255)     NOT NULL,
    ProcedureName  nvarchar(1024)    NULL,
    Body           nvarchar(max)     NULL,
    CollectionTime datetime2(2)      NOT NULL DEFAULT sysdatetime(),
    CONSTRAINT PK_Procedures PRIMARY KEY CLUSTERED (ModuleID)
);
GO
CREATE PROCEDURE dbo.Nolock_AddProcedure
  @ServerName     nvarchar(513), 
  @DatabaseName   nvarchar(255),
  @ProcedureName  nvarchar(1024),
  @Body           nvarchar(max)
AS
BEGIN
  SET NOCOUNT ON;
  INSERT dbo.NolockProcedureList(ServerName,DatabaseName,ProcedureName,Body)
  OUTPUT inserted.ModuleID
    VALUES(@ServerName,@DatabaseName,@ProcedureName,@Body);  
END
GO
CREATE TABLE dbo.NolockStatementList
(
    ModuleID      int           NOT NULL,
    StatementID   int           NOT NULL,
    StatementType nvarchar(255) NOT NULL, 
    StatementText nvarchar(max) NULL, 
    LineNumber    int           NOT NULL,
    TokenStart    int           NOT NULL, 
    TokenEnd      int           NOT NULL, 
    CONSTRAINT PK_Statements PRIMARY KEY CLUSTERED (ModuleID,StatementID)
GO
CREATE PROCEDURE dbo.Nolock_AddStatement
  @ModuleID      int,
  @StatementID   int,
  @StatementType nvarchar(255),
  @StatementText nvarchar(max),
  @LineNumber    int,
  @TokenStart    int,
  @TokenEnd      int
AS
BEGIN
  SET NOCOUNT ON;
  INSERT dbo.NolockStatementList
    (ModuleID,StatementID,StatementType,StatementText,LineNumber,TokenStart,TokenEnd) 
  VALUES
    (@ModuleID,@StatementID,@StatementType,@StatementText,@LineNumber,@TokenStart,@TokenEnd);
END
GO
CREATE TABLE dbo.NolockTokenList
(
    ModuleID      int           NOT NULL, 
    StatementID   int           NOT NULL, 
    TokenID       int           NOT NULL,
    TokenType     nvarchar(255) NOT NULL, 
    TokenText     nvarchar(max) NULL, 
    IsTargetToken tinyint NOT NULL DEFAULT 0,
    IsHintToken   tinyint NOT NULL DEFAULT 0,
    INDEX CIX_Tokens CLUSTERED (ModuleID, StatementID, TokenID)
GO
CREATE PROCEDURE dbo.Nolock_AddToken
  @ModuleID       int,
  @StatementID    int,
  @TokenID        int,
  @TokenType      nvarchar(255),
  @TokenText      nvarchar(max)
AS
BEGIN
  SET NOCOUNT ON;
  INSERT dbo.NolockTokenList
    (ModuleID,StatementID,TokenID,TokenType,TokenText) 
  VALUES
    (COALESCE(@ModuleID,1),COALESCE(@StatementID,1),@TokenID,@TokenType,@TokenText);
END
GO
CREATE PROCEDURE dbo.Nolock_Wipeout
AS
BEGIN
  SET NOCOUNT ON;
  TRUNCATE TABLE dbo.NolockTokens;
  TRUNCATE TABLE dbo.NolockStatements;
  TRUNCATE TABLE dbo.NolockProcedures;
END 
GO

Next, we need to adjust our PowerShell code to call these procedures instead of writing output to the host:

#Add-Type -Path "C:\temp\ParseNOLOCK\Microsoft.SqlServer.TransactSql.ScriptDom.dll";
#region setup
$Parser = [Microsoft.SqlServer.TransactSql.ScriptDom.TSql150Parser]($true)::New(); 
$Errors = [System.Collections.Generic.List[Microsoft.SqlServer.TransactSql.ScriptDom.ParseError]]::New();
$CollectionString = "Server=192.168.1.222\SQL2019;Database=Utility";
$Servers = @("192.168.1.222\SQL2019");
$Conn = New-Object System.Data.SqlClient.SqlConnection;
$global:StatementID = 0;
$global:LastModuleID = 0;
$SQLCommand = @"
CREATE TABLE #Modules
(
    ProcedureName nvarchar(512), 
    Body          nvarchar(max), 
    DatabaseName  sysname, 
    ServerName    sysname
  SELECT ProcedureName = s.name + N''.'' + o.name + CASE 
                       WHEN t.[object_id] IS NOT NULL THEN 
                         N'' (trigger for '' + p.name + N'')'' ELSE N'''' END,
       Body          = OBJECT_DEFINITION(o.object_id),
       DatabaseName  = DB_NAME(),
       ServerName    = @@SERVERNAME
FROM sys.objects AS o WITH (NOLOCK)
INNER JOIN sys.schemas AS s WITH (NOLOCK) 
ON s.[schema_id] = o.[schema_id]
LEFT OUTER JOIN sys.triggers AS t WITH (NOLOCK)
ON o.[object_id] = t.[object_id]
LEFT OUTER JOIN sys.objects AS p WITH (NOLOCK)
ON t.parent_id = p.[object_id]
WHERE o.[type] IN (N''P'', N''TR'')
AND (LOWER(OBJECT_DEFINITION(o.object_id)) LIKE N''%update%from%nolock%''
     OR LOWER(OBJECT_DEFINITION(o.object_id)) LIKE N''%delete%from%nolock%'');';
SELECT ProcedureName, Body, DatabaseName, ServerName FROM #Modules;
"@
function Cleanup()
{
    try
    {
        $Conn = New-Object System.Data.SqlClient.SqlConnection;
        $Conn.ConnectionString  = $CollectionString;
        $Conn.ConnectionString += "Trusted_Connection=Yes; Integrated Security=SSPI;"
        $Conn.Open();
        $Command = $Conn.CreateCommand();
        $Command.CommandType = [System.Data.CommandType]::StoredProcedure;
        $Command.CommandText = "dbo.Nolock_Wipeout";
        $Command.ExecuteNonQuery() > $null;
    }
    catch
    {
        Write-Host "Failed inside cleanup ($($Conn.ConnectionString)).`n$PSItem";            
    }
    finally
    {
        $Conn.Close() > $null;
    }      
}
function WriteARow_Procedure
(
    [string]$Server,     
    [string]$Database,     
    [string]$Procedure,     
    [string]$Body     
)
{
    try
    {
        $Conn = New-Object System.Data.SqlClient.SqlConnection;
        $Conn.ConnectionString  = $CollectionString;
        $Conn.ConnectionString += "Trusted_Connection=Yes; Integrated Security=SSPI;"
        $Conn.Open();
        $Command = $Conn.CreateCommand();
        $Command.CommandType = [System.Data.CommandType]::StoredProcedure;
        $Command.CommandText = "dbo.Nolock_AddProcedure";
        $p1 = $Command.Parameters.Add("@ServerName",    [Data.SqlDbType]::NVarChar,  513).Value = $Server;
        $p2 = $Command.Parameters.Add("@DatabaseName",  [Data.SqlDbType]::NVarChar,  255).Value = $Database;
        $p3 = $Command.Parameters.Add("@ProcedureName", [Data.SqlDbType]::NVarChar, 1024).Value = $Procedure;
        $p4 = $Command.Parameters.Add("@Body",          [Data.SqlDbType]::NVarChar,   -1).Value = $Body;
        $global:LastModuleID = $Command.ExecuteScalar();
    }
    catch
    {
        Write-Host "Failed inside write a row ($($Conn.ConnectionString)).`n$PSItem";            
    }
    finally
    {
        $Conn.Close() > $null;
    }      
}
function WriteARow_Statement
(
    [string]$StatementType,
    [string]$StatementText,     
    [int]$LineNumber,     
    [int]$TokenStart,     
    [int]$TokenEnd
)
{   
    try
    {
        $global:LastStatementID += 1;
        $Conn = New-Object System.Data.SqlClient.SqlConnection;
        $Conn.ConnectionString  = $CollectionString;
        $Conn.ConnectionString += "Trusted_Connection=Yes; Integrated Security=SSPI;"
        $Conn.Open();
        $Command = $Conn.CreateCommand();
        $Command.CommandType = [System.Data.CommandType]::StoredProcedure;
        $Command.CommandText = "dbo.Nolock_AddStatement";
        $p1 = $Command.Parameters.Add("@ModuleID",      [Data.SqlDbType]::Int).Value = $global:LastModuleID;
        $p2 = $Command.Parameters.Add("@StatementID",   [Data.SqlDbType]::Int).Value = $global:LastStatementID;
        $p3 = $Command.Parameters.Add("@StatementType", [Data.SqlDbType]::NVarChar, 255).Value = $StatementType;
        $p4 = $Command.Parameters.Add("@StatementText", [Data.SqlDbType]::NVarChar,  -1).Value = $StatementText;
        $p5 = $Command.Parameters.Add("@LineNumber",    [Data.SqlDbType]::Int).Value = $LineNumber;
        $p6 = $Command.Parameters.Add("@TokenStart",    [Data.SqlDbType]::Int).Value = $TokenStart;
        $p7 = $Command.Parameters.Add("@TokenEnd",      [Data.SqlDbType]::Int).Value = $TokenEnd;
        $Command.ExecuteNonQuery() > $null;
    }
    catch
    {
        Write-Host "Failed inside write a row ($($Conn.ConnectionString)).`n$PSItem";            
    }
    finally
    {
        $Conn.Close() > $null;
    }      
}
function WriteARow_Token
(
    [int]$TokenID,     
    [string]$TokenType,     
    [string]$TokenText,
    [int]$IsTargetToken,
    [int]$IsHintToken
)
{
    try
    {
        $Conn = New-Object System.Data.SqlClient.SqlConnection;
        $Conn.ConnectionString  = $CollectionString;
        $Conn.ConnectionString += "Trusted_Connection=Yes; Integrated Security=SSPI;"
        $Conn.Open();
        $Command = $Conn.CreateCommand();
        $Command.CommandType = [System.Data.CommandType]::StoredProcedure;
        $Command.CommandText = "dbo.Nolock_AddToken";
        $p1 = $Command.Parameters.Add("@ModuleID",     [Data.SqlDbType]::Int).Value = $global:LastModuleID;
        $p2 = $Command.Parameters.Add("@StatementID",  [Data.SqlDbType]::Int).Value = $global:LastStatementID;
        $p3 = $Command.Parameters.Add("@TokenID",      [Data.SqlDbType]::Int).Value = $TokenID;
        $p4 = $Command.Parameters.Add("@TokenType",    [Data.SqlDbType]::NVarChar, 255).Value = $TokenType;
        $p5 = $Command.Parameters.Add("@TokenText",    [Data.SqlDbType]::NVarChar, -1).Value = $TokenText;
        $p6 = $Command.Parameters.Add("@IsTargetToken",[Data.SqlDbType]::TinyInt).Value = $IsTargetToken;
        $p7 = $Command.Parameters.Add("@IsHintToken",  [Data.SqlDbType]::TinyInt).Value = $IsHintToken;
        $Command.ExecuteNonQuery()   > $null;
    }
    catch
    {
        Write-Host "Failed inside write a row ($($Conn.ConnectionString)).`n$PSItem";            
    }
    finally
    {
        $Conn.Close() > $null;
    }      
}
#endregion setup
Cleanup;
for ($i = 0; $i -lt $Servers.Count; $i++)
{
    $Conn.ConnectionString  = "Server=$($Servers[$i]);Database=master;";
    $Conn.ConnectionString += "Trusted_Connection=Yes; Integrated Security=SSPI;"
    $Conn.Open();
    $Command = $Conn.CreateCommand();
    $Command.CommandText = $SQLCommand;
    $Reader = $Command.ExecuteReader();
    while ($Reader.Read()) 
    {
        [string]$ProcedureName  = $Reader.GetValue(0).ToString();
        [string]$Body           = $Reader.GetValue(1).ToString();
        [string]$DatabaseName   = $Reader.GetValue(2).ToString();
        [string]$ServerName     = $Reader.GetValue(3).ToString();
        WriteARow_Procedure -Server $ServerName -Database $DatabaseName -Procedure $ProcedureName -Body $Body;
        $Fragment = $Parser.Parse([System.IO.StringReader]::New($Body), [ref]$Errors);
        $Fragment.Accept([Visitor]::New());
        $global:StatementFirstToken = 0;
        $global:StatementLastToken = 0;
    }
    Write-Host "Wrote $($global:LastModuleID) procedures.";
}
class Visitor: Microsoft.SqlServer.TransactSql.ScriptDom.TSqlFragmentVisitor 
{
  [void]Visit ([Microsoft.SqlServer.TransactSql.ScriptDom.TSqlFragment] $Fragment)
  {
    $FragmentType   = $Fragment.GetType().Name;
    $ThisTokenStart = $Fragment.FirstTokenIndex;
    $ThisTokenEnd   = $Fragment.LastTokenIndex;
    $ThisTokenText  = $Fragment.ScriptTokenStream[$ThisTokenStart].Text;
    $InScope = $false;    
    if ($FragmentType -in ("UpdateStatement", "DeleteStatement"))
    {
        $ThisStatement = "";
        $global:StatementType = $FragmentType.Substring(0,6).ToLower();
        for ($i = $ThisTokenStart; $i -le $ThisTokenEnd; $i++)
        {
            $ThisStatement += $Fragment.ScriptTokenStream[$i].Text;
        }
        if ($ThisStatement -ilike "*from*nolock*")
        {
            $global:StatementID += 1;
            $global:StatementFirstToken = $ThisTokenStart;
            $global:StatementLastToken = $ThisTokenEnd;
            WriteARow_Statement -LastModuleID $global:LastModuleID -StatementType $global:StatementType -StatementText `
                      $ThisStatement -LineNumber $Fragment.StartLine -TokenStart $ThisTokenStart -TokenEnd $ThisTokenEnd;
        }
    }
    if (($ThisTokenStart -ge $global:StatementFirstToken) -and ($ThisTokenEnd -le $global:StatementLastToken))
    {
        $InScope = $true;
    }
    if (($FragmentType -in ("Identifier","FromClause","UpdateStatement","DeleteStatement","AssignmentSetClause")) -and ($InScope))
    {
      $TargetToken = 0;
      if ((($global:StatementType -eq "delete") -and ($FragmentType -eq "FromClause")) -or
          (($global:StatementType -eq "update") -and ($FragmentType -eq "AssignmentSetClause")))
      {
          $TargetToken = 1;
      }
      WriteARow_Token -TokenID $ThisTokenStart -TokenType $FragmentType -TokenText $ThisTokenText `
            -TargetToken $TargetToken -HintToken 0;
    } 
    if (($FragmentType -eq "TableHint") -and ($ThisTokenText -ieq "NOLOCK") -and ($InScope))
    {
      WriteARow_Token -TokenID $ThisTokenStart -TokenType $FragmentType -TokenText $ThisTokenText `
            -TargetToken 0 -HintToken 1;
    }
  }
}

Running that code will store all of the positives in our tables (along with, possibly, still a few false positives). But now that they’re in a relational form, we can pull out the most important elements, such as the object names and aliases affected by a NOLOCK hint, and the object name or alias that is the target of the statement. There may be many of the former, but there can only be one of the latter.

To simplify things in my final query, and to help troubleshoot along the way, I created two views. The first finds the single object or alias targeted by the update or delete statement:

CREATE VIEW dbo.ReferencedTargets
AS
  -- find the highest identifier before the first FROM (delete) or SET (update)
  -- which means - for simple UPDATE/DELETE FROM patterns - that's the target
  WITH TargetSrc AS 
  (
    SELECT ModuleID, StatementID, TokenID, TokenText, TokenType, 
        PreviousIdentifier = LAG(TokenID, 1) OVER 
                             (PARTITION BY ModuleID, StatementID ORDER BY TokenID)
      FROM dbo.NolockTokenList
      WHERE TokenType IN (N'Identifier',N'AssignmentSetClause',N'FromClause')
  ),
  MinTarget AS 
  (
    SELECT ModuleID, StatementID, FirstIdentifier = MIN(PreviousIdentifier) 
      FROM TargetSrc WHERE TokenType <> N'Identifier' 
      GROUP BY ModuleID, StatementID
  )
  SELECT t.ModuleID, t.StatementID, t.TokenID, Target_of_DML = t.TokenText
    FROM MinTarget
    INNER JOIN dbo.NolockTokenList AS t 
      ON MinTarget.FirstIdentifier = t.TokenID
      AND t.ModuleID = MinTarget.ModuleID
      AND t.StatementID = MinTarget.StatementID;
GO

The second view finds all the objects or aliases targeted by NOLOCK hints in each statement:

CREATE VIEW dbo.NolockHintedEntities
AS
  -- find the object or alias associated with each NOLOCK
  WITH NoLockSrc AS 
  (
    SELECT ModuleID, StatementID, TokenID, IsHintToken, 
       PreviousIdentifier = LAG(TokenID,1) OVER 
                                (PARTITION BY ModuleID, StatementID ORDER BY TokenID) 
      FROM dbo.NolockTokenList 
      WHERE (IsHintToken = 1 OR TokenType = N'Identifier')
  )
  SELECT t.ModuleID, t.StatementID, t.TokenID, NOLOCK_applies_to = t.TokenText
  FROM NoLockSrc
    INNER JOIN dbo.NolockTokenList AS t 
      ON NoLockSrc.PreviousIdentifier = t.TokenID
      AND t.ModuleID = NoLockSrc.ModuleID
      AND t.StatementID = NoLockSrc.StatementID
    WHERE NoLockSrc.IsHintToken = 1;
GO

If we query these views…

SELECT --p.ServerName, 
       --p.DatabaseName, 
       p.ProcedureName,
       s.LineNumber,
       n.Target_of_DML,
       s.StatementText
FROM dbo.ReferencedTargets AS n
  INNER JOIN dbo.NolockStatementList AS s 
    ON n.StatementID = s.StatementID 
    AND n.ModuleID = s.ModuleID
  INNER JOIN dbo.NolockProcedureList AS p 
    ON p.ModuleID = s.ModuleID
  ORDER BY p.ProcedureName, s.LineNumber;
SELECT --p.ServerName, 
       --p.DatabaseName,
       p.ProcedureName,
       s.LineNumber,
       n.NOLOCK_applies_to,
       s.StatementText
FROM dbo.NolockHintedEntities AS n
  INNER JOIN dbo.NolockStatementList AS s 
    ON n.StatementID = s.StatementID 
    AND n.ModuleID = s.ModuleID
  INNER JOIN dbo.NolockProcedureList AS p 
    ON p.ModuleID = s.ModuleID
  ORDER BY p.ProcedureName, s.LineNumber;

…we get the following output, and we can see how we will join these views to filter down to just the objects and statements that need to be addressed:

Output of 2 views illustrating how we can join

So now if we write the following query, we can join to reduce the set to exactly those rows where the name of the target exactly matches the name that NOLOCK applies to:

SELECT --p.ServerName, 
       --p.DatabaseName, 
       p.ProcedureName, 
       s.LineNumber, 
       TargetAlias = t.Target_of_DML, 
       s.StatementText
  FROM dbo.ReferencedElements AS t 
  INNER JOIN dbo.NolockHintedElements AS n 
    ON t.StatementID = n.StatementID
    AND t.ModuleID = n.ModuleID
    AND LOWER(t.Target_of_DML) = LOWER(NOLOCK_applies_to)
  INNER JOIN dbo.NolockStatementList AS s 
    ON n.StatementID = s.StatementID 
    AND n.ModuleID = s.ModuleID
  INNER JOIN dbo.NolockProcedureList AS p 
    ON p.ModuleID = s.ModuleID
  ORDER BY p.ProcedureName, s.LineNumber;

Output:

Final result set: only the statements w eneed to address

Now we have a final result set that consists only of the statements that really do contain a problematic NOLOCK pattern. It is perhaps a lot of work to get to this point, but in an environment where there are potentially thousands of false positives, it is worth the effort.

Caveats

This solution is far from perfect and, as I’ve been saying throughout, it is only meant to deal with the most simplistic DML patterns. In our case, this covered almost every single use case, but your mileage may vary. I may come back and revisit this solution to handle other cases this solution currently won’t capture, like the following two examples that will be missed:

;WITH u(e) AS 
(
  SELECT e FROM dbo.foo AS y WITH (NOLOCK)
)
UPDATE r SET e = 1 
  FROM u AS r 
  INNER JOIN dbo.SomethingElse AS s 
  ON 1 = 1;
UPDATE d SET x = 1
  FROM dbo.whatever AS x
  CROSS JOIN 
  (
    SELECT x FROM dbo.something WITH (NOLOCK)
  ) AS d;

…or cases where it still does capture a false positive, like this evil thing someone could write but actually isn’t vulnerable to the problem at hand:

UPDATE d SET x = 1
  FROM dbo.whatever AS d 
  WHERE y IN 
  (
    SELECT y FROM dbo.somethingElse AS d WITH (NOLOCK)
  );

So, there is definitely more work to do. I also want to come back and expand this solution to other problematic patterns, such as MERGE without HOLDLOCK, and to expand the search beyond just the modules stored in the database. It is a lot more expensive – especially on servers with massive amounts of memory – to scan the plan cache for problematic patterns that may be coming in from ad hoc queries or application code, but it is certainly possible, and may be worth doing as well.

Next Steps

See these tips and other resources involving NOLOCK, parsing modules, and TSqlFragmentVisitor:

Resources

Community

Subscribe

Eliminate False Positive SQL NOLOCK patterns in SQL Server – Part 3

Caveats

Resources

Community

Subscribe