Fix invalid DDL for multi-column SummingMergeTree

CHANGELOG.md

@@ -2,6 +2,7 @@ v0.3.1 (Unreleased)
 ---
 ### Bug fixes
 * `Sum`/`SumAsync` over a `double` or `float` column no longer throws `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns `0`, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; the literal generators now convert rather than unbox. The `Float32` read path also converts, since ClickHouse widens `sum(Float32)` to `Float64` (which the driver's `GetFloat()` refuses to downcast). ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46))
+* **SummingMergeTree with multiple sum columns**: `HasSummingMergeTreeEngine("A", "B")` now generates valid DDL (`SummingMergeTree((A, B))`). Previously it emitted a comma-separated argument list (`SummingMergeTree(A, B)`), which ClickHouse rejects with `NUMBER_OF_ARGUMENTS_DOESNT_MATCH`. Single-column and no-column usage are unaffected.
 
 v0.3.0
 ---

RELEASENOTES.md

@@ -2,6 +2,7 @@ v0.3.1 (Unreleased)
 ---
 ### Bug fixes
 * Summing a `double` or `float` column (`.SumAsync(x => x.Value)`) no longer throws `InvalidCastException`. Two ClickHouse-specific mismatches were biting: EF Core hands the float literal generator a boxed `Int32` `0` as the empty-result fallback, and ClickHouse widens `sum(Float32)` to `Float64` so the driver couldn't read it back as a `float`. Both the literal generation and the `Float32` read path now convert instead of hard-casting. ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) (Thanks to @HotTotem!)
+* **SummingMergeTree with multiple sum columns** now produces valid DDL. Configuring more than one sum column (`HasSummingMergeTreeEngine("A", "B")`) previously emitted `SummingMergeTree(A, B)`, which ClickHouse rejects with `NUMBER_OF_ARGUMENTS_DOESNT_MATCH` — the engine takes a single optional parameter that must be a tuple of columns. Multiple columns are now wrapped in a tuple (`SummingMergeTree((A, B))`); single-column and no-column usage are unchanged.
 
 v0.3.0
 ---

src/EFCore.ClickHouse/Migrations/ClickHouseMigrationsSqlGenerator.cs

@@ -538,7 +538,15 @@ private void GenerateEngineArgs(CreateTableOperation operation, string engine, M
             case ClickHouseAnnotationNames.SummingMergeTree:
                 var columns = (string[]?)operation.FindAnnotation(ClickHouseAnnotationNames.SummingMergeTreeColumns)?.Value;
                 if (columns is { Length: > 0 })
-                    builder.Append(string.Join(", ", columns.Select(QuoteColumnOrExpression)));
+                {
+                    var quotedColumns = columns.Select(QuoteColumnOrExpression);
+                    // SummingMergeTree accepts a single optional parameter: the column to sum, or a
+                    // tuple of columns. Multiple columns must be wrapped in a tuple — emitting them as
+                    // a comma-separated argument list produces invalid DDL (NUMBER_OF_ARGUMENTS_DOESNT_MATCH).
+                    builder.Append(columns.Length == 1
+                        ? quotedColumns.First()
+                        : $"({string.Join(", ", quotedColumns)})");
+                }
                 break;
 
             case ClickHouseAnnotationNames.CollapsingMergeTree:

test/EFCore.ClickHouse.Tests/MigrationIntegrationTests.cs

@@ -659,6 +659,133 @@ public async Task Column_with_default_value_applied_by_clickhouse_on_insert()
         Assert.NotEqual("1970-01-01 00:00:00", createdAt); // not epoch — got now()
     }
 
+    // ── Engine-argument round-trips (execute CREATE TABLE against real server) ──
+    // These guard against generating engine DDL that our string-based unit tests accept
+    // but ClickHouse rejects. EnsureCreatedAsync throws if the CREATE TABLE is invalid,
+    // so reaching the assertions proves the generated DDL actually ran on the server.
+
+    [Fact]
+    public async Task SummingMergeTree_single_column_creates_valid_table()
+    {
+        await using var ctx = CreateContext(b =>
+        {
+            b.Entity<SummingEntity>(e =>
+            {
+                e.HasKey(x => x.Id);
+                e.ToTable("smt_single_test", t => t
+                    .HasSummingMergeTreeEngine("Amount")
+                    .WithOrderBy("Id"));
+            });
+        });
+        await ctx.Database.EnsureDeletedAsync();
+        await ctx.Database.EnsureCreatedAsync();
+
+        var engine = await QueryScalar(ctx,
+            $"SELECT engine FROM system.tables WHERE database = '{_databaseName}' AND name = 'smt_single_test'");
+        Assert.Equal("SummingMergeTree", engine);
+
+        // ClickHouse normalizes the stored engine expression without backticks.
+        var createSql = await QueryScalar(ctx,
+            $"SELECT create_table_query FROM system.tables WHERE database = '{_databaseName}' AND name = 'smt_single_test'");
+        Assert.Contains("SummingMergeTree(Amount)", createSql!);
+    }
+
+    [Fact]
+    public async Task SummingMergeTree_multiple_columns_creates_valid_table()
+    {
+        // Regression: multiple sum columns must be emitted as a tuple, e.g.
+        // SummingMergeTree((`Amount`, `Count`)). The comma-separated form
+        // SummingMergeTree(`Amount`, `Count`) fails with NUMBER_OF_ARGUMENTS_DOESNT_MATCH,
+        // so EnsureCreatedAsync would throw before reaching the assertions below.
+        await using var ctx = CreateContext(b =>
+        {
+            b.Entity<SummingEntity>(e =>
+            {
+                e.HasKey(x => x.Id);
+                e.ToTable("smt_multi_test", t => t
+                    .HasSummingMergeTreeEngine("Amount", "Count")
+                    .WithOrderBy("Id"));
+            });
+        });
+        await ctx.Database.EnsureDeletedAsync();
+        await ctx.Database.EnsureCreatedAsync();
+
+        var engine = await QueryScalar(ctx,
+            $"SELECT engine FROM system.tables WHERE database = '{_databaseName}' AND name = 'smt_multi_test'");
+        Assert.Equal("SummingMergeTree", engine);
+
+        // The stored DDL must carry the sum columns as a tuple (ClickHouse normalizes
+        // away the backticks). The comma-separated form never reaches this point.
+        var createSql = await QueryScalar(ctx,
+            $"SELECT create_table_query FROM system.tables WHERE database = '{_databaseName}' AND name = 'smt_multi_test'");
+        Assert.Contains("SummingMergeTree((Amount, Count))", createSql!);
+    }
+
+    [Fact]
+    public async Task CollapsingMergeTree_creates_valid_table()
+    {
+        await using var ctx = CreateContext(b =>
+        {
+            b.Entity<CollapsingEntity>(e =>
+            {
+                e.HasKey(x => x.Id);
+                e.Property(x => x.Sign).HasColumnType("Int8");
+                e.ToTable("cmt_test", t => t
+                    .HasCollapsingMergeTreeEngine("Sign")
+                    .WithOrderBy("Id"));
+            });
+        });
+        await ctx.Database.EnsureDeletedAsync();
+        await ctx.Database.EnsureCreatedAsync();
+
+        var engine = await QueryScalar(ctx,
+            $"SELECT engine FROM system.tables WHERE database = '{_databaseName}' AND name = 'cmt_test'");
+        Assert.Equal("CollapsingMergeTree", engine);
+    }
+
+    [Fact]
+    public async Task VersionedCollapsingMergeTree_creates_valid_table()
+    {
+        await using var ctx = CreateContext(b =>
+        {
+            b.Entity<CollapsingEntity>(e =>
+            {
+                e.HasKey(x => x.Id);
+                e.Property(x => x.Sign).HasColumnType("Int8");
+                e.ToTable("vcmt_test", t => t
+                    .HasVersionedCollapsingMergeTreeEngine("Sign", "Version")
+                    .WithOrderBy("Id"));
+            });
+        });
+        await ctx.Database.EnsureDeletedAsync();
+        await ctx.Database.EnsureCreatedAsync();
+
+        var engine = await QueryScalar(ctx,
+            $"SELECT engine FROM system.tables WHERE database = '{_databaseName}' AND name = 'vcmt_test'");
+        Assert.Equal("VersionedCollapsingMergeTree", engine);
+    }
+
+    [Fact]
+    public async Task AggregatingMergeTree_creates_valid_table()
+    {
+        await using var ctx = CreateContext(b =>
+        {
+            b.Entity<SummingEntity>(e =>
+            {
+                e.HasKey(x => x.Id);
+                e.ToTable("amt_test", t => t
+                    .HasAggregatingMergeTreeEngine()
+                    .WithOrderBy("Id"));
+            });
+        });
+        await ctx.Database.EnsureDeletedAsync();
+        await ctx.Database.EnsureCreatedAsync();
+
+        var engine = await QueryScalar(ctx,
+            $"SELECT engine FROM system.tables WHERE database = '{_databaseName}' AND name = 'amt_test'");
+        Assert.Equal("AggregatingMergeTree", engine);
+    }
+
     // ── Helpers ──────────────────────────────────────────────────────────────
 
     private TestContext CreateContext(Action<ModelBuilder> configure)
@@ -766,4 +893,18 @@ public class FullFeaturedEntity
         public string Name { get; set; } = string.Empty;
         public DateTime CreatedAt { get; set; }
     }
+
+    public class SummingEntity
+    {
+        public long Id { get; set; }
+        public long Amount { get; set; }
+        public long Count { get; set; }
+    }
+
+    public class CollapsingEntity
+    {
+        public long Id { get; set; }
+        public sbyte Sign { get; set; }
+        public ulong Version { get; set; }
+    }
 }

test/EFCore.ClickHouse.Tests/MigrationSqlGeneratorTests.cs

@@ -587,7 +587,25 @@ public void SummingMergeTree_multiple_columns()
             op.Columns.Add(new AddColumnOperation { Name = "Id", ColumnType = "Int64", ClrType = typeof(long) });
         });
 
-        Assert.Contains("ENGINE = SummingMergeTree(`Amount`, `Count`)", sql);
+        // Multiple sum columns must be wrapped in a tuple. `SummingMergeTree(`Amount`, `Count`)`
+        // is invalid DDL — ClickHouse rejects it with NUMBER_OF_ARGUMENTS_DOESNT_MATCH.
+        Assert.Contains("ENGINE = SummingMergeTree((`Amount`, `Count`))", sql);
+    }
+
+    [Fact]
+    public void SummingMergeTree_single_column()
+    {
+        var sql = GenerateCreateTable(op =>
+        {
+            op.AddAnnotation(ClickHouseAnnotationNames.Engine, ClickHouseAnnotationNames.SummingMergeTree);
+            op.AddAnnotation(ClickHouseAnnotationNames.SummingMergeTreeColumns, new[] { "Amount" });
+            op.AddAnnotation(ClickHouseAnnotationNames.OrderBy, new[] { "Id" });
+            op.Columns.Add(new AddColumnOperation { Name = "Id", ColumnType = "Int64", ClrType = typeof(long) });
+        });
+
+        // A single sum column is passed directly, without tuple wrapping.
+        Assert.Contains("ENGINE = SummingMergeTree(`Amount`)", sql);
+        Assert.DoesNotContain("SummingMergeTree((", sql);
     }
 
     [Fact]