Skip to content

Commit 077d561

Browse files
austindrenskiaustindrenski
committed
Address review by @roji 
1 parent e142aed commit 077d561

File tree

11 files changed

+71
-48
lines changed

11 files changed

+71
-48
lines changed

‎src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ public class NpgsqlNodaTimeTypeMappingSourcePlugin : IRelationalTypeMappingSourc
4141
/// <summary>
4242
/// Constructs an instance of the <see cref="NpgsqlNodaTimeTypeMappingSourcePlugin"/> class.
4343
/// </summary>
44-
public NpgsqlNodaTimeTypeMappingSourcePlugin()
44+
public NpgsqlNodaTimeTypeMappingSourcePlugin([NotNull] ISqlGenerationHelper sqlGenerationHelper)
4545
{
46-
_timestampLocalDateTimeRange = new NpgsqlRangeTypeMapping("tsrange", null, typeof(NpgsqlRange<LocalDateTime>), _timestampLocalDateTime);
47-
_timestampInstantRange = new NpgsqlRangeTypeMapping("tsrange", null, typeof(NpgsqlRange<Instant>), _timestampInstant);
48-
_timestamptzInstantRange = new NpgsqlRangeTypeMapping("tstzrange", null, typeof(NpgsqlRange<Instant>), _timestamptzInstant);
49-
_timestamptzZonedDateTimeRange = new NpgsqlRangeTypeMapping("tstzrange", null, typeof(NpgsqlRange<ZonedDateTime>), _timestamptzZonedDateTime);
50-
_timestamptzOffsetDateTimeRange = new NpgsqlRangeTypeMapping("tstzrange", null, typeof(NpgsqlRange<OffsetDateTime>), _timestamptzOffsetDateTime);
51-
_dateRange = new NpgsqlRangeTypeMapping("daterange", null, typeof(NpgsqlRange<LocalDate>), _date);
46+
_timestampLocalDateTimeRange = new NpgsqlRangeTypeMapping("tsrange", typeof(NpgsqlRange<LocalDateTime>), _timestampLocalDateTime, sqlGenerationHelper);
47+
_timestampInstantRange = new NpgsqlRangeTypeMapping("tsrange", typeof(NpgsqlRange<Instant>), _timestampInstant, sqlGenerationHelper);
48+
_timestamptzInstantRange = new NpgsqlRangeTypeMapping("tstzrange", typeof(NpgsqlRange<Instant>), _timestamptzInstant, sqlGenerationHelper);
49+
_timestamptzZonedDateTimeRange = new NpgsqlRangeTypeMapping("tstzrange", typeof(NpgsqlRange<ZonedDateTime>), _timestamptzZonedDateTime, sqlGenerationHelper);
50+
_timestamptzOffsetDateTimeRange = new NpgsqlRangeTypeMapping("tstzrange", typeof(NpgsqlRange<OffsetDateTime>), _timestamptzOffsetDateTime, sqlGenerationHelper);
51+
_dateRange = new NpgsqlRangeTypeMapping("daterange", typeof(NpgsqlRange<LocalDate>), _date, sqlGenerationHelper);
5252

5353
var storeTypeMappings = new Dictionary<string, RelationalTypeMapping[]>(StringComparer.OrdinalIgnoreCase)
5454
{

‎src/EFCore.PG/Infrastructure/Internal/NpgsqlOptionsExtension.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,15 @@ public override bool ApplyServices(IServiceCollection services)
9090
// See https://github.com/aspnet/EntityFrameworkCore/pull/10091
9191
public override int? MinBatchSize => base.MinBatchSize ?? 2;
9292

93+
/// <summary>
94+
/// Returns a copy of the current instance configured with the specified range mapping.
95+
/// </summary>
96+
[NotNull]
97+
public virtual NpgsqlOptionsExtension WithRangeMapping<TSubtype>(
98+
[NotNull] string rangeName,
99+
[CanBeNull] string subtypeName)
100+
=> WithRangeMapping<TSubtype>(rangeName, null, subtypeName);
101+
93102
/// <summary>
94103
/// Returns a copy of the current instance configured with the specified range mapping.
95104
/// </summary>

‎src/EFCore.PG/Metadata/PostgresRange.cs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public static PostgresRange GetOrAddPostgresRange(
3333
if (FindPostgresRange(annotatable, schema, name) is PostgresRange rangeType)
3434
return rangeType;
3535

36-
rangeType = new PostgresRange(annotatable, BuildAnnotationName(schema, name));
36+
rangeType = new PostgresRange(annotatable, BuildAnnotationName(annotatable, schema, name));
3737
rangeType.SetData(subtype, canonicalFunction, subtypeOpClass, collation, subtypeDiff);
3838
return rangeType;
3939
}
@@ -47,14 +47,22 @@ public static PostgresRange FindPostgresRange(
4747
Check.NotNull(annotatable, nameof(annotatable));
4848
Check.NotEmpty(name, nameof(name));
4949

50-
var annotationName = BuildAnnotationName(schema, name);
50+
var annotationName = BuildAnnotationName(annotatable, schema, name);
5151

5252
return annotatable[annotationName] == null ? null : new PostgresRange(annotatable, annotationName);
5353
}
5454

5555
[NotNull]
56-
static string BuildAnnotationName(string schema, string name)
57-
=> NpgsqlAnnotationNames.RangePrefix + (schema == null ? name : schema + '.' + name);
56+
static string BuildAnnotationName(IAnnotatable annotatable, string schema, string name)
57+
{
58+
if (schema != null)
59+
return $"{NpgsqlAnnotationNames.RangePrefix}{schema}.{name}";
60+
61+
if (annotatable[RelationalAnnotationNames.DefaultSchema] is string defaultSchema)
62+
return $"{NpgsqlAnnotationNames.RangePrefix}{defaultSchema}.{name}";
63+
64+
return $"{NpgsqlAnnotationNames.RangePrefix}{name}";
65+
}
5866

5967
[NotNull]
6068
public static IEnumerable<PostgresRange> GetPostgresRanges([NotNull] IAnnotatable annotatable)
@@ -66,8 +74,8 @@ public static IEnumerable<PostgresRange> GetPostgresRanges([NotNull] IAnnotatabl
6674
[NotNull]
6775
public Annotatable Annotatable => (Annotatable)_annotatable;
6876

69-
[NotNull]
70-
public string Schema => GetData().Schema;
77+
[CanBeNull]
78+
public string Schema => GetData().Schema ?? (string)_annotatable[RelationalAnnotationNames.DefaultSchema];
7179

7280
[NotNull]
7381
public string Name => GetData().Name;

‎src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,10 @@ protected override void Generate(
652652
Check.NotNull(operation, nameof(operation));
653653
Check.NotNull(builder, nameof(builder));
654654

655+
// Alter operations may not have a default schema attached. If not, try to attach one for schema-qualified types.
656+
if (operation[RelationalAnnotationNames.DefaultSchema] == null)
657+
operation[RelationalAnnotationNames.DefaultSchema] = model[RelationalAnnotationNames.DefaultSchema];
658+
655659
GenerateEnumStatements(operation, model, builder);
656660
GenerateRangeStatements(operation, model, builder);
657661

@@ -776,7 +780,7 @@ protected virtual void GenerateRangeStatements(
776780
foreach (var rangeTypeToDrop in PostgresRange.GetPostgresRanges(operation.OldDatabase)
777781
.Where(oe => PostgresRange.GetPostgresRanges(operation).All(ne => ne.Name != oe.Name)))
778782
{
779-
GenerateDropRange(rangeTypeToDrop, operation.OldDatabase, builder);
783+
GenerateDropRange(rangeTypeToDrop, builder);
780784
}
781785

782786
if (PostgresRange.GetPostgresRanges(operation).FirstOrDefault(nr =>
@@ -792,16 +796,14 @@ protected virtual void GenerateCreateRange(
792796
[NotNull] IModel model,
793797
[NotNull] MigrationCommandListBuilder builder)
794798
{
795-
var schema = GetSchemaOrDefault(rangeType.Schema, model);
796-
797799
// Schemas are normally created (or rather ensured) by the model differ, which scans all tables, sequences
798800
// and other database objects. However, it isn't aware of ranges, so we always ensure schema on range creation.
799-
if (schema != null)
800-
Generate(new EnsureSchemaOperation { Name = schema }, model, builder);
801+
if (rangeType.Schema != null)
802+
Generate(new EnsureSchemaOperation { Name=rangeType.Schema }, model, builder);
801803

802804
builder
803805
.Append("CREATE TYPE ")
804-
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, schema))
806+
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, rangeType.Schema))
805807
.AppendLine($" AS RANGE (")
806808
.IncrementIndent();
807809

@@ -825,16 +827,11 @@ protected virtual void GenerateCreateRange(
825827
.AppendLine(");");
826828
}
827829

828-
protected virtual void GenerateDropRange(
829-
[NotNull] PostgresRange rangeType,
830-
[CanBeNull] IAnnotatable oldDatabase,
831-
[NotNull] MigrationCommandListBuilder builder)
830+
protected virtual void GenerateDropRange([NotNull] PostgresRange rangeType, [NotNull] MigrationCommandListBuilder builder)
832831
{
833-
var schema = GetSchemaOrDefault(rangeType.Schema, oldDatabase);
834-
835832
builder
836833
.Append("DROP TYPE ")
837-
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, schema))
834+
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, rangeType.Schema))
838835
.AppendLine(";");
839836
}
840837

‎src/EFCore.PG/Storage/Internal/Mapping/NpgsqlRangeTypeMapping.cs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,41 +9,43 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping
99
{
1010
public class NpgsqlRangeTypeMapping : NpgsqlTypeMapping
1111
{
12-
[NotNull] static readonly NpgsqlSqlGenerationHelper SqlGenerationHelper =
13-
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies());
14-
15-
[CanBeNull] readonly string _storeTypeSchema;
16-
17-
[NotNull]
18-
public override string StoreType => SqlGenerationHelper.DelimitIdentifier(base.StoreType, _storeTypeSchema);
12+
[NotNull] readonly ISqlGenerationHelper _sqlGenerationHelper;
1913

2014
public RelationalTypeMapping SubtypeMapping { get; }
2115

16+
public NpgsqlRangeTypeMapping(
17+
[NotNull] string storeType,
18+
[NotNull] Type clrType,
19+
[NotNull] RelationalTypeMapping subtypeMapping,
20+
[NotNull] ISqlGenerationHelper sqlGenerationHelper)
21+
: this(storeType, null, clrType, subtypeMapping, sqlGenerationHelper) {}
22+
2223
public NpgsqlRangeTypeMapping(
2324
[NotNull] string storeType,
2425
[CanBeNull] string storeTypeSchema,
2526
[NotNull] Type clrType,
26-
[NotNull] RelationalTypeMapping subtypeMapping)
27-
: base(storeType, clrType, GenerateNpgsqlDbType(subtypeMapping))
27+
[NotNull] RelationalTypeMapping subtypeMapping,
28+
[NotNull] ISqlGenerationHelper sqlGenerationHelper)
29+
: base(sqlGenerationHelper.DelimitIdentifier(storeType, storeTypeSchema), clrType, GenerateNpgsqlDbType(subtypeMapping))
2830
{
29-
_storeTypeSchema = storeTypeSchema;
3031
SubtypeMapping = subtypeMapping;
32+
_sqlGenerationHelper = sqlGenerationHelper;
3133
}
3234

3335
protected NpgsqlRangeTypeMapping(
3436
RelationalTypeMappingParameters parameters,
3537
NpgsqlDbType npgsqlDbType,
36-
[CanBeNull] string storeTypeSchema,
37-
[NotNull] RelationalTypeMapping subtypeMapping)
38+
[NotNull] RelationalTypeMapping subtypeMapping,
39+
[NotNull] ISqlGenerationHelper sqlGenerationHelper)
3840
: base(parameters, npgsqlDbType)
3941
{
40-
_storeTypeSchema = storeTypeSchema;
4142
SubtypeMapping = subtypeMapping;
43+
_sqlGenerationHelper = sqlGenerationHelper;
4244
}
4345

4446
[NotNull]
4547
protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
46-
=> new NpgsqlRangeTypeMapping(parameters, NpgsqlDbType, _storeTypeSchema, SubtypeMapping);
48+
=> new NpgsqlRangeTypeMapping(parameters, NpgsqlDbType, SubtypeMapping, _sqlGenerationHelper);
4749

4850
protected override string GenerateNonNullSqlLiteral(object value)
4951
{

‎src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,17 @@ public class NpgsqlTypeMappingSource : RelationalTypeMappingSource
103103

104104
public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependencies,
105105
[NotNull] RelationalTypeMappingSourceDependencies relationalDependencies,
106+
[NotNull] ISqlGenerationHelper sqlGenerationHelper,
106107
[CanBeNull] INpgsqlOptions npgsqlOptions=null)
107108
: base(dependencies, relationalDependencies)
108109
{
109110
// Initialize some mappings which depend on other mappings
110-
_int4range = new NpgsqlRangeTypeMapping("int4range", null, typeof(NpgsqlRange<int>), _int4);
111-
_int8range = new NpgsqlRangeTypeMapping("int8range", null, typeof(NpgsqlRange<long>), _int8);
112-
_numrange = new NpgsqlRangeTypeMapping("numrange", null, typeof(NpgsqlRange<decimal>), _numeric);
113-
_tsrange = new NpgsqlRangeTypeMapping("tsrange", null, typeof(NpgsqlRange<DateTime>), _timestamp);
114-
_tstzrange = new NpgsqlRangeTypeMapping("tstzrange", null, typeof(NpgsqlRange<DateTime>), _timestamptz);
115-
_daterange = new NpgsqlRangeTypeMapping("daterange", null, typeof(NpgsqlRange<DateTime>), _timestamptz);
111+
_int4range = new NpgsqlRangeTypeMapping("int4range", typeof(NpgsqlRange<int>), _int4, sqlGenerationHelper);
112+
_int8range = new NpgsqlRangeTypeMapping("int8range", typeof(NpgsqlRange<long>), _int8, sqlGenerationHelper);
113+
_numrange = new NpgsqlRangeTypeMapping("numrange", typeof(NpgsqlRange<decimal>), _numeric, sqlGenerationHelper);
114+
_tsrange = new NpgsqlRangeTypeMapping("tsrange", typeof(NpgsqlRange<DateTime>), _timestamp, sqlGenerationHelper);
115+
_tstzrange = new NpgsqlRangeTypeMapping("tstzrange", typeof(NpgsqlRange<DateTime>), _timestamptz, sqlGenerationHelper);
116+
_daterange = new NpgsqlRangeTypeMapping("daterange", typeof(NpgsqlRange<DateTime>), _timestamptz, sqlGenerationHelper);
116117

117118
// Note that PostgreSQL has aliases to some built-in type name aliases (e.g. int4 for integer),
118119
// these are mapped as well.
@@ -252,7 +253,7 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc
252253
: throw new Exception($"Could not map range {rangeName}, no mapping was found for subtype {subtypeName}");
253254

254255
var rangeClrType = typeof(NpgsqlRange<>).MakeGenericType(subtypeClrType);
255-
var rangeMapping = new NpgsqlRangeTypeMapping(rangeName, schemaName, rangeClrType, subtypeMapping);
256+
var rangeMapping = new NpgsqlRangeTypeMapping(rangeName, schemaName, rangeClrType, subtypeMapping, sqlGenerationHelper);
256257
StoreTypeMappings[rangeMapping.StoreType] = new RelationalTypeMapping[] { rangeMapping };
257258
ClrTypeMappings[rangeMapping.ClrType] = rangeMapping;
258259
}

‎test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,6 +1711,7 @@ line line
17111711
Array.Empty<ITypeMappingSourcePlugin>()
17121712
),
17131713
new RelationalTypeMappingSourceDependencies(Array.Empty<IRelationalTypeMappingSourcePlugin>()),
1714+
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
17141715
options
17151716
);
17161717

‎test/EFCore.PG.Plugins.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ public void GenerateSqlLiteral_returns_period_literal()
118118

119119
#region Support
120120

121-
static readonly IRelationalTypeMappingSourcePlugin Mapper = new NpgsqlNodaTimeTypeMappingSourcePlugin();
121+
static readonly IRelationalTypeMappingSourcePlugin Mapper =
122+
new NpgsqlNodaTimeTypeMappingSourcePlugin(new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()));
122123

123124
static RelationalTypeMapping GetMapping(string storeType)
124125
=> Mapper.FindMapping(new RelationalTypeMappingInfo(storeType));

‎test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ public void GenerateSqlLiteral_returns_ranking_normalization_literal() => Assert
350350
Array.Empty<ITypeMappingSourcePlugin>()
351351
),
352352
new RelationalTypeMappingSourceDependencies(Array.Empty<IRelationalTypeMappingSourcePlugin>()),
353+
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
353354
new NpgsqlOptions()
354355
);
355356

‎test/EFCore.PG.Tests/Update/NpgsqlModificationCommandBatchFactoryTest.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public void Uses_MaxBatchSize_specified_in_NpgsqlOptionsExtension()
2727
Array.Empty<ITypeMappingSourcePlugin>()
2828
),
2929
new RelationalTypeMappingSourceDependencies(Array.Empty<IRelationalTypeMappingSourcePlugin>()),
30+
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
3031
new NpgsqlOptions()
3132
);
3233
var factory = new NpgsqlModificationCommandBatchFactory(
@@ -63,6 +64,7 @@ public void MaxBatchSize_is_optional()
6364
Array.Empty<ITypeMappingSourcePlugin>()
6465
),
6566
new RelationalTypeMappingSourceDependencies(Array.Empty<IRelationalTypeMappingSourcePlugin>()),
67+
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
6668
new NpgsqlOptions()
6769
);
6870
var factory = new NpgsqlModificationCommandBatchFactory(

0 commit comments

Comments
 (0)