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

SqliteDataRecord.GetStream only works with 32-bit keys #27138

Closed
cocowalla opened this issue Jan 7, 2022 · 2 comments · Fixed by #27187
Closed

SqliteDataRecord.GetStream only works with 32-bit keys #27138

cocowalla opened this issue Jan 7, 2022 · 2 comments · Fixed by #27187
Assignees
@bricelam
Labels
area-adonet-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-bug
Milestone
7.0.0

Comments

@cocowalla
Copy link
Contributor

According to the SQLite docs, rowid is a signed 64-bit integer. However, SqliteDataRecord.GetStream treats it as a 32-bit integer: https://github.com/dotnet/efcore/blob/main/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs#L376

I've confirmed this is an issue - when using a 64-bit integer as PK, the call to GetInt32 can result in an overflow.

Microsoft.Data.Sqlite version: 6.0.0
Target framework: .NET 6.0
Operating system: Windows 10
@bricelam
Copy link
Contributor

bricelam commented Jan 11, 2022 •
edited
Loading

Indeed, that should be GetInt64. Looks like it's been this way since the original PR in 3.0.0. Feel free to send a PR to fix this if you're interested. Otherwise, we'll get around to it sometime in 7.0.

@bricelam bricelam added type-bug good first issue This issue should be relatively straightforward to fix. labels Jan 11, 2022
@bricelam bricelam added this to the 7.0.0 milestone Jan 11, 2022
@cocowalla
Copy link
Contributor Author

Sure, if I'll send a PR tomorrow if I remember! Thanks for the quick confirmation, BTW 👍

cocowalla added a commit to cocowalla/efcore that referenced this issue Jan 13, 2022
@cocowalla
fix: dotnet#27138, stream with 64-bit PK
d6d8587
@cocowalla cocowalla mentioned this issue Jan 13, 2022
Merged
bricelam pushed a commit to cocowalla/efcore that referenced this issue Jan 14, 2022
@cocowalla @bricelam
fix: dotnet#27138, stream with 64-bit PK
4a76535
@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 14, 2022
@ghost ghost closed this as completed in #27187 Jan 14, 2022
ghost pushed a commit that referenced this issue Jan 14, 2022
@cocowalla @bricelam
fix: #27138, stream with 64-bit PK (#27187)
82a0c46 
* fix: #27138, stream with 64-bit PK

* Clean up PR #27187

Co-authored-by: Brice Lambson <brice@bricelam.net>
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview1, 7.0.0 Nov 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bricelam bricelam

Labels
area-adonet-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-bug
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

fix: #27138, stream with 64-bit PK cocowalla/efcore
3 participants
@cocowalla @bricelam @ajcvickers