-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Adding Half type support for SqlLite #35328
base: main
Are you sure you want to change the base?
Changes from 6 commits
f3b7014
9e92723
59d7712
bdf52d2
ce48282
983c5bf
ca1afda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Data; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using Microsoft.EntityFrameworkCore.Storage.Json; | ||
|
||
namespace Microsoft.EntityFrameworkCore.Storage | ||
{ | ||
/// <summary> | ||
/// <para> | ||
/// Represents the mapping between a .NET <see cref="Half" /> type and a database type. | ||
/// </para> | ||
/// <para> | ||
/// This type is typically used by database providers (and other extensions). It is generally | ||
/// not used in application code. | ||
/// </para> | ||
/// </summary> | ||
/// <remarks> | ||
/// See <see href="https://aka.ms/efcore-docs-providers">Implementation of database providers and extensions</see> | ||
/// for more information and examples. | ||
/// </remarks> | ||
public class HalfTypeMapping : RelationalTypeMapping | ||
{ | ||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public static HalfTypeMapping Default { get; } = new("REAL"); | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public HalfTypeMapping( | ||
string storeType, | ||
DbType? dbType = System.Data.DbType.Single) | ||
: this(new RelationalTypeMappingParameters( | ||
cincuranet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
new CoreTypeMappingParameters( | ||
typeof(Half), jsonValueReaderWriter: JsonHalfReaderWriter.Instance), | ||
storeType, | ||
dbType: dbType)) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
protected HalfTypeMapping(RelationalTypeMappingParameters parameters) : base(parameters) { } | ||
|
||
/// <inheritdoc/> | ||
protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) => new HalfTypeMapping(parameters); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Data; | ||
|
||
namespace Microsoft.EntityFrameworkCore.Sqlite.Storage.Internal | ||
{ | ||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public class SqlliteHalfTypeMapping : HalfTypeMapping | ||
cincuranet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public static new SqlliteHalfTypeMapping Default { get; } = new(SqliteTypeMappingSource.RealTypeName); | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public SqlliteHalfTypeMapping( | ||
string storeType, | ||
DbType? dbType = System.Data.DbType.Single) | ||
: base(storeType, dbType) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
protected SqlliteHalfTypeMapping(RelationalTypeMappingParameters parameters) | ||
: base(parameters) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// Creates a copy of this mapping. | ||
/// </summary> | ||
/// <param name="parameters">The parameters for this mapping.</param> | ||
/// <returns>The newly created mapping.</returns> | ||
protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) | ||
=> new SqlliteHalfTypeMapping(parameters); | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
protected override string GenerateNonNullSqlLiteral(object value) | ||
=> new DoubleTypeMapping(StoreType).GenerateSqlLiteral((double)(Half)value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this needs specific implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What did you mean specific? If value is null, this will generate an exception? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean not relying on |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Globalization; | ||
using System.Text.Json; | ||
|
||
namespace Microsoft.EntityFrameworkCore.Storage.Json | ||
{ | ||
/// <summary> | ||
/// Reads and writes JSON for Half type values. | ||
/// </summary> | ||
public sealed class JsonHalfReaderWriter : JsonValueReaderWriter<Half> | ||
{ | ||
private const string HalfFormatConst = "{0:0.0###}"; | ||
|
||
private static readonly PropertyInfo InstanceProperty = typeof(JsonHalfReaderWriter).GetProperty(nameof(Instance))!; | ||
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public static JsonHalfReaderWriter Instance { get; } = new(); | ||
|
||
private JsonHalfReaderWriter() | ||
{ | ||
|
||
} | ||
|
||
/// <inheritdoc/> | ||
public override Expression ConstructorExpression | ||
=> Expression.Property(null, InstanceProperty); | ||
|
||
/// <inheritdoc/> | ||
public override Half FromJsonTyped(ref Utf8JsonReaderManager manager, object? existingObject = null) => | ||
Half.Parse(manager.CurrentReader.GetString()!, CultureInfo.InvariantCulture); | ||
|
||
/// <inheritdoc/> | ||
public override void ToJsonTyped(Utf8JsonWriter writer, Half value) => | ||
writer.WriteStringValue(string.Format(CultureInfo.InvariantCulture, HalfFormatConst, value)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
cincuranet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
namespace Microsoft.EntityFrameworkCore.Storage.ValueConversion | ||
{ | ||
/// <summary> | ||
/// Converts double values to Half type. | ||
/// </summary> | ||
/// <remarks> | ||
/// See <see href="https://aka.ms/efcore-docs-value-converters">EF Core value converters</see> for more information and examples. | ||
/// </remarks> | ||
public class DoubleToHalfConverter : ValueConverter<double?, Half?> | ||
cincuranet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
private static readonly ConverterMappingHints DefaultHints = new(precision: 4); | ||
|
||
/// <summary> | ||
/// Creates a new instance of this converter. | ||
/// </summary> | ||
/// <remarks> | ||
/// See <see href="https://aka.ms/efcore-docs-value-converters">EF Core value converters</see> for more information and examples. | ||
/// </remarks> | ||
public DoubleToHalfConverter() | ||
: this(new()) | ||
{ | ||
|
||
cincuranet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// <summary> | ||
/// Creates a new instance of this converter. | ||
/// </summary> | ||
/// <remarks> | ||
/// See <see href="https://aka.ms/efcore-docs-value-converters">EF Core value converters</see> for more information and examples. | ||
/// </remarks> | ||
/// <param name="mappingHints"> | ||
/// Hints that can be used by the <see cref="ITypeMappingSource" /> to create data types with appropriate | ||
/// facets for the converted data. | ||
/// </param> | ||
public DoubleToHalfConverter(ConverterMappingHints mappingHints) | ||
: base( | ||
v => (Half)v!, | ||
v => v.HasValue | ||
? (double)v.Value | ||
: double.MinValue, | ||
DefaultHints.With(mappingHints)) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// A <see cref="ValueConverterInfo" /> for the default use of this converter. | ||
/// </summary> | ||
public static ValueConverterInfo DefaultInfo { get; } | ||
= new(typeof(double), typeof(Half), i => new DoubleToHalfConverter(i.MappingHints!), DefaultHints); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ public class SqliteTypeMappingSourceTest : RelationalTypeMappingSourceTestBase | |
[InlineData("TEXT", typeof(TimeSpan?), DbType.Time)] | ||
[InlineData("TEXT", typeof(decimal?), DbType.Decimal)] | ||
[InlineData("REAL", typeof(float?), DbType.Single)] | ||
[InlineData("REAL", typeof(Half?), DbType.Single)] | ||
[InlineData("REAL", typeof(double?), DbType.Double)] | ||
[InlineData("INTEGER", typeof(ByteEnum?), DbType.Byte)] | ||
[InlineData("INTEGER", typeof(ShortEnum?), DbType.Int16)] | ||
|
@@ -176,6 +177,7 @@ public void Does_mappings_for_store_type(string storeType, Type clrType, DbType? | |
[InlineData("REAL", typeof(float), DbType.Single)] | ||
[InlineData("UNREALISTIC", typeof(float), DbType.Single)] | ||
[InlineData("RUBBISH", typeof(float), DbType.Single)] | ||
[InlineData("RUBBISH", typeof(Half?), DbType.Single)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you're missing some values here for completeness. |
||
[InlineData("REAL", typeof(double), DbType.Double)] | ||
[InlineData("UNREALISTIC", typeof(double), DbType.Double)] | ||
[InlineData("RUBBISH", typeof(double), DbType.Double)] | ||
|
@@ -304,6 +306,7 @@ public void Does_default_mappings_for_values() | |
Assert.Equal("TEXT", CreateRelationalTypeMappingSource(model).GetMappingForValue(1.0m).StoreType); | ||
Assert.Equal("REAL", CreateRelationalTypeMappingSource(model).GetMappingForValue(1.0).StoreType); | ||
Assert.Equal("REAL", CreateRelationalTypeMappingSource(model).GetMappingForValue(1.0f).StoreType); | ||
Assert.Equal("REAL", CreateRelationalTypeMappingSource(model).GetMappingForValue((Half)1.0).StoreType); | ||
} | ||
|
||
[ConditionalFact] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Data.DbType.Single
is not correct here. Given that there's noHalf
inSystem.Data.DbType
, I think we can have just specific implementation for SQLite.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like new("Real")?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I think the
HalfTypeMapping
should not exist. Simply remove it and have only SQLite implementation.