Skip to content

Add Half type support for SQLite (#30931)#37481

Open
kimjaejung96 wants to merge 4 commits into
dotnet:mainfrom
kimjaejung96:users/kimjaejung96/half-type-sqlite-30931
Open

Add Half type support for SQLite (#30931)#37481
kimjaejung96 wants to merge 4 commits into
dotnet:mainfrom
kimjaejung96:users/kimjaejung96/half-type-sqlite-30931

Conversation

@kimjaejung96

@kimjaejung96 kimjaejung96 commented Jan 11, 2026

Copy link
Copy Markdown
  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

Summary

Add Half (16-bit floating point) type support for Microsoft.Data.Sqlite and EF Core SQLite provider.

Fixes #30931

Changes

Microsoft.Data.Sqlite (ADO.NET layer)

File Change
SqliteValueReader.cs Add GetFieldValue<Half> support
SqliteValueBinder.cs Add Half binding + type mapping

EF Core SQLite Provider

File Change
SqliteHalfTypeMapping.cs New - SQLite-specific type mapping
SqliteTypeMappingSource.cs Register Half mapping

Tests

File Change
SqliteDataReaderTest.cs Half read/write tests
SqliteTypeMappingSourceTest.cs CLR/Store type mapping tests (including UNREALISTIC)
SqliteTypeMappingTest.cs SQL literal generation + clone test

Design Notes

Based on #35328 feedback:

  1. SQLite-specific only - No changes to EFCore.Relational (no HalfTypeMapping in common layer)
  2. No DbType - System.Data.DbType has no Half value, so DbType is null
  3. Float pattern - Follows existing float implementation (read via GetFloat(), bind via BindDouble())
  4. Conditional compilation - Uses #if NET6_0_OR_GREATER for netstandard2.0 compatibility

@kimjaejung96 kimjaejung96 requested a review from a team as a code owner January 11, 2026 06:04
bricelam
bricelam reviewed Jan 12, 2026
View reviewed changes

@bricelam bricelam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking good to me.

You should also add logic for Half to SqliteDatabaseModelFactory.

Comment thread src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs Outdated
@AndriySvyryd

AndriySvyryd commented Jan 12, 2026

Copy link
Copy Markdown
Member

For reference, the previous attempt was #35328

@kimjaejung96

kimjaejung96 commented Jan 12, 2026

Copy link
Copy Markdown
Author

Thanks again @bricelam — I pushed updates addressing the feedback:

  • SqliteValueReader: Half now uses GetDouble
  • Added Half handling to SqliteDatabaseModelFactory for scaffolding

(Also, thanks to AndriySvyryd for pointing me to the previous attempt.)

bricelam
bricelam approved these changes Jan 13, 2026
View reviewed changes

@bricelam bricelam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread src/EFCore.Sqlite.Core/Scaffolding/Internal/SqliteDatabaseModelFactory.cs Outdated
@roji roji force-pushed the main branch 2 times, most recently from 249ae47 to 6b86657 Compare January 13, 2026 17:46
@kimjaejung96 kimjaejung96 requested a review from bricelam January 17, 2026 01:38
@AndriySvyryd AndriySvyryd assigned AndriySvyryd and unassigned roji Jun 9, 2026
@AndriySvyryd AndriySvyryd requested a review from Copilot June 9, 2026 16:30
@AndriySvyryd

AndriySvyryd commented Jun 9, 2026

Copy link
Copy Markdown
Member

@kimjaejung96 Sorry for the delay. Please rebase on latest main and use [Fact] insted of [ConditionalFact]

Copilot AI reviewed Jun 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds System.Half (16-bit floating point) support across the Microsoft.Data.Sqlite ADO.NET layer and the EF Core SQLite provider, enabling reading/writing Half values and mapping them to SQLite REAL.

Changes:

  • Added GetFieldValue<Half> handling in Microsoft.Data.Sqlite and parameter binding support for Half (guarded by #if NET6_0_OR_GREATER).
  • Introduced SqliteHalfTypeMapping and registered it in SqliteTypeMappingSource for EF Core SQLite.
  • Updated scaffolding type inference/default parsing for HALF, and added provider + ADO.NET tests for mapping and SQL literal generation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs Adds tests validating GetFieldValue<Half> behavior.
test/EFCore.Sqlite.Tests/Storage/SqliteTypeMappingTest.cs Adds SQL literal generation test for Half.
test/EFCore.Sqlite.Tests/Storage/SqliteTypeMappingSourceTest.cs Extends mapping coverage tests to include Half/Half?.
src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs Adds Half support in GetFieldValue<T> under NET6_0_OR_GREATER.
src/Microsoft.Data.Sqlite.Core/SqliteValueBinder.cs Adds Half parameter binding and type inference under NET6_0_OR_GREATER.
src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs Registers the new Half mapping for CLR type Half.
src/EFCore.Sqlite.Core/Storage/Internal/SqliteHalfTypeMapping.cs New type mapping for Half stored as SQLite REAL.
src/EFCore.Sqlite.Core/Scaffolding/Internal/SqliteDatabaseModelFactory.cs Adds scaffolding recognition/inference/default parsing support for HALF.

Comment thread src/EFCore.Sqlite.Core/Storage/Internal/SqliteHalfTypeMapping.cs
Comment on lines +30 to +35
public SqliteHalfTypeMapping(string storeType)
: base(
new RelationalTypeMappingParameters(
new CoreTypeMappingParameters(typeof(Half)),
storeType))
{

@AndriySvyryd AndriySvyryd Jun 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kimjaejung96 Can you also do this? You can add the JsonHalfReaderWriter in Core and also return it from https://github.com/dotnet/efcore/blob/main/src/EFCore/Storage/Json/JsonValueReaderWriterSource.cs#L37

@kimjaejung96
Add Half type support for SQLite
8c8a79d 
- Add Half support in SqliteValueReader.GetFieldValue<T>
- Add Half binding in SqliteValueBinder
- Add SqliteHalfTypeMapping for EF Core SQLite provider
- SQLite-specific implementation only (per maintainer feedback)
- Use REAL store type, DbType is null (no DbType.Half exists)

Fixes dotnet#30931
@kimjaejung96
Address PR review feedback
19958d2 
- Use GetDouble instead of GetFloat for Half reading (avoids extra conversion)
- Add Half scaffolding support to SqliteDatabaseModelFactory
  - Add HALF, FLOAT16 type hints for Half CLR type mapping
  - Add Half range validation in REAL value type handling
  - Add Half default value parsing
@kimjaejung96
Remove FLOAT16 from Half type hints
2a0199d 
Per reviewer feedback, keep only HALF for consistency with other
floating-point type hints (SINGLE for float).
@kimjaejung96
Use Fact for Half literal test
66dff1a
@kimjaejung96 kimjaejung96 force-pushed the users/kimjaejung96/half-type-sqlite-30931 branch from d2c4353 to 66dff1a Compare June 14, 2026 08:55
@kimjaejung96

kimjaejung96 commented Jun 14, 2026

Copy link
Copy Markdown
Author

Thanks for the review. I've rebased onto the latest main and changed the new Half literal test from [ConditionalFact] to [Fact].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@AndriySvyryd AndriySvyryd Awaiting requested review from AndriySvyryd

@wtgodbe wtgodbe Awaiting requested review from wtgodbe

+1 more reviewer

@bricelam bricelam bricelam approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@AndriySvyryd AndriySvyryd

Labels

community-contribution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Microsoft.Data.Sqlite: Support Half

5 participants

@kimjaejung96 @AndriySvyryd @bricelam @roji