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

Microsoft.Data.Sqlite: Improve SqliteDataReader.GetFieldType() when NULL #14315

Merged
merged 1 commit into from
Jun 25, 2019
Merged

Microsoft.Data.Sqlite: Improve SqliteDataReader.GetFieldType() when NULL #14315

merged 1 commit into from
Jun 25, 2019

Conversation

AlexanderTaeschner
Copy link
Contributor

@AlexanderTaeschner AlexanderTaeschner commented Jan 3, 2019

In case a column value is NULL, SqliteDataReader.GetFieldType() now tries to return the field type based on the column affinity.
SQLite does not provide the affinity via its API, therefore the corresponding C function sqlite3AffinityType was ported to C#. The affinity NUMERIC has no corresponding data type, so here the data type TEXT is returned (as suggested as default in #13831).
A special case is a column where no type is specified, here the explicit rule was used, that such a column is of affinity BLOB (see Determination Of Column Affinity).
The corresponding changes were also introduced for the SqliteDataReader.GetSchemaTable() DataType column.

Fixes #13831 and fixes #13839

@bricelam bricelam self-assigned this Jan 4, 2019
@seesharper
Copy link

Any updates on this. Will this make it into the next release?

@bricelam
Copy link
Contributor

Haven’t had a chance to review it yet. It will make it into 3.0.0, but not the next preview.

Also, I’m still not sure if we want TEXT or BLOB to be the ultimate fallback.

Copy link
Contributor

@bricelam bricelam left a comment

Choose a reason for hiding this comment

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

Finally getting around to this... 😁

src/Microsoft.Data.Sqlite.Core/SqliteDataReader.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@bricelam bricelam left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@bricelam
Copy link
Contributor

(Rebased on top of master)

@bricelam bricelam merged commit 01c26cc into dotnet:master Jun 25, 2019
@AlexanderTaeschner AlexanderTaeschner deleted the Issue13831 branch June 25, 2019 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants