Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable value conversion support for identity/serial/hilo properties #1945

Merged
merged 2 commits into from
Aug 6, 2021
Merged
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
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

<Copyright>Copyright 2021 © The Npgsql Development Team</Copyright>
<Company>Npgsql</Company>
<VersionPrefix>6.0.0-preview7</VersionPrefix>
<VersionPrefix>6.0.0-rc.1</VersionPrefix>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<PackageLicenseExpression>PostgreSQL</PackageLicenseExpression>
<PackageProjectUrl>https://github.com/npgsql/efcore.pg</PackageProjectUrl>
Expand Down
4 changes: 2 additions & 2 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project>
<PropertyGroup>
<EFCoreVersion>6.0.0-preview.7.21378.4</EFCoreVersion>
<MicrosoftExtensionsVersion>6.0.0-preview.7.21377.19</MicrosoftExtensionsVersion>
<EFCoreVersion>6.0.0-rc.1.21406.1</EFCoreVersion>
<MicrosoftExtensionsVersion>6.0.0-rc.1.21401.3</MicrosoftExtensionsVersion>
<NpgsqlVersion>6.0.0-preview6</NpgsqlVersion>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ public ExtensionInfo(IDbContextOptionsExtension extension)

public override bool IsDatabaseProvider => false;

public override long GetServiceProviderHashCode() => Extension.IsGeographyDefault.GetHashCode();
public override int GetServiceProviderHashCode() => Extension.IsGeographyDefault.GetHashCode();

public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo other)
=> true;

public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ public ExtensionInfo(IDbContextOptionsExtension extension)

public override bool IsDatabaseProvider => false;

public override long GetServiceProviderHashCode() => 0;
public override int GetServiceProviderHashCode() => 0;

public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo other)
=> true;

public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
=> debugInfo["Npgsql:" + nameof(NpgsqlNodaTimeDbContextOptionsBuilderExtensions.UseNodaTime)] = "1";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,26 +406,26 @@ private static void CheckValueGenerationStrategy(IReadOnlyProperty property, Npg
/// <returns> <see langword="true" /> if compatible. </returns>
public static bool IsCompatibleWithValueGeneration(IReadOnlyProperty property)
{
var type = property.ClrType;
var valueConverter = property.GetValueConverter()
?? property.FindTypeMapping()?.Converter;

return type.IsInteger()
&& (property.GetValueConverter()
?? property.FindTypeMapping()?.Converter)
== null;
var type = (valueConverter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();

return type.IsInteger();
}

private static bool IsCompatibleWithValueGeneration(
IReadOnlyProperty property,
in StoreObjectIdentifier storeObject,
ITypeMappingSource? typeMappingSource)
{
var type = property.ClrType;
var valueConverter = property.GetValueConverter()
?? (property.FindRelationalTypeMapping(storeObject)
?? typeMappingSource?.FindMapping((IProperty)property))?.Converter;

var type = (valueConverter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();

return type.IsInteger()
&& (property.GetValueConverter()
?? (property.FindRelationalTypeMapping(storeObject)
?? typeMappingSource?.FindMapping((IProperty)property))?.Converter)
== null;
return type.IsInteger();
}

#endregion Value generation
Expand Down
33 changes: 18 additions & 15 deletions src/EFCore.PG/Infrastructure/Internal/NpgsqlOptionsExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public override DbContextOptionsExtensionInfo Info

private sealed class ExtensionInfo : RelationalExtensionInfo
{
private long? _serviceProviderHash;
private int? _serviceProviderHash;
private string? _logFragment;

public ExtensionInfo(IDbContextOptionsExtension extension)
Expand Down Expand Up @@ -324,26 +324,29 @@ public override string LogFragment
}
}

public override long GetServiceProviderHashCode()
public override int GetServiceProviderHashCode()
{
unchecked
if (_serviceProviderHash == null)
{
if (_serviceProviderHash == null)
var hashCode = new HashCode();

foreach (var userRangeDefinition in Extension._userRangeDefinitions)
{
_serviceProviderHash = Extension._userRangeDefinitions.Aggregate(
base.GetServiceProviderHashCode(),
(h, ud) => (h * 397) ^ ud.GetHashCode());
_serviceProviderHash = (_serviceProviderHash * 397) ^ Extension.AdminDatabase?.GetHashCode() ?? 0L;
_serviceProviderHash = (_serviceProviderHash * 397) ^ (Extension.PostgresVersion?.GetHashCode() ?? 0L);
_serviceProviderHash = (_serviceProviderHash * 397) ^ Extension.UseRedshift.GetHashCode();
_serviceProviderHash = (_serviceProviderHash * 397) ^ (Extension.ProvideClientCertificatesCallback?.GetHashCode() ?? 0L);
_serviceProviderHash = (_serviceProviderHash * 397) ^ (Extension.RemoteCertificateValidationCallback?.GetHashCode() ?? 0L);
_serviceProviderHash = (_serviceProviderHash * 397) ^ (Extension.ProvidePasswordCallback?.GetHashCode() ?? 0L);
_serviceProviderHash = (_serviceProviderHash * 397) ^ Extension.ReverseNullOrdering.GetHashCode();
hashCode.Add(userRangeDefinition);
}

return _serviceProviderHash.Value;
hashCode.Add(Extension.AdminDatabase);
hashCode.Add(Extension.PostgresVersion);
hashCode.Add(Extension.UseRedshift);
hashCode.Add(Extension.ProvideClientCertificatesCallback);
hashCode.Add(Extension.RemoteCertificateValidationCallback);
hashCode.Add(Extension.ProvidePasswordCallback);
hashCode.Add(Extension.ReverseNullOrdering);

_serviceProviderHash = hashCode.ToHashCode();
}

return _serviceProviderHash.Value;
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ public NpgsqlModificationCommandBatch(

protected override int GetParameterCount() => _parameterCount;

protected override bool CanAddCommand(ModificationCommand modificationCommand)
protected override bool CanAddCommand(IReadOnlyModificationCommand modificationCommand)
{
if (ModificationCommands.Count >= _maxBatchSize)
return false;

var newParamCount = (long)_parameterCount + (long)modificationCommand.ColumnModifications.Count;
var newParamCount = (long)_parameterCount + modificationCommand.ColumnModifications.Count;
if (newParamCount > int.MaxValue)
return false;

Expand Down
17 changes: 7 additions & 10 deletions src/EFCore.PG/Update/Internal/NpgsqlUpdateSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ public NpgsqlUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies)

public override ResultSetMapping AppendInsertOperation(
StringBuilder commandStringBuilder,
ModificationCommand command,
IReadOnlyModificationCommand command,
int commandPosition)
=> AppendInsertOperation(commandStringBuilder, command, commandPosition, false);

public virtual ResultSetMapping AppendInsertOperation(
StringBuilder commandStringBuilder,
ModificationCommand command,
IReadOnlyModificationCommand command,
int commandPosition,
bool overridingSystemValue)
{
Expand Down Expand Up @@ -51,7 +51,7 @@ public virtual ResultSetMapping AppendInsertOperation(

public override ResultSetMapping AppendUpdateOperation(
StringBuilder commandStringBuilder,
ModificationCommand command,
IReadOnlyModificationCommand command,
int commandPosition)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Expand All @@ -78,7 +78,7 @@ public override ResultSetMapping AppendUpdateOperation(
// ReSharper disable once ParameterTypeCanBeEnumerable.Local
private void AppendReturningClause(
StringBuilder commandStringBuilder,
IReadOnlyList<ColumnModification> operations)
IReadOnlyList<IColumnModification> operations)
{
commandStringBuilder
.AppendLine()
Expand All @@ -95,19 +95,16 @@ public override void AppendNextSequenceValueOperation(StringBuilder commandStrin

public override void AppendBatchHeader(StringBuilder commandStringBuilder)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));

// TODO: Npgsql
}

protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, ColumnModification columnModification)
protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, IColumnModification columnModification)
{
throw new NotImplementedException();
throw new NotSupportedException();
}

protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected)
{
throw new NotImplementedException();
throw new NotSupportedException();
}

public enum ResultsGrouping
Expand Down
29 changes: 29 additions & 0 deletions test/EFCore.PG.FunctionalTests/Migrations/MigrationsNpgsqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,35 @@ public override async Task Drop_column_primary_key()
@"ALTER TABLE ""People"" DROP COLUMN ""Id"";");
}

