Fix identifier parsing and improve FROM clause parsing (#3368)

Co-authored-by: Martin Costello <[email protected]>

Author: stevejgordon

src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Fix diacritic identifier parsing hang and improve FROM clause parsing.
+([#3368](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3368))
+
 ## 1.13.0-beta.1
 
 Released 2025-Oct-22

src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Fix diacritic identifier parsing hang and improve FROM clause parsing.
+([#3368](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3368))
+
 ## 1.13.0-beta.1
 
 Released 2025-Oct-22

src/Shared/SqlProcessor.cs

@@ -15,6 +15,7 @@ internal static class SqlProcessor
     private const char SanitizationPlaceholder = '?';
     private const char SpaceChar = ' ';
     private const char CommaChar = ',';
+    private const char OpenSquareBracketChar = '[';
     private const char OpenParenChar = '(';
     private const char CloseParenChar = ')';
     private const char DashChar = '-';
@@ -35,6 +36,13 @@ internal static class SqlProcessor
     private static readonly SearchValues<char> WhitespaceSearchValues = SearchValues.Create(WhitespaceChars);
 #endif
 
+    // This is not an exhaustive list but covers the majority of common reserved SQL keywords that may follow a FROM clause.
+    // This is used when determining if the previous token is a keyword in order to identify the end of a comma separated FROM clause.
+    // NOTE: These are ordered so that more likely keywords appear first to shorten the comparison loop.
+    private static readonly string[] FromClauseReservedKeywords = [
+        "WHERE", "BY", "AS", "JOIN", "WITH", "CROSS", "HAVING", "WINDOW", "LIMIT", "OFFSET", "TABLESAMPLE", "PIVOT", "UNPIVOT"
+    ];
+
     // We can extend this in the future to include more keywords if needed.
     // The keywords should be ordered by frequency of use to optimize performance.
     // This only includes keywords that may be the first keyword in a statement.
@@ -148,9 +156,9 @@ private static bool IsAsciiDigit(char c) =>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private static bool IsAsciiIdentifierChar(char c) =>
 #if NET
-        char.IsAsciiLetter(c) || char.IsAsciiDigit(c) || c == UnderscoreChar || c == DotChar;
+        char.IsLetter(c) || char.IsAsciiDigit(c) || c == UnderscoreChar || c == DotChar;
 #else
-        IsAsciiLetter(c) || IsAsciiDigit(c) || c == UnderscoreChar || c == DotChar;
+        char.IsLetter(c) || IsAsciiDigit(c) || c == UnderscoreChar || c == DotChar;
 #endif
 
     private static SqlStatementInfo SanitizeSql(ReadOnlySpan<char> sql)
@@ -371,6 +379,7 @@ private static void ParseNextToken(
                     state.ParsePosition += keywordLength;
                     state.PreviousTokenStartPosition = start;
                     state.PreviousTokenEndPosition = start + keywordLength;
+                    state.InFromClause = potentialKeywordInfo.SqlKeyword == SqlKeyword.From;
 
                     // No further parsing needed for this token
                     return;
@@ -401,14 +410,31 @@ private static void ParseNextToken(
             var length = i - start;
             if (length > 0)
             {
+                var tokenSpan = sql.Slice(start, length);
+
+                // Special handling: if we are in a FROM clause, check if this identifier is a reserved keyword
+                // that indicates the end of the FROM clause.
+                if (state.InFromClause)
+                {
+                    foreach (var reservedKeyword in FromClauseReservedKeywords)
+                    {
+                        if (IsCaseInsensitiveMatch(tokenSpan, reservedKeyword))
+                        {
+                            // We've matched a reserved keyword so are no longer in the FROM clause.
+                            state.InFromClause = false;
+                            break;
+                        }
+                    }
+                }
+
                 // Copy to sanitized buffer.
-                sql.Slice(start, length).CopyTo(buffer.Slice(state.SanitizedPosition));
+                tokenSpan.CopyTo(buffer.Slice(state.SanitizedPosition));
                 state.SanitizedPosition += length;
 
                 // Optionally copy to summary buffer.
                 if (state.CaptureNextTokenInSummary)
                 {
-                    sql.Slice(start, length).CopyTo(state.SummaryBuffer.Slice(state.SummaryPosition));
+                    tokenSpan.CopyTo(state.SummaryBuffer.Slice(state.SummaryPosition));
                     state.SummaryPosition += length;
 
                     // Add a space after the identifier. The trailing space will be trimmed later.
@@ -423,15 +449,35 @@ private static void ParseNextToken(
         }
         else
         {
-            // Special case: if the token is a comma and follows a FROM keyword,
-            // we are in an old style implicit join and we want to capture the next valid identifier in the summary.
-            var prevKeyword = state.PreviousParsedKeyword?.SqlKeyword ?? SqlKeyword.Unknown;
-            state.CaptureNextTokenInSummary = prevKeyword == SqlKeyword.From && currentChar == CommaChar;
+            // If we are in a FROM clause, we want to capture the next identifier following a comma or open square bracket.
+            // Commas may occur when listing multiple tables in a FROM clause.
+            // Brackets may occur when using schema-qualified or delimited identifiers.
+            state.CaptureNextTokenInSummary = state.InFromClause && (currentChar == CommaChar || currentChar == OpenSquareBracketChar);
 
             buffer[state.SanitizedPosition++] = currentChar;
             state.ParsePosition++;
 
-            // We don't update previous token start/end positions for single-char tokens.
+            // NOTE: We don't update previous token start/end positions for single-char tokens.
+        }
+
+        static bool IsCaseInsensitiveMatch(ReadOnlySpan<char> tokenSpan, string reservedKeyword)
+        {
+            if (tokenSpan.Length != reservedKeyword.Length)
+            {
+                return false;
+            }
+
+            var reservedKeywordSpan = reservedKeyword.AsSpan();
+
+            for (var charPos = 0; charPos < tokenSpan.Length; charPos++)
+            {
+                if ((tokenSpan[charPos] | 0x20) != (reservedKeywordSpan[charPos] | 0x20))
+                {
+                    return false;
+                }
+            }
+
+            return true;
         }
     }
 
@@ -724,6 +770,10 @@ private ref struct ParseState
         // Stored in state to avoid slicing repeatedly.
         public Span<char> SummaryBuffer;
 
+        /// <summary>
+        /// Will be set if a keyword has been matched by the parser.
+        /// Not all keywords are necessarily matched.
+        /// </summary>
         public SqlKeywordInfo? PreviousParsedKeyword; // 8 bytes (reference type)
 
         public SqlKeyword FirstSummaryKeyword; // 4 bytes (enum, underlying int)
@@ -739,7 +789,16 @@ private ref struct ParseState
         public int PreviousTokenStartPosition; // 4 bytes
         public int PreviousTokenEndPosition; // 4 bytes
 
+        // NOTE: If the number of bool fields increases significantly, consider combining into a bitfield.
+
         public bool CaptureNextTokenInSummary; // 1 byte
+
+        /// <summary>
+        /// Used to track if we are in a FROM clause for special handling of comma-separated table lists.
+        /// When set to <c>true</c>, subsequent unmatched tokens will be compared against reserved keywords.
+        /// As soon as we match a reserved keyword, we exit the FROM clause state.
+        /// </summary>
+        public bool InFromClause; // 1 byte
     }
 
     private sealed class SqlKeywordInfo

test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorAdditionalTestCases.json

@@ -857,5 +857,31 @@
       ],
       "db.query.summary": "ALTER TABLE MyTable"
     }
+  },
+  {
+    "name": "query_with_diacritic_characters",
+    "input": {
+      "db.system.name": "microsoft.sql_server",
+      "query": "DECLARE @__geräteIds_0 nvarchar(4000) = N'[1,2,3]';\n\nSELECT [t].[Id]\nFROM [Tests] AS [t]\nWHERE [t].[Id] IN (\n\tSELECT [g].[value]\n\tFROM OPENJSON(@__geräteIds_0) WITH ([value] bigint '$') AS [g]\n)"
+    },
+    "expected": {
+      "db.query.text": [
+        "DECLARE @__geräteIds_0 nvarchar(?) = N?;\n\nSELECT [t].[Id]\nFROM [Tests] AS [t]\nWHERE [t].[Id] IN (\n\tSELECT [g].[value]\n\tFROM OPENJSON(@__geräteIds_0) WITH ([value] bigint ?) AS [g]\n)"
+      ],
+      "db.query.summary": "SELECT Tests"
+    }
+  },
+  {
+    "name": "cross_join_comma_separated_syntax_followed_by_composite_group_by",
+    "input": {
+      "db.system.name": "other_sql",
+      "query": "SELECT a, b FROM TableA, TableB GROUP BY a, b"
+    },
+    "expected": {
+      "db.query.text": [
+        "SELECT a, b FROM TableA, TableB GROUP BY a, b"
+      ],
+      "db.query.summary": "SELECT TableA TableB"
+    }
   }
 ]