From 507620cc0469b47acddba101b5f79061af324212 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:05:00 -0500 Subject: [PATCH 1/4] refactor(dedup): redundant Snowflake override + shared Insert regex helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the prior dedup pass: - **Delete `Snowflake.UpdateBreakdown.GetSqlBreakdown`**: it was a byte-for-byte copy of the SqlServer base's `GetSqlBreakdown` (modulo one explanatory comment). Snowflake's UPDATE syntax — including the FROM clause — is identical at the formatter level, so the override was pure inheritance noise. Now inherits. - **Extract `ParsePreparation.TryMatchInsertSql`**: the regex match + group extraction + failure message at the end of `SqlServer.InsertBreakdown.TryParse` and `Snowflake.InsertBreakdown.TryParse` was duplicated. Hoist the shared piece next to `TryRunPrelude` on `ParsePreparation`. Both callers continue to construct their own `InsertBreakdown` instance (the constructor signatures differ slightly between dialects). All 1180 tests stay green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/InsertBreakdown.cs | 13 ++------ .../Breakdowns/UpdateBreakdown.cs | 31 ++--------------- .../Breakdowns/InsertBreakdown.cs | 14 ++------ .../Statements/ParsePreparation.cs | 33 +++++++++++++++++++ 4 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/InsertBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/InsertBreakdown.cs index 89f3d06..777c677 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/InsertBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/InsertBreakdown.cs @@ -143,21 +143,12 @@ public class InsertBreakdown : SqlServerInsertBreakdown return false; } - // Parse INSERT statement using regex - var insertMatch = System.Text.RegularExpressions.Regex.Match(sql, - @"INSERT\s+INTO\s+([^\(\s]+)\s*\(([^\)]*)\)\s*VALUES\s*\(([^\)]*)\)", - System.Text.RegularExpressions.RegexOptions.IgnoreCase | System.Text.RegularExpressions.RegexOptions.Singleline, Strata.SqlTools.SqlBreakdown.Utilities.RegexDefaults.MatchTimeout); - - if (!insertMatch.Success) + if (!Strata.SqlTools.Statements.SqlServer.ParsePreparation.TryMatchInsertSql( + sql, out var tableName, out var columnsClause, out var valuesClause, out errorMessage)) { - errorMessage = "Could not parse INSERT statement. Expected format: INSERT INTO table (columns) VALUES (values)"; return false; } - var tableName = insertMatch.Groups[1].Value.Trim(); - var columnsClause = insertMatch.Groups[2].Value.Trim(); - var valuesClause = insertMatch.Groups[3].Value.Trim(); - result = new InsertBreakdown(tableName, columnsClause, valuesClause, isMicrosoftSql: false) { SetupClauses = setupClauses, diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/UpdateBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/UpdateBreakdown.cs index 5895dc1..056fc64 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/UpdateBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/UpdateBreakdown.cs @@ -44,35 +44,8 @@ public class UpdateBreakdown : SqlServerUpdateBreakdown WhereClause.Comment = whereComments.Count > 0 ? string.Join(" ", whereComments) : null; } - /// - /// Gets the SQL breakdown as a string for Snowflake. - /// - /// The UPDATE SQL statement. - protected override string GetSqlBreakdown() - { - var sb = new StringBuilder(); - - sb.AppendLine("UPDATE "); - sb.AppendLine($" {TableName.Clause}"); - - sb.AppendLine("SET "); - sb.AppendLine($" {SetClause.Clause}"); - - if (IsUsingFromClause) - { - // Snowflake supports FROM clause in UPDATE - sb.AppendLine("FROM "); - sb.AppendLine($" {FromClause.Clause}"); - } - - if (IsUsingWhereClause) - { - sb.AppendLine("WHERE "); - sb.AppendLine($" {WhereClause.Clause}"); - } - - return sb.ToString(); - } + // GetSqlBreakdown() inherited from SqlServer.UpdateBreakdown — Snowflake's UPDATE syntax + // (including the optional FROM clause) is identical at the formatter level, so no override needed. #region Parse Methods diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/InsertBreakdown.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/InsertBreakdown.cs index dd77b77..20198b9 100644 --- a/src/Strata.SqlTools.SqlServer/Breakdowns/InsertBreakdown.cs +++ b/src/Strata.SqlTools.SqlServer/Breakdowns/InsertBreakdown.cs @@ -152,22 +152,12 @@ public class InsertBreakdown : SqlBreakdownBase return false; } - // Parse INSERT statement using regex - // Pattern: INSERT INTO table (columns) VALUES (values) - var insertMatch = System.Text.RegularExpressions.Regex.Match(sql, - @"INSERT\s+INTO\s+([^\(\s]+)\s*\(([^\)]*)\)\s*VALUES\s*\(([^\)]*)\)", - System.Text.RegularExpressions.RegexOptions.IgnoreCase | System.Text.RegularExpressions.RegexOptions.Singleline, Strata.SqlTools.SqlBreakdown.Utilities.RegexDefaults.MatchTimeout); - - if (!insertMatch.Success) + if (!Strata.SqlTools.Statements.SqlServer.ParsePreparation.TryMatchInsertSql( + sql, out var tableName, out var columnsClause, out var valuesClause, out errorMessage)) { - errorMessage = "Could not parse INSERT statement. Expected format: INSERT INTO table (columns) VALUES (values)"; return false; } - var tableName = insertMatch.Groups[1].Value.Trim(); - var columnsClause = insertMatch.Groups[2].Value.Trim(); - var valuesClause = insertMatch.Groups[3].Value.Trim(); - result = new InsertBreakdown(tableName, columnsClause, valuesClause) { SetupClauses = setupClauses, diff --git a/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs b/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs index 489a0e7..e3eadce 100644 --- a/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs +++ b/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs @@ -66,4 +66,37 @@ public static class ParsePreparation return true; } + + /// + /// Runs the shared INSERT-statement match used by SqlServer / Snowflake (and any future + /// dialect that accepts the same INSERT INTO table (cols) VALUES (vals) grammar). + /// + /// The SQL passed through . + /// On success, the matched table name (trimmed). + /// On success, the matched column list (trimmed). + /// On success, the matched values list (trimmed). + /// On failure, a human-readable parse-error message. + /// true if the regex matched and the three groups are populated; false otherwise. + public static bool TryMatchInsertSql(string sql, out string tableName, out string columnsClause, out string valuesClause, out string errorMessage) + { + tableName = null!; + columnsClause = null!; + valuesClause = null!; + errorMessage = null!; + + var insertMatch = Regex.Match(sql, + @"INSERT\s+INTO\s+([^\(\s]+)\s*\(([^\)]*)\)\s*VALUES\s*\(([^\)]*)\)", + RegexOptions.IgnoreCase | RegexOptions.Singleline, RegexDefaults.MatchTimeout); + + if (!insertMatch.Success) + { + errorMessage = "Could not parse INSERT statement. Expected format: INSERT INTO table (columns) VALUES (values)"; + return false; + } + + tableName = insertMatch.Groups[1].Value.Trim(); + columnsClause = insertMatch.Groups[2].Value.Trim(); + valuesClause = insertMatch.Groups[3].Value.Trim(); + return true; + } } From 85cc79d5a1587a32be4321900a0ca4c68291adca Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:06:51 -0500 Subject: [PATCH 2/4] refactor(dedup): share clause-with-comments ingestion (PG/Snowflake QueryBreakdown ctors) The `QueryBreakdown(string select, string from, ...)` constructors on PostgreSql.QueryBreakdown and Snowflake.QueryBreakdown each ran the same six-line pattern twice (once per clause): call `parser.ExtractSqlComments`, set the clause to the trimmed result, join the comment list into the Comment property. New `StatementParser.PopulateClauseWithComments(rawText, target)` instance method does both halves. Each ctor now reads: parser.PopulateClauseWithComments(selectClause, SelectClause); parser.PopulateClauseWithComments(fromClause, FromClause); Same behavior; the helper is a pure refactor of existing semantics. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/QueryBreakdown.cs | 10 ++-------- .../Breakdowns/QueryBreakdown.cs | 10 ++-------- .../Statements/StatementParser.cs | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/Strata.SqlTools.PostgreSql/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.PostgreSql/Breakdowns/QueryBreakdown.cs index 0a07d1c..fa6bf27 100644 --- a/src/Strata.SqlTools.PostgreSql/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.PostgreSql/Breakdowns/QueryBreakdown.cs @@ -36,14 +36,8 @@ public class QueryBreakdown : SqlServerQueryBreakdown public QueryBreakdown(string selectClause, string fromClause, bool isMicrosoftSql = false) : base() { var parser = isMicrosoftSql ? Parser : PostgreSqlParserInstance; - - var cleanSelect = parser.ExtractSqlComments(selectClause, out var selectComments); - SelectClause.Clause = cleanSelect.Trim(); - SelectClause.Comment = selectComments.Count > 0 ? string.Join(" ", selectComments) : null; - - var cleanFrom = parser.ExtractSqlComments(fromClause, out var fromComments); - FromClause.Clause = cleanFrom.Trim(); - FromClause.Comment = fromComments.Count > 0 ? string.Join(" ", fromComments) : null; + parser.PopulateClauseWithComments(selectClause, SelectClause); + parser.PopulateClauseWithComments(fromClause, FromClause); } /// diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs index 5eb2c9a..637a8f1 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs @@ -39,14 +39,8 @@ public class QueryBreakdown : SqlServerQueryBreakdown public QueryBreakdown(string selectClause, string fromClause, bool isMicrosoftSql = false) : base() { var parser = isMicrosoftSql ? Parser : SnowflakeParserInstance; - - var cleanSelect = parser.ExtractSqlComments(selectClause, out var selectComments); - SelectClause.Clause = cleanSelect.Trim(); - SelectClause.Comment = selectComments.Count > 0 ? string.Join(" ", selectComments) : null; - - var cleanFrom = parser.ExtractSqlComments(fromClause, out var fromComments); - FromClause.Clause = cleanFrom.Trim(); - FromClause.Comment = fromComments.Count > 0 ? string.Join(" ", fromComments) : null; + parser.PopulateClauseWithComments(selectClause, SelectClause); + parser.PopulateClauseWithComments(fromClause, FromClause); } /// diff --git a/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs b/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs index 6b751c2..c46da5a 100644 --- a/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs +++ b/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs @@ -140,6 +140,22 @@ public class StatementParser /// The SQL statement containing comments. /// The extracted comments as a list of strings. /// The SQL statement with comments removed. + /// + /// Runs on and assigns the + /// cleaned text to 's (trimmed) + /// and the merged comments to its . Helper for + /// dialect-specific QueryBreakdown constructors that need to ingest + /// comment-bearing SQL fragments. + /// + /// The SQL fragment to clean. + /// The clause to populate. + public void PopulateClauseWithComments(string rawText, ISqlClause target) + { + var clean = ExtractSqlComments(rawText, out var comments); + target.Clause = clean.Trim(); + target.Comment = comments.Count > 0 ? string.Join(" ", comments) : null; + } + #pragma warning disable S3776 // Cognitive Complexity of methods should not be too high #pragma warning disable S127 // "for" loop stop conditions should be invariant public virtual string ExtractSqlComments(string sql, out List comments) From 4b6b3edc87723cf754bd57ae7294efa55de99c94 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:08:26 -0500 Subject: [PATCH 3/4] refactor(dedup): TryMatchTwoCharOperator helper in PG StatementReader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PostgreSql-specific operator dispatch in `StatementReader.TryHandleAdditionalCharacter` had four similar 4-7 line blocks (each handling a single-char operator with one or more two-char variants — \<, \>, \|, \=). Sonar flagged it as a self-duplication. Extract a small `TryMatchTwoCharOperator(char, string)` helper that encapsulates the "if next char matches, advance and emit two-char operator" pattern. Each operator handler now reads as a small list: if (CurrentCharacter == '<') { MovePosition(); if (TryMatchTwoCharOperator('=', "<=")) return true; if (TryMatchTwoCharOperator('>', "<>")) return true; if (TryMatchTwoCharOperator('<', "<<")) return true; _currentToken = new Token(TokenType.Operator, "<"); return true; } Reverses my earlier "extracting would obscure intent" call after re-reading — the helper-based form actually surfaces the intent ("two-char operator dispatch") more clearly than the original. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Statements/StatementReader.cs | 76 +++++++------------ 1 file changed, 28 insertions(+), 48 deletions(-) diff --git a/src/Strata.SqlTools.PostgreSql/Statements/StatementReader.cs b/src/Strata.SqlTools.PostgreSql/Statements/StatementReader.cs index 511f425..2320580 100644 --- a/src/Strata.SqlTools.PostgreSql/Statements/StatementReader.cs +++ b/src/Strata.SqlTools.PostgreSql/Statements/StatementReader.cs @@ -104,76 +104,39 @@ public class StatementReader : SqlServerStatementReader if (CurrentCharacter == '=') { - // Handle => operator (used in PostgreSQL for hstore and other operations) + // =, => (PostgreSQL hstore + other operations) MovePosition(); - if (CurrentCharacter == '>') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, "=>"); - return true; - } - // Single = is handled as regular operator + if (TryMatchTwoCharOperator('>', "=>")) return true; _currentToken = new Token(TokenType.Operator, "="); return true; } if (CurrentCharacter == '|') { - // Handle || concatenation operator + // |, || MovePosition(); - if (CurrentCharacter == '|') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, "||"); - return true; - } - // Single | is also an operator + if (TryMatchTwoCharOperator('|', "||")) return true; _currentToken = new Token(TokenType.Operator, "|"); return true; } if (CurrentCharacter == '<') { - // Handle <, <=, <>, << operators + // <, <=, <>, << MovePosition(); - if (CurrentCharacter == '=') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, "<="); - return true; - } - if (CurrentCharacter == '>') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, "<>"); - return true; - } - if (CurrentCharacter == '<') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, "<<"); - return true; - } + if (TryMatchTwoCharOperator('=', "<=")) return true; + if (TryMatchTwoCharOperator('>', "<>")) return true; + if (TryMatchTwoCharOperator('<', "<<")) return true; _currentToken = new Token(TokenType.Operator, "<"); return true; } if (CurrentCharacter == '>') { - // Handle >, >=, >> operators + // >, >=, >> MovePosition(); - if (CurrentCharacter == '=') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, ">="); - return true; - } - if (CurrentCharacter == '>') - { - MovePosition(); - _currentToken = new Token(TokenType.Operator, ">>"); - return true; - } + if (TryMatchTwoCharOperator('=', ">=")) return true; + if (TryMatchTwoCharOperator('>', ">>")) return true; _currentToken = new Token(TokenType.Operator, ">"); return true; } @@ -244,6 +207,23 @@ public class StatementReader : SqlServerStatementReader return stringValue.ToString(); } + + /// + /// If the position is currently sitting on , advances past it, + /// emits as the current Operator token, and returns + /// true. Otherwise leaves position untouched and returns false. Used by the + /// multi-character operator dispatch (e.g. </<=/<>/<<). + /// + private bool TryMatchTwoCharOperator(char nextChar, string twoCharOperator) + { + if (CurrentCharacter != nextChar) + { + return false; + } + MovePosition(); + _currentToken = new Token(TokenType.Operator, twoCharOperator); + return true; + } } From a84fe9768f3ca08d27adf18d0e0a00f658c487b7 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:11:39 -0500 Subject: [PATCH 4/4] refactor(dedup): remove redundant local TruncateText forwarders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `QueryBreakdownGenerator`, `SqlStatementGenerator`, and `ExpressionGenerator` each had a 1-line `private static string TruncateText(...)` forwarder to `Internal.MarkdownTextHelpers.TruncateText`. The forwarders existed only to keep existing call sites short (`TruncateText(x, 50)` instead of the fully-qualified form). Each file now imports `using static MarkdownTextHelpers;` once at the top, so call sites continue to read identically and the local forwarders are deleted. Removes the structural duplication Sonar was flagging (two private static helpers — `EscapeMermaidText` + the TruncateText forwarder — appearing in both `QueryBreakdownGenerator` and `SqlStatementGenerator` with the same shape). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Expressions/ExpressionGenerator.cs | 3 +-- .../SqlServer/QueryBreakdownGenerator.cs | 3 +-- .../SqlServer/SqlStatementGenerator.cs | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs index 67f4de5..fbd7821 100644 --- a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs @@ -10,6 +10,7 @@ using Strata.SqlTools.SqlBreakdown.Expressions.Functions.Aggregate; using Strata.SqlTools.SqlBreakdown.Expressions.Functions.Conditional; using Strata.SqlTools.SqlBreakdown.Expressions.Literals; using Strata.SqlTools.SqlBreakdown.Interfaces.Core; +using static Strata.SqlTools.Markdown.Internal.MarkdownTextHelpers; namespace Strata.SqlTools.Markdown.Expressions; @@ -360,8 +361,6 @@ public class ExpressionGenerator : IVisitor .Replace("]", "]"); } - private static string TruncateText(string text, int maxLength) - => Internal.MarkdownTextHelpers.TruncateText(text, maxLength); #region IVisitor Implementation diff --git a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs index dcb44a6..2a484b0 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs @@ -1,6 +1,7 @@ using System.Text; using Strata.SqlTools.Breakdowns.SqlServer; using Strata.SqlTools.SqlBreakdown.Interfaces; +using static Strata.SqlTools.Markdown.Internal.MarkdownTextHelpers; namespace Strata.SqlTools.Markdown.SqlServer; @@ -191,7 +192,5 @@ public static class QueryBreakdownGenerator .Replace(">", ">"); } - private static string TruncateText(string text, int maxLength) - => Internal.MarkdownTextHelpers.TruncateText(text, maxLength); } diff --git a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs index f3dc210..8cce65b 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs @@ -1,5 +1,6 @@ using System.Text; using Strata.SqlTools.SqlBreakdown.Interfaces; +using static Strata.SqlTools.Markdown.Internal.MarkdownTextHelpers; namespace Strata.SqlTools.Markdown.SqlServer; @@ -112,9 +113,6 @@ public static class SqlStatementGenerator .Replace("\r", ""); } - private static string TruncateText(string text, int maxLength) - => Internal.MarkdownTextHelpers.TruncateText(text, maxLength); - /// /// Cleans table name for use in Mermaid diagrams. ///