Fix {name:Type} hints being dropped when an earlier brace is not a type hint

CHANGELOG.md

@@ -28,6 +28,7 @@ Unreleased
 
 **Bug Fixes:**
 * Fixed `{name:Type}` parameter type hints being mis-detected in queries containing `//` comments, nested block comments, backtick/double-quoted identifiers, backslash escapes or `$tag$` heredocs. A bare `#` no longer starts a comment (only `# ` and `#!` do) (issue #508).
+* Fixed `{name:Type}` parameter type hints being dropped, or a hint being invented for a parameter that does not exist, when the query contains another `{` that is not a type hint — for example a `SETTINGS` map value such as `additional_table_filters = {'t': 'a > 0'}`. A dropped hint fell back to CLR-type inference, losing precision (issue #510).
 * Fixed JSON typed paths whose names start with `max_dynamic_paths` or `max_dynamic_types` being mistaken for JSON settings and decoded as dynamic values.
 * Fixed enum type names rendering as invalid ClickHouse syntax. Enum labels are now quoted and escaped, and the declaration includes its closing parenthesis.
 * Fixed `InsertOptions.WithColumnTypes()` and `InsertOptions.WithQueryId()` silently dropping some caller-set options (such as `AcceptEncoding`) when copying.

ClickHouse.Driver.Tests/ADO/SqlParameterTypeExtractorTests.cs

@@ -541,4 +541,67 @@ public void ExtractTypeHints_UnterminatedQuotedToken_ReturnsEmptyDictionary(stri
 
         Assert.That(hints, Is.Empty);
     }
+    [Test]
+    [TestCase("SELECT 1 AS `x{y`, {val:Int32}")]
+    [TestCase("SELECT 1 AS \"x{y\", {val:Int32}")]
+    [TestCase("SELECT $$x{y$$, {val:Int32}")]
+    [TestCase("SELECT 1 // x{y\n, {val:Int32}")]
+    [TestCase("SELECT 1 AS `x{a:b{c`, {val:Int32}")]
+    [TestCase("SELECT {val:Int32}, 1 AS `y{a b:Int32}`")]
+    [TestCase("SELECT {val:Int32} SETTINGS additional_table_filters = {'t': 'a > 0'}")]
+    public void ExtractTypeHints_BraceThatIsNotATypeHint_HintStillExtracted(string sql)
+    {
+        var hints = SqlParameterTypeExtractor.ExtractTypeHints(sql);
+
+        Assert.That(hints, Has.Count.EqualTo(1));
+        Assert.That(hints["val"], Is.EqualTo("Int32"));
+    }
+
+    [Test]
+    [TestCase("SELECT {a}, {val:Int32}")]
+    [TestCase("SELECT {a:Int32, {val:Int32}")]
+    public void ExtractTypeHints_MalformedBracePrecedingHint_HintStillExtracted(string sql)
+    {
+        // The server rejects both of these queries; the cases pin that a malformed brace cannot
+        // corrupt hint extraction for the rest of the query.
+        var hints = SqlParameterTypeExtractor.ExtractTypeHints(sql);
+
+        Assert.That(hints, Has.Count.EqualTo(1));
+        Assert.That(hints["val"], Is.EqualTo("Int32"));
+    }
+
+    [Test]
+    [TestCase("SELECT {a}")]
+    [TestCase("SELECT {a}, {b}")]
+    public void ExtractTypeHints_ParameterWithoutType_NotIncluded(string sql)
+    {
+        var hints = SqlParameterTypeExtractor.ExtractTypeHints(sql);
+
+        Assert.That(hints, Is.Empty);
+    }
+
+    [Test]
+    [TestCase("SELECT {`a`:Int32}")]
+    [TestCase("SELECT {\"a\":Int32}")]
+    [TestCase("SELECT {a.b:Int32}")]
+    public void ExtractTypeHints_NameThatIsNotABareWord_NotIncluded(string sql)
+    {
+        // A parameter name is a bare word, so a quoted identifier is not a valid name: the server
+        // rejects all three of these queries.
+        var hints = SqlParameterTypeExtractor.ExtractTypeHints(sql);
+
+        Assert.That(hints, Is.Empty);
+    }
+
+    [Test]
+    [TestCase("SELECT {$p_1:Int32}", "$p_1")]
+    [TestCase("SELECT {1a:Int32}", "1a")]
+    [TestCase("SELECT {a\n:Int32}", "a")]
+    public void ExtractTypeHints_UnusualButValidParameterName_ReturnsType(string sql, string expectedName)
+    {
+        var hints = SqlParameterTypeExtractor.ExtractTypeHints(sql);
+
+        Assert.That(hints, Has.Count.EqualTo(1));
+        Assert.That(hints[expectedName], Is.EqualTo("Int32"));
+    }
 }

ClickHouse.Driver.Tests/ParameterCollectionTests.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using ClickHouse.Driver.ADO.Parameters;
 using NUnit.Framework;
 
@@ -51,4 +52,19 @@ public void TestParameterCollectionOperations()
         collection.Clear();
         Assert.That(collection.Count, Is.EqualTo(0));
     }
+
+    [Test]
+    [TestCase("SELECT 1 AS `x{y`, {dt:DateTime64(3, 'UTC')}")]
+    [TestCase("SELECT $$x{y$$, {dt:DateTime64(3, 'UTC')}")]
+    public void ResolveTypeNames_QueryContainsBraceThatIsNotATypeHint_UsesHint(string sql)
+    {
+        var collection = new ClickHouseParameterCollection
+        {
+            new ClickHouseDbParameter { ParameterName = "dt", Value = new DateTime(2020, 1, 2, 3, 4, 5, 123, DateTimeKind.Utc) },
+        };
+
+        var resolvedTypes = collection.ResolveTypeNames(sql, null);
+
+        Assert.That(resolvedTypes["dt"], Is.EqualTo("DateTime64(3, 'UTC')"));
+    }
 }