[ConditionalFact]
public override async Task Drop_column_computed_and_non_computed_with_dependency()
{
if (TestEnvironment.PostgresVersion.IsUnder(12))
{
return;
}

await Test(
builder => builder.Entity("People").Property<int>("Id"),
builder => builder.Entity(
"People", e =>
{
e.Property<int>("X");
e.Property<int>("Y").HasComputedColumnSql($"{DelimitIdentifier("X")} + 1", stored: true);
}),
builder => { },
model =>
{
var table = Assert.Single(model.Tables);
Assert.Equal("Id", Assert.Single(table.Columns).Name);
});

AssertSql(
@"ALTER TABLE ""People"" DROP COLUMN ""Y"";",
//
@"ALTER TABLE ""People"" DROP COLUMN ""X"";");
}

[Fact]
public virtual async Task Drop_column_system()
{
Expand Down
107 changes: 107 additions & 0 deletions test/EFCore.PG.FunctionalTests/NpgsqlValueGenerationScenariosTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.EntityFrameworkCore.ValueGeneration.Internal;
using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities;
using Xunit;

Expand Down Expand Up @@ -150,6 +152,12 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
.HasDefaultValue();
}

public class BlogWithStringKey
{
public string Id { get; set; }
public string Name { get; set; }
}

[Fact]
public void Insert_with_key_default_value_from_sequence()
{
Expand Down Expand Up @@ -194,6 +202,105 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
}
}

[ConditionalFact]
public void Insert_uint_to_Identity_column_using_value_converter()
{
using var testStore = NpgsqlTestStore.CreateInitialized(DatabaseName);
using (var context = new BlogContextUIntToIdentityUsingValueConverter(testStore.Name))
{
context.Database.EnsureCreatedResiliently();

context.AddRange(
new BlogWithUIntKey { Name = "One Unicorn" }, new BlogWithUIntKey { Name = "Two Unicorns" });

context.SaveChanges();
}

using (var context = new BlogContextUIntToIdentityUsingValueConverter(testStore.Name))
{
var blogs = context.UnsignedBlogs.OrderBy(e => e.Id).ToList();

Assert.Equal((uint)1, blogs[0].Id);
Assert.Equal((uint)2, blogs[1].Id);
}
}

public class BlogContextUIntToIdentityUsingValueConverter : ContextBase
{
public BlogContextUIntToIdentityUsingValueConverter(string databaseName)
: base(databaseName)
{
}

public DbSet<BlogWithUIntKey> UnsignedBlogs { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);

modelBuilder
.Entity<BlogWithUIntKey>()
.Property(e => e.Id)
.HasConversion<int>();
}
}

public class BlogWithUIntKey
{
public uint Id { get; set; }
public string Name { get; set; }
}

[ConditionalFact]
public void Insert_string_to_Identity_column_using_value_converter()
{
using var testStore = NpgsqlTestStore.CreateInitialized(DatabaseName);
using (var context = new BlogContextStringToIdentityUsingValueConverter(testStore.Name))
{
context.Database.EnsureCreatedResiliently();

context.AddRange(
new BlogWithStringKey { Name = "One Unicorn" }, new BlogWithStringKey { Name = "Two Unicorns" });

context.SaveChanges();
}

using (var context = new BlogContextStringToIdentityUsingValueConverter(testStore.Name))
{
var blogs = context.StringyBlogs.OrderBy(e => e.Id).ToList();

Assert.Equal("1", blogs[0].Id);
Assert.Equal("2", blogs[1].Id);
}
}

public class BlogContextStringToIdentityUsingValueConverter : ContextBase
{
public BlogContextStringToIdentityUsingValueConverter(string databaseName)
: base(databaseName)
{
}

public DbSet<BlogWithStringKey> StringyBlogs { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);

Guid guid;
modelBuilder
.Entity<BlogWithStringKey>()
.Property(e => e.Id)
.HasValueGenerator<TemporaryStringValueGenerator>()
.HasConversion(
v => Guid.TryParse(v, out guid)
? default
: int.Parse(v),
v => v.ToString())
.ValueGeneratedOnAdd();
}
}

[Fact]
public void Insert_with_explicit_non_default_keys()
{
Expand Down
Loading