diff --git a/CHANGELOG.md b/CHANGELOG.md index dff11e1..623f54a 100644 --- a/CHANGELOG.md +++ b/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)) +* **Schema-to-database migration mapping**: migration `schema` values are now treated as ClickHouse database names. v0.3.0 --- diff --git a/src/EFCore.ClickHouse/Extensions/ClickHouseBulkInsertExtensions.cs b/src/EFCore.ClickHouse/Extensions/ClickHouseBulkInsertExtensions.cs index f3456de..ec0e7d1 100644 --- a/src/EFCore.ClickHouse/Extensions/ClickHouseBulkInsertExtensions.cs +++ b/src/EFCore.ClickHouse/Extensions/ClickHouseBulkInsertExtensions.cs @@ -1,3 +1,4 @@ +using ClickHouse.EntityFrameworkCore.Infrastructure.Internal; using ClickHouse.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -27,7 +28,11 @@ public static async Task BulkInsertAsync( var tableName = entityType.GetTableName() ?? throw new InvalidOperationException( $"The entity type '{typeof(TEntity).Name}' is not mapped to a table."); - + + // Schemas represent Databases in ClickHouse EF Core provider, as ClickHouse does not support Schemas + var database = entityType.GetSchema(); + var qualifiedTableName = ClickHouseIdentifierHelper.BuildQualifiedTableName(tableName, database); + // Build column list and property accessors var properties = entityType.GetProperties() .Where(p => p.GetTableColumnMappings().Any()) @@ -53,6 +58,6 @@ public static async Task BulkInsertAsync( return row; }); - return await client.InsertBinaryAsync(tableName, columns, rows, cancellationToken: cancellationToken); + return await client.InsertBinaryAsync(qualifiedTableName, columns, rows, cancellationToken: cancellationToken); } } diff --git a/src/EFCore.ClickHouse/Infrastructure/Internal/ClickHouseIdentifierHelper.cs b/src/EFCore.ClickHouse/Infrastructure/Internal/ClickHouseIdentifierHelper.cs new file mode 100644 index 0000000..81e0e49 --- /dev/null +++ b/src/EFCore.ClickHouse/Infrastructure/Internal/ClickHouseIdentifierHelper.cs @@ -0,0 +1,12 @@ +namespace ClickHouse.EntityFrameworkCore.Infrastructure.Internal; + +internal static class ClickHouseIdentifierHelper +{ + internal static string DelimitIdentifier(string name) + => $"`{name.Replace("`", "``")}`"; + + internal static string BuildQualifiedTableName(string tableName, string? schema) + => string.IsNullOrWhiteSpace(schema) + ? DelimitIdentifier(tableName) + : $"{DelimitIdentifier(schema)}.{DelimitIdentifier(tableName)}"; +} \ No newline at end of file diff --git a/src/EFCore.ClickHouse/Migrations/ClickHouseMigrationsSqlGenerator.cs b/src/EFCore.ClickHouse/Migrations/ClickHouseMigrationsSqlGenerator.cs index 4f26041..5d0e115 100644 --- a/src/EFCore.ClickHouse/Migrations/ClickHouseMigrationsSqlGenerator.cs +++ b/src/EFCore.ClickHouse/Migrations/ClickHouseMigrationsSqlGenerator.cs @@ -38,7 +38,7 @@ protected override void Generate(MigrationOperation operation, IModel? model, Mi protected virtual void Generate(ClickHouseCreateDatabaseOperation operation, MigrationCommandListBuilder builder) { builder - .Append("CREATE DATABASE ") + .Append("CREATE DATABASE IF NOT EXISTS ") .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)); EndStatement(builder, suppressTransaction: true); } @@ -277,11 +277,13 @@ protected override void Generate( IModel? model, MigrationCommandListBuilder builder) { + var targetSchema = operation.NewSchema ?? operation.Schema; + builder .Append("RENAME TABLE ") .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema)) .Append(" TO ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.NewName!, operation.NewSchema)); + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.NewName!, targetSchema)); EndStatement(builder); } @@ -427,7 +429,13 @@ protected override void Generate(RenameSequenceOperation operation, IModel? mode => throw new NotSupportedException("ClickHouse does not support sequences."); protected override void Generate(EnsureSchemaOperation operation, IModel? model, MigrationCommandListBuilder builder) - => throw new NotSupportedException("ClickHouse does not support schemas. Use databases instead."); + { + // To respect EFCore syntax, we treat schemas as databases + if (string.IsNullOrWhiteSpace(operation.Name)) + return; + + Generate(new ClickHouseCreateDatabaseOperation { Name = operation.Name }, builder); + } // ENGINE clause generation diff --git a/src/EFCore.ClickHouse/Properties/AssemblyInfo.cs b/src/EFCore.ClickHouse/Properties/AssemblyInfo.cs new file mode 100644 index 0000000..4a0aab2 --- /dev/null +++ b/src/EFCore.ClickHouse/Properties/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("EFCore.ClickHouse.Tests")] diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseCreator.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseCreator.cs index c06b000..2c347a5 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseCreator.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseCreator.cs @@ -1,7 +1,11 @@ using System.Data.Common; using ClickHouse.Driver.ADO; using ClickHouse.Driver.ADO.Parameters; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.Extensions.Logging; namespace ClickHouse.EntityFrameworkCore.Storage.Internal; @@ -10,21 +14,39 @@ public class ClickHouseDatabaseCreator : RelationalDatabaseCreator private readonly IClickHouseRelationalConnection _connection; private readonly IRawSqlCommandBuilder _rawSqlCommandBuilder; private readonly ISqlGenerationHelper _sqlGenerationHelper; + private readonly ICurrentDbContext _currentDbContext; + private readonly ILogger _logger; public ClickHouseDatabaseCreator( RelationalDatabaseCreatorDependencies dependencies, IClickHouseRelationalConnection connection, IRawSqlCommandBuilder rawSqlCommandBuilder, - ISqlGenerationHelper sqlGenerationHelper) + ISqlGenerationHelper sqlGenerationHelper, + ICurrentDbContext currentDbContext, + ILogger logger) : base(dependencies) { _connection = connection; _rawSqlCommandBuilder = rawSqlCommandBuilder; _sqlGenerationHelper = sqlGenerationHelper; + _currentDbContext = currentDbContext; + _logger = logger; } private string GetDatabaseName() - => new ClickHouseConnectionStringBuilder(_connection.ConnectionString).Database; + { + var connectionString = _connection.ConnectionString; + var connectionStringBuilder = new DbConnectionStringBuilder + { + ConnectionString = connectionString + }; + var hasExplicitDatabase = connectionStringBuilder.ContainsKey("Database"); + var connectionDatabase = hasExplicitDatabase + ? new ClickHouseConnectionStringBuilder(connectionString).Database + : null; + var schema = _currentDbContext.Context.Model.GetDefaultSchema(); + return ClickHouseDatabaseNameResolver.Resolve(connectionDatabase, schema, _logger); + } public override bool Exists() { diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseNameResolver.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseNameResolver.cs new file mode 100644 index 0000000..5e5b84a --- /dev/null +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseDatabaseNameResolver.cs @@ -0,0 +1,39 @@ +using Microsoft.Extensions.Logging; + +namespace ClickHouse.EntityFrameworkCore.Storage.Internal; + +internal static class ClickHouseDatabaseNameResolver +{ + public static string Resolve( + string? connectionDatabase, + string? schema, + ILogger logger) + { + var hasConnectionDatabase = !string.IsNullOrWhiteSpace(connectionDatabase); + var hasSchema = !string.IsNullOrWhiteSpace(schema); + + if (!hasSchema) + return connectionDatabase ?? string.Empty; + + if (!hasConnectionDatabase) + return schema!; + + if (string.Equals(connectionDatabase, schema, StringComparison.Ordinal)) + { + logger.LogInformation( + "The ClickHouse connection string database '{Database}' is the same as the configured EF Core schema." + + " The Database defined in the Schema overrides the Connection String database. Setting both is unnecessary.", + connectionDatabase); + } + else + { + logger.LogWarning( + "The configured EF Core schema '{Schema}' overrides the ClickHouse connection " + + "string database '{Database}'. The connection string database will not be used for this context.", + schema, + connectionDatabase); + } + + return schema!; + } +} diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseSqlGenerationHelper.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseSqlGenerationHelper.cs index 1adc94a..c9ccc08 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseSqlGenerationHelper.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseSqlGenerationHelper.cs @@ -1,4 +1,5 @@ using System.Text; +using ClickHouse.EntityFrameworkCore.Infrastructure.Internal; using Microsoft.EntityFrameworkCore.Storage; namespace ClickHouse.EntityFrameworkCore.Storage.Internal; @@ -23,10 +24,21 @@ public override void DelimitIdentifier(StringBuilder builder, string identifier) } public override string DelimitIdentifier(string name, string? schema) - => DelimitIdentifier(name); + => ClickHouseIdentifierHelper.BuildQualifiedTableName(name, schema); public override void DelimitIdentifier(StringBuilder builder, string name, string? schema) - => DelimitIdentifier(builder, name); + { + // If no Schema is provided, assume default, if schema is provided, add it to the identifier + if (string.IsNullOrWhiteSpace(schema)) + { + DelimitIdentifier(builder, name); + return; + } + + DelimitIdentifier(builder, schema); + builder.Append('.'); + DelimitIdentifier(builder, name); + } public override string EscapeIdentifier(string identifier) => identifier.Replace("`", "``"); diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseTypeMappingSource.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseTypeMappingSource.cs index 22e9e08..0a3a389 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseTypeMappingSource.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseTypeMappingSource.cs @@ -302,8 +302,9 @@ public ClickHouseTypeMappingSource( // ParseStoreTypeName may normalize or partially unwrap the store type in order to resolve // the underlying CLR mapping. EF Core's Property.GetColumnType() prefers - // RelationalTypeMapping.StoreType over the user's annotation, so preserve the explicit - // HasColumnType(...) text verbatim on the resolved mapping. + // RelationalTypeMapping.StoreType over the user's annotation, so preserve explicit + // HasColumnType(...) text on the resolved mapping except for scalar Nullable(...), + // whose nullability belongs in the migration operation's IsNullable property. private static RelationalTypeMapping PreserveExplicitStoreType( RelationalTypeMapping mapping, in RelationalTypeMappingInfo mappingInfo) @@ -315,6 +316,26 @@ private static RelationalTypeMapping PreserveExplicitStoreType( return mapping; } + // Nullable is represented by EF Core's property nullability for scalar columns. + // Do not retain a store-type Nullable(...) wrapper, otherwise migrations emit + // Nullable(T) in `type:` instead of T with `nullable: true`. + var normalizedStoreTypeName = NormalizeNullableStoreType(storeTypeName); + if (!string.Equals(normalizedStoreTypeName, storeTypeName, StringComparison.Ordinal)) + { + var normalizedInfo = new RelationalTypeMappingInfo( + storeTypeName: normalizedStoreTypeName, + storeTypeNameBase: mappingInfo.StoreTypeNameBase ?? normalizedStoreTypeName, + unicode: null, + size: mappingInfo.Size, + precision: mappingInfo.Precision, + scale: mappingInfo.Scale); + RelationalTypeMappingInfo? normalizedCloneInfo = normalizedInfo; + return mapping.Clone(in normalizedCloneInfo, storeTypePostfix: StoreTypePostfix.None); + } + + if (string.Equals(storeTypeName, mapping.StoreType, StringComparison.Ordinal)) + return mapping; + // Force StoreTypePostfix.None so the constructor does not rebuild the type name // from the inner facets (e.g. Decimal's PrecisionAndScale postfix would produce // "LowCardinality(Decimal32(4))(9,4)" otherwise). The local exists because @@ -323,6 +344,19 @@ private static RelationalTypeMapping PreserveExplicitStoreType( return mapping.Clone(in cloneInfo, storeTypePostfix: StoreTypePostfix.None); } + private static string NormalizeNullableStoreType(string storeType) + { + var trimmedStoreType = storeType.Trim(); + if (TryUnwrapPrefix(trimmedStoreType, "Nullable", out var nullableInner)) + return nullableInner; + + if (TryUnwrapPrefix(trimmedStoreType, "LowCardinality", out var inner) + && TryUnwrapPrefix(inner, "Nullable", out nullableInner)) + return $"LowCardinality({nullableInner})"; + + return storeType; + } + private static bool IsCollectionClrType(Type? clrType) { if (clrType is null || clrType == typeof(string) || clrType == typeof(byte[])) diff --git a/src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs b/src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs index 870b77b..c9a747e 100644 --- a/src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs +++ b/src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs @@ -1,3 +1,4 @@ +using ClickHouse.EntityFrameworkCore.Infrastructure.Internal; using ClickHouse.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Storage; @@ -82,12 +83,13 @@ public override async Task ExecuteAsync( // Group commands by table name and write-column set for correct row alignment var groups = _commands.GroupBy(c => ( + c.Schema, // Schemas are treated as Databases c.TableName, Columns: string.Join(",", c.ColumnModifications.Where(cm => cm.IsWrite).Select(cm => cm.ColumnName)))); foreach (var group in groups) { - var tableName = group.Key.TableName; + var tableName = ClickHouseIdentifierHelper.BuildQualifiedTableName(group.Key.TableName, group.Key.Schema); var commands = group.ToList(); var columns = commands[0].ColumnModifications diff --git a/test/EFCore.ClickHouse.Tests/DatabaseNameResolverTests.cs b/test/EFCore.ClickHouse.Tests/DatabaseNameResolverTests.cs new file mode 100644 index 0000000..1ebbbb5 --- /dev/null +++ b/test/EFCore.ClickHouse.Tests/DatabaseNameResolverTests.cs @@ -0,0 +1,40 @@ +using ClickHouse.EntityFrameworkCore.Storage.Internal; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace EFCore.ClickHouse.Tests; + +public class DatabaseNameResolverTests +{ + [Fact] + public void ConnectionDatabase_without_schema_is_used() + { + var database = ClickHouseDatabaseNameResolver.Resolve("connection_db", null, NullLogger.Instance); + + Assert.Equal("connection_db", database); + } + + [Fact] + public void Schema_without_connection_database_is_used() + { + var database = ClickHouseDatabaseNameResolver.Resolve(null, "schema_db", NullLogger.Instance); + + Assert.Equal("schema_db", database); + } + + [Fact] + public void Schema_overrides_different_connection_database() + { + var database = ClickHouseDatabaseNameResolver.Resolve("connection_db", "schema_db", NullLogger.Instance); + + Assert.Equal("schema_db", database); + } + + [Fact] + public void Identical_schema_and_connection_database_use_schema() + { + var database = ClickHouseDatabaseNameResolver.Resolve("same_db", "same_db", NullLogger.Instance); + + Assert.Equal("same_db", database); + } +} diff --git a/test/EFCore.ClickHouse.Tests/ExtendedTypeMappingTests.cs b/test/EFCore.ClickHouse.Tests/ExtendedTypeMappingTests.cs index 610031d..7320ce9 100644 --- a/test/EFCore.ClickHouse.Tests/ExtendedTypeMappingTests.cs +++ b/test/EFCore.ClickHouse.Tests/ExtendedTypeMappingTests.cs @@ -2582,15 +2582,13 @@ public class TypeMappingSourceStoreTypeTests { /// /// Verifies that explicit HasColumnType(...) text is preserved on the resolved - /// mapping's StoreType, even when the resolver normalizes/parses to discover - /// CLR semantics. + /// mapping's StoreType for non-nullability wrappers. /// [Theory] - [InlineData("Nullable(Int32)")] - [InlineData("Nullable(String)")] + [InlineData("Int32")] + [InlineData("String")] [InlineData("LowCardinality(String)")] - [InlineData("LowCardinality(Nullable(String))")] - [InlineData("Nullable(Float64)")] + [InlineData("Float64")] [InlineData("Enum8('a'=1,'b'=2)")] [InlineData("Dynamic(max_types=16)")] [InlineData("Json(max_dynamic_paths=256, max_dynamic_types=8, a.b UInt32, SKIP a.e)")] @@ -2603,6 +2601,31 @@ public void FindMapping_PreservesExplicitStoreType(string storeType) Assert.Equal(storeType, mapping.StoreType); } + [Theory] + [InlineData(typeof(string), "LowCardinality(Nullable(String))", "LowCardinality(String)")] + [InlineData(typeof(int), "LowCardinality(Nullable(Int32))", "LowCardinality(Int32)")] + [InlineData(typeof(double), "LowCardinality(Nullable(Float64))", "LowCardinality(Float64)")] + [InlineData(typeof(DateTime), "LowCardinality(Nullable(DateTime))", "LowCardinality(DateTime)")] + [InlineData(typeof(decimal), "LowCardinality(Nullable(Decimal(18, 4)))", "LowCardinality(Decimal(18, 4))")] + [InlineData(typeof(Guid), "LowCardinality(Nullable(UUID))", "LowCardinality(UUID)")] + [InlineData(typeof(string), "Nullable(String)", "String")] + [InlineData(typeof(int), "Nullable(Int32)", "Int32")] + [InlineData(typeof(double), "Nullable(Float64)", "Float64")] + [InlineData(typeof(DateTime), "Nullable(DateTime)", "DateTime")] + [InlineData(typeof(decimal), "Nullable(Decimal(18, 4))", "Decimal(18, 4)")] + [InlineData(typeof(string), "Nullable(Enum8('a'=1,'b'=2))", "Enum8('a'=1,'b'=2)")] + public void FindMapping_NormalizesTopLevelNullableStoreTypes( + Type clrType, + string storeType, + string expectedStoreType) + { + var source = GetTypeMappingSource(); + var mapping = source.FindMapping(clrType, storeType); + + Assert.NotNull(mapping); + Assert.Equal(expectedStoreType, mapping.StoreType); + } + [Theory] [InlineData("AggregateFunction(uniq, UInt64)")] [InlineData("SimpleAggregateFunction(sum, UInt64)")] @@ -2661,12 +2684,12 @@ public void FindMapping_ClrEnum_WithExplicitEnumStoreType_Preserved() } [Fact] - public void FindMapping_PreservesNullableDecimalWrapper() + public void FindMapping_NormalizesNullableDecimalWrapper() { var source = GetTypeMappingSource(); var mapping = source.FindMapping(typeof(decimal?), "Nullable(Decimal(18, 4))"); Assert.NotNull(mapping); - Assert.Equal("Nullable(Decimal(18, 4))", mapping.StoreType); + Assert.Equal("Decimal(18, 4)", mapping.StoreType); } // The store-type parser must respect single-quoted string literals when @@ -2731,6 +2754,19 @@ public void FindMapping_ContainerTypes_Resolves(string storeType) Assert.NotNull(mapping); } + [Theory] + [MemberData(nameof(ClickHouseCompositeTypeTestCases.NullableSubtypeCases), MemberType = typeof(ClickHouseCompositeTypeTestCases))] + public void FindMapping_CompositeTypesWithNullableSubtypes_PreservesNestedType( + string storeType, + string expectedStoreType) + { + var source = GetTypeMappingSource(); + var mapping = source.FindMapping(typeof(object), storeType); + + Assert.NotNull(mapping); + Assert.Equal(expectedStoreType, mapping.StoreType); + } + [Fact] public void FindMapping_TimeSpan_ResolvesFromClrType() { diff --git a/test/EFCore.ClickHouse.Tests/MigrationSqlGeneratorTests.cs b/test/EFCore.ClickHouse.Tests/MigrationSqlGeneratorTests.cs index 1f69333..c63810a 100644 --- a/test/EFCore.ClickHouse.Tests/MigrationSqlGeneratorTests.cs +++ b/test/EFCore.ClickHouse.Tests/MigrationSqlGeneratorTests.cs @@ -231,12 +231,17 @@ public void CreateSequence_throws_NotSupportedException() } [Fact] - public void EnsureSchema_throws_NotSupportedException() + public void EnsureSchema_generates_idempotent_CREATE_DATABASE() { - Assert.Throws(() => - { - Generate(new EnsureSchemaOperation { Name = "dbo" }); - }); + var sql = Generate(new EnsureSchemaOperation { Name = "dbo" }); + Assert.Contains("CREATE DATABASE IF NOT EXISTS `dbo`", sql); + } + + [Fact] + public void EnsureSchema_empty_name_is_noop() + { + var sql = Generate(new EnsureSchemaOperation { Name = "" }); + Assert.Equal(string.Empty, sql); } [Fact] @@ -246,6 +251,31 @@ public void RenameTable_generates_RENAME_TABLE() Assert.Contains("RENAME TABLE `old_table` TO `new_table`", sql); } + [Fact] + public void RenameTable_with_schema_qualifies_database_and_table() + { + var sql = Generate(new RenameTableOperation + { + Name = "old_table", + Schema = "db1", + NewName = "new_table", + NewSchema = "db2" + }); + Assert.Contains("RENAME TABLE `db1`.`old_table` TO `db2`.`new_table`", sql); + } + + [Fact] + public void RenameTable_without_new_schema_keeps_existing_schema() + { + var sql = Generate(new RenameTableOperation + { + Name = "old_table", + Schema = "db1", + NewName = "new_table" + }); + Assert.Contains("RENAME TABLE `db1`.`old_table` TO `db1`.`new_table`", sql); + } + [Fact] public void RenameColumn_generates_ALTER_TABLE_RENAME_COLUMN() { @@ -740,6 +770,17 @@ public void AddColumn_generates_ALTER_TABLE() Assert.Contains("ALTER TABLE `t` ADD COLUMN `NewCol` String", sql); } + [Fact] + public void AddColumn_with_schema_qualifies_database_and_table() + { + var op = new AddColumnOperation + { + Schema = "analytics", Table = "t", Name = "NewCol", ColumnType = "String", ClrType = typeof(string) + }; + var sql = Generate(op); + Assert.Contains("ALTER TABLE `analytics`.`t` ADD COLUMN `NewCol` String", sql); + } + [Fact] public void AlterColumn_generates_MODIFY_COLUMN() { @@ -760,10 +801,10 @@ public void DropColumn_generates_ALTER_TABLE() } [Fact] - public void CreateDatabase_generates_CREATE_DATABASE() + public void CreateDatabase_generates_idempotent_CREATE_DATABASE() { var sql = Generate(new ClickHouseCreateDatabaseOperation { Name = "my_db" }); - Assert.Contains("CREATE DATABASE `my_db`", sql); + Assert.Contains("CREATE DATABASE IF NOT EXISTS `my_db`", sql); } [Fact] @@ -912,12 +953,52 @@ public void HasColumnType_LowCardinality_String_preserved_in_CreateTable_DDL() => AssertColumnTypePreserved("LowCardinality(String)"); [Fact] - public void HasColumnType_LowCardinality_NullableString_preserved_in_CreateTable_DDL() - => AssertColumnTypePreserved("LowCardinality(Nullable(String))"); + public void HasColumnType_LowCardinality_NullableString_uses_property_nullability() + { + using var ctx = new LowCardinalityNullableContext(); + var model = ctx.GetService().Model.GetRelationalModel(); + var differ = ctx.GetService(); + var operations = differ.GetDifferences(source: null, target: model); + + var createTable = Assert.Single(operations.OfType()); + var pathColumn = createTable.Columns.Single(c => c.Name == "Path"); + Assert.Equal("LowCardinality(String)", pathColumn.ColumnType); + Assert.True(pathColumn.IsNullable); + + var generator = ctx.GetService(); + var sql = string.Join("\n", generator.Generate(operations).Select(c => c.CommandText)); + Assert.Contains("`Path` LowCardinality(String)", sql); + Assert.DoesNotContain("Nullable(LowCardinality", sql); + Assert.DoesNotContain("LowCardinality(Nullable", sql); + } [Fact] - public void HasColumnType_Nullable_String_preserved_in_CreateTable_DDL() - => AssertColumnTypePreserved("Nullable(String)"); + public void HasColumnType_Nullable_String_uses_nullable_migration_metadata() + { + using var ctx = new NullableStringContext(); + var model = ctx.GetService().Model.GetRelationalModel(); + var differ = ctx.GetService(); + var operations = differ.GetDifferences(source: null, target: model); + + var createTable = Assert.Single(operations.OfType()); + var pathColumn = createTable.Columns.Single(c => c.Name == "Path"); + Assert.Equal("String", pathColumn.ColumnType); + Assert.True(pathColumn.IsNullable); + } + + [Fact] + public void HasColumnType_Nullable_Enum_uses_nullable_migration_metadata() + { + using var ctx = new NullableEnumContext(); + var model = ctx.GetService().Model.GetRelationalModel(); + var differ = ctx.GetService(); + var operations = differ.GetDifferences(source: null, target: model); + + var createTable = Assert.Single(operations.OfType()); + var languageColumn = createTable.Columns.Single(c => c.Name == "Language"); + Assert.Equal("Enum8('a'=1,'b'=2)", languageColumn.ColumnType); + Assert.True(languageColumn.IsNullable); + } [Fact] public void HasColumnType_Array_LowCardinality_element_preserved_in_CreateTable_DDL() @@ -1024,11 +1105,53 @@ private sealed class LowCardinalityStringContext : LowCardinalityContextBase private sealed class LowCardinalityNullableContext : LowCardinalityContextBase { protected override string ColumnType => "LowCardinality(Nullable(String))"; + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(e => + { + e.HasKey(x => x.Id); + e.Property(x => x.Path).HasColumnType(ColumnType); + e.ToTable("page_views", t => t.HasMergeTreeEngine().WithOrderBy("Id")); + }); + } } private sealed class NullableStringContext : LowCardinalityContextBase { protected override string ColumnType => "Nullable(String)"; + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(e => + { + e.HasKey(x => x.Id); + e.Property(x => x.Path).HasColumnType(ColumnType); + e.ToTable("page_views", t => t.HasMergeTreeEngine().WithOrderBy("Id")); + }); + } + } + + private sealed class NullableEnumContext : DbContext + { + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseClickHouse("Host=localhost;Database=test"); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(e => + { + e.HasKey(x => x.Id); + e.Property(x => x.Language).HasColumnType("Nullable(Enum8('a'=1,'b'=2))"); + e.ToTable("languages", t => t.HasMergeTreeEngine().WithOrderBy("Id")); + }); + } + } + + private sealed class NullableEnumEntity + { + public int Id { get; set; } + public string? Language { get; set; } } private sealed class AggregateFunctionContext : LowCardinalityContextBase diff --git a/test/EFCore.ClickHouse.Tests/TestCaseClasses/ClickHouseCompositeTypeTestCases.cs b/test/EFCore.ClickHouse.Tests/TestCaseClasses/ClickHouseCompositeTypeTestCases.cs new file mode 100644 index 0000000..9fed324 --- /dev/null +++ b/test/EFCore.ClickHouse.Tests/TestCaseClasses/ClickHouseCompositeTypeTestCases.cs @@ -0,0 +1,49 @@ +using System.Collections.Generic; + +namespace EFCore.ClickHouse.Tests; + +public static class ClickHouseCompositeTypeTestCases +{ + private static readonly string[] NullableSubtypes = + [ + "String", + "Int32", + "Int64", + "Float64", + "DateTime", + "UUID", + "Decimal(18, 4)", + ]; + + public static IEnumerable NullableSubtypeCases() + { + foreach (var subtype in NullableSubtypes) + { + yield return + [ + $"LowCardinality(Nullable({subtype}))", + $"LowCardinality({subtype})", + ]; + yield return + [ + $"Array(Nullable({subtype}))", + $"Array(Nullable({subtype}))", + ]; + yield return + [ + $"Map(String, Nullable({subtype}))", + $"Map(String, Nullable({subtype}))", + ]; + yield return + [ + $"Tuple(Nullable({subtype}), String)", + $"Tuple(Nullable({subtype}), String)", + ]; + yield return + [ + $"Variant(String, Nullable({subtype}))", + $"Variant(String, Nullable({subtype}))", + ]; + } + } +}