ClickHouse.Driver.Tests/SQL/SqlParameterizedSelectTests.cs

@@ -395,6 +395,25 @@ public async Task AddParameterWithTypeOverride_IdentifierViaExplicitTypeAndAdoPl
         Assert.That(result, Is.EqualTo(0UL)); // value of the `number` column, not the literal "number"
     }
 
+    [Test]
+    [TestCase("SELECT 1 AS `x{y`, {dt:DateTime64(3, 'UTC')} AS res")]
+    [TestCase("SELECT 1 AS \"x{y\", {dt:DateTime64(3, 'UTC')} AS res")]
+    [TestCase("SELECT $$x{y$$ AS q, {dt:DateTime64(3, 'UTC')} AS res")]
+    [TestCase("SELECT 1 // x{y\n, {dt:DateTime64(3, 'UTC')} AS res")]
+    [TestCase("SELECT 1 AS `x{a:b{c`, {dt:DateTime64(3, 'UTC')} AS res")]
+    public async Task AddParameter_HintPrecededByBraceThatIsNotAHint_KeepsSubSecondPrecision(string sql)
+    {
+        // The server accepts every query here. When the leading { consumed the hint, the parameter
+        // fell back to CLR-type inference (DateTime) and lost its sub-second component.
+        var value = new DateTime(2020, 1, 2, 3, 4, 5, 123, DateTimeKind.Utc);
+        using var command = connection.CreateCommand();
+        command.CommandText = sql;
+        command.AddParameter("dt", value);
+
+        var row = (await command.ExecuteReaderAsync()).GetEnsureSingleRow();
+        Assert.That(row.Last(), Is.EqualTo(value));
+    }
+
     /// <summary>
     /// Drops every name handed out by <see cref="CreateTableName"/>. Best-effort: a table that
     /// cannot be dropped must not fail an otherwise passing fixture.

ClickHouse.Driver/ADO/Parameters/SqlParameterTypeExtractor.cs

@@ -97,8 +97,35 @@ private static (string name, string type, int endIndex) TryExtractParameter(stri
         if (sql[startIndex] != '{')
             return (null, null, 0);
 
-        // Find the colon that separates name from type
-        var colonIndex = sql.IndexOf(':', startIndex + 1);
+        // Find the colon that separates name from type, searching only within this parameter's own
+        // name: it must be a single run of parameter name characters, optionally surrounded by
+        // whitespace. Otherwise a brace that is not a type hint, such as one inside a backtick-quoted
+        // alias, would consume the colon of a later parameter and silently drop its hint.
+        var colonIndex = -1;
+        var nameLength = 0;
+        var afterName = false;
+
+        for (var j = startIndex + 1; j < sql.Length; j++)
+        {
+            var nameChar = sql[j];
+            if (nameChar == ':')
+            {
+                colonIndex = j;
+                break;
+            }
+
+            if (char.IsWhiteSpace(nameChar))
+            {
+                afterName = nameLength > 0;
+                continue;
+            }
+
+            if (afterName || !IsParameterNameChar(nameChar))
+                return (null, null, 0);
+
+            nameLength++;
+        }
+
         if (colonIndex < 0)
             return (null, null, 0);
 
@@ -120,6 +147,12 @@ private static (string name, string type, int endIndex) TryExtractParameter(stri
                 // Quoted token within the type, e.g. an Enum value or a named tuple element
                 i = SkipQuotedToken(sql, i);
             }
+            else if (c == '{')
+            {
+                // A type definition never contains an opening brace, so this parameter is
+                // unterminated and the brace starts a new one
+                return (null, null, 0);
+            }
             else if (c == '}')
             {
                 // End of parameter
@@ -140,6 +173,14 @@ private static (string name, string type, int endIndex) TryExtractParameter(stri
         return (null, null, 0);
     }
 
+    /// <summary>
+    /// Determines whether the character can appear in a ClickHouse query parameter name. The server
+    /// parses the name as a bare word, which is narrower than an identifier: a quoted identifier such
+    /// as {`a`:Int32} or {"a":Int32} is rejected as a syntax error. Only ASCII word characters and $.
+    /// </summary>
+    private static bool IsParameterNameChar(char c) =>
+        (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '$';
+
     /// <summary>
     /// Skips to the end of a line
     /// Returns the index of the first character after the newline, or sql.Length if no newline found.

RELEASENOTES.md

@@ -28,6 +28,7 @@ Unreleased
 
 **Bug Fixes:**
 * Fixed `{name:Type}` parameter type hints being mis-detected in queries containing `//` comments, nested block comments, backtick/double-quoted identifiers, backslash escapes or `$tag$` heredocs. A bare `#` no longer starts a comment (only `# ` and `#!` do) (issue #508).
+* Fixed `{name:Type}` parameter type hints being dropped, or a hint being invented for a parameter that does not exist, when the query contains another `{` that is not a type hint — for example a `SETTINGS` map value such as `additional_table_filters = {'t': 'a > 0'}`. A dropped hint fell back to CLR-type inference, losing precision (issue #510).
 * Fixed JSON typed paths whose names start with `max_dynamic_paths` or `max_dynamic_types` being mistaken for JSON settings and decoded as dynamic values.
 * Fixed enum type names rendering as invalid ClickHouse syntax. Enum labels are now quoted and escaped, and the declaration includes its closing parenthesis.
 * Fixed `InsertOptions.WithColumnTypes()` and `InsertOptions.WithQueryId()` silently dropping some caller-set options (such as `AcceptEncoding`) when copying.