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.

Sign up for GitHub

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

Jump to bottom

Microsoft.Data.Sqlite: Support Half #30931

Open
bricelam opened this issue May 19, 2023 · 2 comments · May be fixed by #35328
Open

Microsoft.Data.Sqlite: Support Half #30931

bricelam opened this issue May 19, 2023 · 2 comments · May be fixed by #35328
Labels
area-adonet-sqlite good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone
Backlog

Comments

@bricelam
Copy link
Contributor

Similar to how we handle float, we could use REAL in the database for these values.
@bricelam bricelam added type-enhancement area-adonet-sqlite good first issue This issue should be relatively straightforward to fix. labels May 19, 2023
@ilmalte
Copy link
Contributor

ilmalte commented May 19, 2023

Hey @bricelam, may I give a try to work on this?

Do you see this implementation for Sqlite only?

If so, I was thinking about starting from this class and create the mapping for the type.
src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs

Then I would cover the new type in the test Create_and_clone_with_converter.

efcore/test/EFCore.Sqlite.Tests/Storage/SqliteTypeMappingTest.cs

Line 78 in 6b97368

public override void Create_and_clone_with_converter(Type mappingType, Type type)

Any guidance, suggestion or heads up?

@bricelam
Copy link
Contributor Author

bricelam commented May 19, 2023 •
edited
Loading

@ilmalte Sure, feel free to work on it! Yes, this issue is just about SQLite.

First, you'll need to add support for it in the ADO.NET provider. Just follow what we do for float. Here's the code for reading values: (Note, there's a bit of duplication required in this method.)

efcore/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs

Lines 202 to 205 in c983103

if (typeof(T) == typeof(float))
{
return (T)(object)GetFloat(ordinal);
}

And, here's the code for writing:

efcore/src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs

Lines 161 to 165 in c983103

else if (type == typeof(float))
{
var value = (double)(float)_value;
BindDouble(value);
}

Then yeah, for the EF Core changes, the type mapper is the right place.

We should also update the scaffolding code which uses heuristics to determine the CLR type:

efcore/src/EFCore.Sqlite.Core/Scaffolding/Internal/SqliteDatabaseModelFactory.cs

Lines 696 to 707 in c983103

if (_floatTypes.Contains(baseColumnType))
{
if (min >= float.MinValue
&& max <= float.MaxValue)
{
column["ClrType"] = typeof(float);
continue;
}
_logger.OutOfRangeWarning(column.Name, table.Name, "float");
}

@ajcvickers ajcvickers added this to the Backlog milestone May 24, 2023
ilmalte added a commit to ilmalte/efcore that referenced this issue Jul 21, 2023
@ilmalte
feat: support half for Sqlite
6fcd0d7 
dotnet#30931
@newmasterSG newmasterSG mentioned this issue Nov 25, 2024
Closed
6 tasks
@cincuranet cincuranet mentioned this issue Dec 11, 2024
Open
@newmasterSG newmasterSG linked a pull request Dec 13, 2024 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-adonet-sqlite good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Milestone
Backlog
3 participants
@bricelam @ajcvickers @ilmalte