Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using ClickHouse.EntityFrameworkCore.Infrastructure.Internal;
using ClickHouse.EntityFrameworkCore.Storage.Internal;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
Expand Down Expand Up @@ -27,7 +28,11 @@ public static async Task<long> BulkInsertAsync<TEntity>(
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())
Expand All @@ -53,6 +58,6 @@ public static async Task<long> BulkInsertAsync<TEntity>(
return row;
});

return await client.InsertBinaryAsync(tableName, columns, rows, cancellationToken: cancellationToken);
return await client.InsertBinaryAsync(qualifiedTableName, columns, rows, cancellationToken: cancellationToken);
}
}
Original file line number Diff line number Diff line change
@@ -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)}";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Comment on lines +433 to +437

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the looks of it, the EFCore EnsureSchema() function means "Let's make sure this is there" so for now it seems to make sense, would it not make more sense to alter ClickHouseCreateDatabase?

Would this be advisable in case the database already exists?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should definitely be with the IF NOT EXISTS, we don't want this to throw when the db exists.

}

// ENGINE clause generation

Expand Down
3 changes: 3 additions & 0 deletions src/EFCore.ClickHouse/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("EFCore.ClickHouse.Tests")]
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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<ClickHouseDatabaseCreator> _logger;

public ClickHouseDatabaseCreator(
RelationalDatabaseCreatorDependencies dependencies,
IClickHouseRelationalConnection connection,
IRawSqlCommandBuilder rawSqlCommandBuilder,
ISqlGenerationHelper sqlGenerationHelper)
ISqlGenerationHelper sqlGenerationHelper,
ICurrentDbContext currentDbContext,
ILogger<ClickHouseDatabaseCreator> 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()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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!;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text;
using ClickHouse.EntityFrameworkCore.Infrastructure.Internal;
using Microsoft.EntityFrameworkCore.Storage;

namespace ClickHouse.EntityFrameworkCore.Storage.Internal;
Expand All @@ -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("`", "``");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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[]))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using ClickHouse.EntityFrameworkCore.Infrastructure.Internal;
using ClickHouse.EntityFrameworkCore.Storage.Internal;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage;
Expand Down Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions test/EFCore.ClickHouse.Tests/DatabaseNameResolverTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading