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

BufferedDataReader needs to be accommodating to incorrect values #20364

Closed
smitpatel opened this issue Mar 20, 2020 · 5 comments · Fixed by #22233
Closed

BufferedDataReader needs to be accommodating to incorrect values #20364

smitpatel opened this issue Mar 20, 2020 · 5 comments · Fixed by #22233
Assignees
@smitpatel
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-5.0 type-bug
Milestone
5.0.0

Comments

@smitpatel
Copy link
Member

While working on #20278, we enabled tests which has wrong data to verify we throw better exceptions. All the tests are working correctly for Sqlite. But for SqlServer where we use BufferedDataReader, the reader throws early exception while trying to read values. Hence the code not reach the place where we intercept exception to throw better message (that code is part of shaper), BufferedDataReader buffers before calling into shaper.
@roji roji mentioned this issue Mar 21, 2020
Closed
13 tasks
@ajcvickers ajcvickers added this to the 5.0.0 milestone Mar 23, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, Backlog Jun 10, 2020
@smitpatel smitpatel mentioned this issue Jun 16, 2020
Closed
@smitpatel
Copy link
Member Author

This requires passing enabledDetailedErrors to Execute method via RelationalCommandParameterObject which is public.

@AndriySvyryd
Copy link
Member

Adding a property to RelationalCommandParameterObject is not a breaking change.
@dotnet/efteam Does anyone think we shouldn't do this in 5.0?

@smitpatel
Copy link
Member Author

No new ctor to take value for that parameter?

@AndriySvyryd
Copy link
Member

No new ctor to take value for that parameter?

Yes, a new overload

@ajcvickers
Copy link
Member

I'm good with it.

smitpatel added a commit that referenced this issue Aug 26, 2020
@smitpatel
Throw detailed exception message in buffered data reader
79e693c 
Resolves #20364

Adds code for error handling when create buffered data reader.
Code needs to handle unexpected nulls and incorrect type of data encountered
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed consider-for-current-release labels Aug 26, 2020
@smitpatel smitpatel modified the milestones: Backlog, 5.0.0-rc1 Aug 26, 2020
@smitpatel smitpatel mentioned this issue Aug 26, 2020
Merged
smitpatel added a commit that referenced this issue Aug 26, 2020
@smitpatel
Throw detailed exception message in buffered data reader
b3a03ab 
Resolves #20364

Adds code for error handling when create buffered data reader.
Code needs to handle unexpected nulls and incorrect type of data encountered
@ghost ghost closed this as completed in #22233 Aug 26, 2020
ghost pushed a commit that referenced this issue Aug 26, 2020
@smitpatel
Throw detailed exception message in buffered data reader (#22233)
944864f 
Resolves #20364

Adds code for error handling when create buffered data reader.
Code needs to handle unexpected nulls and incorrect type of data encountered
@ajcvickers ajcvickers modified the milestones: 5.0.0-rc1, 5.0.0 Nov 7, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smitpatel smitpatel

Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-5.0 type-bug
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

Throw detailed exception message in buffered data reader dotnet/efcore
3 participants
@ajcvickers @smitpatel @AndriySvyryd