remediate: address review findings (#273)

(A) findings — addressed in code:
- Doc comments on `LikeExpr.NeedsEscape` and `EscapeLikeMetaChars` updated
  to flag dialect-aware render-time escaping (#1)
- New `MaybeDoubleBackslashes` helper: prefix/suffix in non-folded LIKE
  concat path now defensively backslash-doubled for MySQL+default (#7)
- Inline comments at both literal-pattern emission sites explain the
  single-quote-first / backslash-double-second order (#9)
- `SqlDialectConfig.ParseAttribute` returns `(Config, Schema)` tuple in
  a single pass; `ContextParser` consumes the tuple (#28)
- Code comment in `ParseAttribute` documents the SQLite default for
  missing `Dialect=` as intentional pre-existing behavior (#11)
- All 11 method bodies in `SqlAssembler.cs` now uniformly use the
  `var dialect = config.Dialect;` shadow at entry (#25)
- Extracted `IsDockerUnavailable`, `TableExistsAsync`, `ExecAsync` from
  the two MySQL test containers into shared `TestContainerHelpers.cs` (#26)
- Added `<remarks>` footgun warning to the back-compat
  `Render(SqlDialect)` overload — silently defaults the carrier flag (#33)
- Updated `MySqlBackslashEscapes` XML doc to make non-MySQL no-op
  behavior explicit; future QRY rule noted as follow-up (#37)

(B) findings — added gap-filling tests:
- `Contains_ParameterBound_BackslashEscapesTrue_NoDoubling` proves the
  parameter-bound LIKE path bypasses doubling (#20)
- `Where_Contains_LiteralBackslash_DefaultMode_ReturnsMatch` integration
  test exercises the seeded-but-unqueried `"a\b"` row through the full
  pipeline against live default-mode MySQL (#21)
- `Where_Contains_AnsiForm_NoBackslashEscapesSession_ReturnsMatch`
  integration test exercises the opt-out roundtrip with new
  `MyAnsiSessionDb` context against default-mode container with
  session-level `SET sql_mode = ...,NO_BACKSLASH_ESCAPES` (#2)

All 3,384 tests pass.

Author: DJGosnell

_sessions/273-sql-dialect-config/review.md

@@ -6,7 +6,7 @@ remote: https://github.com/Dtronix/Quarry.git
 base-branch: master
 
 ## State
-phase: IMPLEMENT
+phase: REMEDIATE
 status: active
 issue: #273
 pr:
@@ -56,4 +56,4 @@ Leave `NO_BACKSLASH_ESCAPES` in the main `MySqlTestContainer.cs` `--sql-mode` ar
 ## Session Log
 | # | Phase Start | Phase End | Summary |
 |---|------------|-----------|---------|
-| 1 | INTAKE | IMPLEMENT | 2026-04-29: Loaded issue #273. Worktree/branch created. Baseline 3364 tests green. DESIGN: 7 decisions recorded (3.B mode-aware emit, refactor+fix scope, internal sealed record, MySqlBackslashEscapes default true, attribute-only-MySqlBackslashEscapes-this-PR, keep test mitigation+add focused container, generator-internal carrier). PLAN: 4 phases written and approved. |
+| 1 | INTAKE | REVIEW | 2026-04-29: Loaded issue #273. Worktree/branch created. Baseline 3364 tests green. DESIGN: 7 decisions recorded (3.B mode-aware emit, refactor+fix scope, internal sealed record, MySqlBackslashEscapes default true, attribute-only-MySqlBackslashEscapes-this-PR, keep test mitigation+add focused container, generator-internal carrier). PLAN: 4 phases written and approved. IMPLEMENT: 4 phases committed. Phase 1 introduced SqlDialectConfig carrier (3364 tests green). Phase 2 added MySqlBackslashEscapes attribute+carrier flag (3372 tests). Phase 3 threaded SqlDialectConfig through SqlExprRenderer + SqlAssembler and branched LIKE emit on the flag (3378 tests). Phase 4 added default-mode container + 3 integration regression tests (3381 tests). |

_sessions/273-sql-dialect-config/workflow.md

@@ -1010,11 +1010,13 @@ SqlDialect.SQLite or SqlDialect.PostgreSQL
 
     private static void AppendTableRef(StringBuilder sb, SqlDialectConfig config, TableRef table)
     {
-        sb.Append(SqlFormatting.FormatTableName(config.Dialect, table.TableName, table.SchemaName));
+        var dialect = config.Dialect;
+        sb.Append(SqlFormatting.FormatTableName(dialect, table.TableName, table.SchemaName));
     }
 
     private static void AppendSelectColumns(StringBuilder sb, SqlDialectConfig config, IReadOnlyList<ProjectedColumn> columns, int paramOffset = 0)
     {
+        var dialect = config.Dialect;
         for (int i = 0; i < columns.Count; i++)
         {
             if (i > 0) sb.Append(", ");
@@ -1025,7 +1027,7 @@ private static void AppendSelectColumns(StringBuilder sb, SqlDialectConfig confi
             if (col.IsAggregateFunction && !string.IsNullOrEmpty(col.Alias))
             {
                 sb.Append(" AS ");
-                sb.Append(SqlFormatting.QuoteIdentifier(config.Dialect, col.Alias!));
+                sb.Append(SqlFormatting.QuoteIdentifier(dialect, col.Alias!));
             }
         }
     }

src/Quarry.Generator/IR/SqlAssembler.cs

@@ -377,7 +377,13 @@ internal sealed class LikeExpr : SqlExpr
     public string? LikePrefix { get; }
     /// <summary>Suffix for LIKE pattern (e.g., "%" for Contains).</summary>
     public string? LikeSuffix { get; }
-    /// <summary>Whether the pattern needs ESCAPE '\'.</summary>
+    /// <summary>
+    /// Whether the pattern needs an ESCAPE clause. Set at parse time when the
+    /// literal pattern contained LIKE metacharacters that <see cref="Translation.SqlLikeHelpers.EscapeLikeMetaChars"/>
+    /// escaped. The actual ESCAPE character emitted is dialect-aware: ANSI single
+    /// backslash (`'\'`) by default, doubled (`'\\'`) when the renderer's
+    /// <c>SqlDialectConfig</c> targets MySQL with <c>MySqlBackslashEscapes=true</c>.
+    /// </summary>
     public bool NeedsEscape { get; }
 
     public LikeExpr(SqlExpr operand, SqlExpr pattern, bool isNegated = false,

src/Quarry.Generator/IR/SqlExprNodes.cs

@@ -33,6 +33,17 @@ public static string Render(SqlExpr expr, SqlDialectConfig config, int parameter
     /// default <see cref="SqlDialectConfig"/> and forwards. Callers that don't yet have a
     /// <see cref="SqlDialectConfig"/> can keep passing the bare enum.
     /// </summary>
+    /// <remarks>
+    /// <b>Footgun:</b> the wrapper defaults <c>MySqlBackslashEscapes</c> to <c>true</c>
+    /// (matching the attribute default and stock MySQL). Calling this overload with
+    /// <see cref="SqlDialect.MySQL"/> will silently emit doubled-backslash LIKE patterns —
+    /// which is correct for stock MySQL but wrong for servers configured with
+    /// <c>NO_BACKSLASH_ESCAPES</c>. Prefer the
+    /// <see cref="Render(SqlExpr, SqlDialectConfig, int, bool, bool)"/> overload and pass
+    /// the per-context <c>SqlDialectConfig</c> from <c>BoundCallSite.DialectConfig</c>.
+    /// This overload exists for fragment-render paths that have no per-context dialect
+    /// (e.g. <c>TranslatedClause.SqlFragment</c>, which always uses PG).
+    /// </remarks>
     public static string Render(SqlExpr expr, SqlDialect dialect, int parameterBaseIndex = 0, bool useGenericParamFormat = false, bool stripOuterParens = false)
         => Render(expr, new SqlDialectConfig(dialect), parameterBaseIndex, useGenericParamFormat, stripOuterParens);
 
@@ -64,7 +75,9 @@ public static void RenderTo(StringBuilder sb, SqlExpr expr, SqlDialectConfig con
 
     /// <summary>
     /// Backwards-compatible overload accepting a bare <see cref="SqlDialect"/>. Wraps it in a
-    /// default <see cref="SqlDialectConfig"/> and forwards.
+    /// default <see cref="SqlDialectConfig"/> and forwards. See the same-named
+    /// <see cref="Render(SqlExpr, SqlDialect, int, bool, bool)"/> overload for the
+    /// <c>MySqlBackslashEscapes</c> footgun warning that applies here too.
     /// </summary>
     public static void RenderTo(StringBuilder sb, SqlExpr expr, SqlDialect dialect, int parameterBaseIndex = 0, bool useGenericParamFormat = false, bool stripOuterParens = false)
         => RenderTo(sb, expr, new SqlDialectConfig(dialect), parameterBaseIndex, useGenericParamFormat, stripOuterParens);
@@ -388,12 +401,16 @@ private static void RenderLike(LikeExpr like, SqlDialectConfig config, int param
         if ((hasPrefix || hasSuffix) && like.Pattern is LiteralExpr literalPattern
             && literalPattern.ClrType == "string" && !literalPattern.IsNull)
         {
+            // Order matters: single-quote escape FIRST (to '' pairs), then backslash
+            // doubling. Reversing would let the backslash pass turn an emitted '' pair
+            // into '\\' '' (no real input contains that today, but the order keeps the
+            // pipeline correct under any future input).
             var escaped = literalPattern.SqlText.Replace("'", "''");
             if (doubleBackslashes) escaped = escaped.Replace("\\", "\\\\");
             sb.Append('\'');
-            if (hasPrefix) sb.Append(like.LikePrefix);
+            if (hasPrefix) sb.Append(MaybeDoubleBackslashes(like.LikePrefix!, doubleBackslashes));
             sb.Append(escaped);
-            if (hasSuffix) sb.Append(like.LikeSuffix);
+            if (hasSuffix) sb.Append(MaybeDoubleBackslashes(like.LikeSuffix!, doubleBackslashes));
             sb.Append('\'');
         }
         else if (!hasPrefix && !hasSuffix)
@@ -404,6 +421,7 @@ private static void RenderLike(LikeExpr like, SqlDialectConfig config, int param
             if (doubleBackslashes && like.Pattern is LiteralExpr bareLit
                 && bareLit.ClrType == "string" && !bareLit.IsNull)
             {
+                // See comment above — same single-quote-first, backslash-double-second order.
                 var bareEscaped = bareLit.SqlText.Replace("'", "''").Replace("\\", "\\\\");
                 sb.Append('\'').Append(bareEscaped).Append('\'');
             }
@@ -415,13 +433,13 @@ private static void RenderLike(LikeExpr like, SqlDialectConfig config, int param
         else
         {
             var parts = new List<string>();
-            if (hasPrefix) parts.Add($"'{like.LikePrefix}'");
+            if (hasPrefix) parts.Add($"'{MaybeDoubleBackslashes(like.LikePrefix!, doubleBackslashes)}'");
 
             var patternSb = new StringBuilder();
             RenderExpr(like.Pattern, config, paramBase, patternSb, genericParams);
             parts.Add(patternSb.ToString());
 
-            if (hasSuffix) parts.Add($"'{like.LikeSuffix}'");
+            if (hasSuffix) parts.Add($"'{MaybeDoubleBackslashes(like.LikeSuffix!, doubleBackslashes)}'");
 
             if (parts.Count == 1)
             {
@@ -450,6 +468,17 @@ private static void RenderLike(LikeExpr like, SqlDialectConfig config, int param
         }
     }
 
+    /// <summary>
+    /// When <paramref name="doubleBackslashes"/> is <c>true</c>, doubles every backslash
+    /// in <paramref name="text"/> so the SQL parser on default-mode MySQL collapses
+    /// the doubled form back to a single literal backslash. No-op otherwise.
+    /// Today's analyzer only emits LIKE prefix/suffix as <c>%</c>, so the doubling
+    /// is a defensive guard for a future code path that puts a backslash in
+    /// <see cref="LikeExpr.LikePrefix"/> or <see cref="LikeExpr.LikeSuffix"/>.
+    /// </summary>
+    private static string MaybeDoubleBackslashes(string text, bool doubleBackslashes)
+        => doubleBackslashes && text.IndexOf('\\') >= 0 ? text.Replace("\\", "\\\\") : text;
+
     private static void RenderSubquery(SubqueryExpr sub, SqlDialectConfig config, int paramBase, StringBuilder sb, bool genericParams)
     {
         if (!sub.IsResolved)

src/Quarry.Generator/IR/SqlExprRenderer.cs

@@ -86,19 +86,8 @@ private static bool InheritsFromQuarryContext(INamedTypeSymbol classSymbol)
         if (attributeData == null)
             return null;
 
-        // Extract dialect configuration and Schema from attribute
-        var dialectConfig = SqlDialectConfig.FromAttribute(attributeData);
-        string? schema = null;
-
-        foreach (var namedArg in attributeData.NamedArguments)
-        {
-            switch (namedArg.Key)
-            {
-                case "Schema":
-                    schema = namedArg.Value.Value as string;
-                    break;
-            }
-        }
+        // Extract dialect configuration and Schema from attribute (single pass)
+        var (dialectConfig, schema) = SqlDialectConfig.ParseAttribute(attributeData);
 
         // Discover entities via partial QueryBuilder<T> properties
         var (entities, mappings) = DiscoverEntities(classDeclaration, semanticModel, cancellationToken);

src/Quarry.Generator/Parsing/ContextParser.cs

@@ -12,10 +12,21 @@ internal sealed record SqlDialectConfig(
     SqlDialect Dialect,
     bool MySqlBackslashEscapes = true)
 {
-    public static SqlDialectConfig FromAttribute(AttributeData attribute)
+    /// <summary>
+    /// Parses a <c>QuarryContextAttribute</c> into both a <see cref="SqlDialectConfig"/>
+    /// (dialect + mode flags) and a <c>Schema</c> string. Single pass over
+    /// <see cref="AttributeData.NamedArguments"/> so callers don't need to iterate twice.
+    /// </summary>
+    public static (SqlDialectConfig Config, string? Schema) ParseAttribute(AttributeData attribute)
     {
+        // Default Dialect=SQLite when the attribute omits a Dialect= named arg.
+        // Preserves pre-refactor ContextParser behavior (the same silent default).
+        // A consumer-facing QRY diagnostic for missing Dialect= would be a clearer
+        // ergonomic improvement — tracked as a follow-up; out of scope for #273
+        // which is the carrier refactor + LIKE-emit fix.
         var dialect = SqlDialect.SQLite;
         var mysqlBackslashEscapes = true;
+        string? schema = null;
 
         foreach (var named in attribute.NamedArguments)
         {
@@ -29,9 +40,20 @@ public static SqlDialectConfig FromAttribute(AttributeData attribute)
                     if (named.Value.Value is bool b)
                         mysqlBackslashEscapes = b;
                     break;
+                case "Schema":
+                    schema = named.Value.Value as string;
+                    break;
             }
         }
 
-        return new SqlDialectConfig(dialect, mysqlBackslashEscapes);
+        return (new SqlDialectConfig(dialect, mysqlBackslashEscapes), schema);
     }
+
+    /// <summary>
+    /// Parses just the dialect-config portion of a <c>QuarryContextAttribute</c>.
+    /// Convenience for tests; production code that also needs Schema should use
+    /// <see cref="ParseAttribute(AttributeData)"/>.
+    /// </summary>
+    public static SqlDialectConfig FromAttribute(AttributeData attribute)
+        => ParseAttribute(attribute).Config;
 }

src/Quarry.Generator/Sql/SqlDialectConfig.cs

@@ -11,7 +11,11 @@ namespace Quarry.Generators.Translation;
 internal static class SqlLikeHelpers
 {
     /// <summary>
-    /// Escapes LIKE metacharacters using backslash escaping (cross-dialect with ESCAPE '\').
+    /// Escapes LIKE metacharacters (<c>%</c>, <c>_</c>, <c>\</c>) using ANSI single-backslash
+    /// form. Runs at parse time before context binding, so the result is dialect-agnostic.
+    /// The renderer (<see cref="IR.SqlExprRenderer"/>) applies dialect-aware doubling at
+    /// emit time when targeting MySQL with <c>MySqlBackslashEscapes=true</c> — this layer
+    /// always produces ANSI form (single backslash escapes).
     /// </summary>
     public static string EscapeLikeMetaChars(string value)
     {

src/Quarry.Generator/Translation/SqlLikeHelpers.cs

@@ -3,6 +3,7 @@
 using Quarry;
 using Quarry.Tests.Samples;
 using MyDefault = Quarry.Tests.Samples.MyDefault;
+using MyAnsi = Quarry.Tests.Samples.MyAnsi;
 
 namespace Quarry.Tests.Integration;
 
@@ -50,6 +51,32 @@ public async Task Where_Contains_LiteralUnderscore_DefaultMode_ReturnsMatch()
         Assert.That(rows, Is.EquivalentTo(new[] { 1 }));
     }
 
+    /// <summary>
+    /// The most subtle escape case: the user's literal contains an actual backslash.
+    /// Pipeline: input <c>"a\b"</c> (1 backslash) → <c>EscapeLikeMetaChars</c> doubles to
+    /// <c>"a\\b"</c> (2) → renderer doubles again for MySQL+default to <c>"a\\\\b"</c>
+    /// (4 backslashes in SQL source) → MySQL parser collapses each <c>\\</c> to <c>\</c>,
+    /// yielding the LIKE pattern <c>"a\\b"</c> (2 backslashes in the runtime pattern) →
+    /// with <c>ESCAPE '\\'</c> (parsed: <c>\</c>), the LIKE evaluator interprets the second
+    /// <c>\\</c> in the pattern as escape+<c>\</c> = literal <c>\</c>, matching <c>"a\b"</c>
+    /// (1 literal backslash) in the column value.
+    /// </summary>
+    [Test]
+    public async Task Where_Contains_LiteralBackslash_DefaultMode_ReturnsMatch()
+    {
+        var cs = await MySqlDefaultModeTestContainer.GetConnectionStringAsync();
+        await using var conn = new MySqlConnection(cs);
+        await conn.OpenAsync();
+        await using var db = new MyDefault.MyDefaultDb(conn);
+
+        var rows = await db.Users()
+            .Where(u => u.UserName.Contains("a\\b"))
+            .Select(u => u.UserId)
+            .ExecuteFetchAllAsync();
+
+        Assert.That(rows, Is.EquivalentTo(new[] { 3 }));
+    }
+
     [Test]
     public async Task Where_Contains_LiteralPercent_DefaultMode_ReturnsMatch()
     {
@@ -66,6 +93,44 @@ public async Task Where_Contains_LiteralPercent_DefaultMode_ReturnsMatch()
         Assert.That(rows, Is.EquivalentTo(new[] { 2 }));
     }
 
+    /// <summary>
+    /// Opt-out roundtrip: <c>MyAnsiSessionDb</c> has <c>MySqlBackslashEscapes = false</c>,
+    /// so the generator emits ANSI single-backslash form (<c>'%user\_name%' ESCAPE '\'</c>).
+    /// We boot the default-mode container (stock sql_mode = backslash IS an escape),
+    /// then explicitly flip the session's <c>sql_mode</c> to add
+    /// <c>NO_BACKSLASH_ESCAPES</c> on this connection only. With the session-level
+    /// override, the ANSI form parses correctly and matches the expected row.
+    /// Proves both directions of the carrier flag → emit path work end-to-end.
+    /// </summary>
+    [Test]
+    public async Task Where_Contains_AnsiForm_NoBackslashEscapesSession_ReturnsMatch()
+    {
+        var cs = await MySqlDefaultModeTestContainer.GetConnectionStringAsync();
+        await using var conn = new MySqlConnection(cs);
+        await conn.OpenAsync();
+
+        // Session-level override: switch THIS connection to NO_BACKSLASH_ESCAPES
+        // sql_mode without affecting the container's server-wide mode (which other
+        // tests in this fixture rely on for the default-mode behavior).
+        await using (var setMode = conn.CreateCommand())
+        {
+            setMode.CommandText =
+                "SET SESSION sql_mode = " +
+                "'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE," +
+                "ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION,NO_BACKSLASH_ESCAPES'";
+            await setMode.ExecuteNonQueryAsync();
+        }
+
+        await using var db = new MyAnsi.MyAnsiSessionDb(conn);
+
+        var rows = await db.Users()
+            .Where(u => u.UserName.Contains("user_name"))
+            .Select(u => u.UserId)
+            .ExecuteFetchAllAsync();
+
+        Assert.That(rows, Is.EquivalentTo(new[] { 1 }));
+    }
+
     /// <summary>
     /// Sanity probe: under default sql_mode the broken-on-#273 emit shape
     /// (single ESCAPE backslash, ANSI form) actually fails with 1064.