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

[5.0.1] Query: Properly generate expression to read from BufferedDataReader #23295

Merged
merged 1 commit into from
Nov 17, 2020

Conversation

smitpatel
Copy link
Member

@smitpatel smitpatel commented Nov 12, 2020

Resolves #23282
Resolves #23104

Issue: When buffering is enabled, first time when we read particular property slot, we create expression to read into buffer and out of the buffer. We mistakenly bypassed changing expression to read from value buffer when same property slot was being read again, causing cast errors.

Description

Any type mapping having custom read expression from data reader throws an exception while retrying strategy is enabled. This was reported for SQL Server spatial types, but we know that Oracle uses the same pattern for some of its type mappings, so this is likely to be broken for Oracle too once they have an updated provider.

Customer Impact

When retrying strategy is enabled (which is recommended for all SQL Azure applications), querying for spatial types or any other non-primitive type will hit an exception.

How found

Customer reported on 5.0.

Test coverage

Added test for affected scenario.

Regression?

Yes, this worked in 3.1.

Risk

Low. This reverts back to 3.1 behavior in terms of generating correct expression.

@ajcvickers
Copy link
Member

@smitpatel Are you confident we don't need to test any other variations?

@smitpatel smitpatel changed the title Query: Properly generate expression to read from BufferedDataReader [5.0.1] Query: Properly generate expression to read from BufferedDataReader Nov 12, 2020
@smitpatel
Copy link
Member Author

I enabled retrying execution strategy for whole SqlServer functional tests and ran all tests and all passed (except obvious wrong ones in the tests which tests retrying strategy). Further looking at blame (which is on me), it was incorrect refactor and this code is pretty close to what 3.1 did. So I am quite confident.
Though I want @AndriySvyryd & @maumar both to review and approve.

@ajcvickers
Copy link
Member

Thanks @smitpatel. Makes sense.

@ajcvickers
Copy link
Member

Approved by Tactics for 5.0.1

Resolves #23282
Resolves #23104

Issue:
When buffering is enabled, first time when we read particular property slot, we create expression to read into buffer and out of the buffer.
We mistakenly bypassed changing expression to read from value buffer when same property slot was being read again, causing cast errors
Also matches the type being read out of the buffered data reader
@smitpatel smitpatel merged commit d185ae4 into release/5.0 Nov 17, 2020
@smitpatel smitpatel deleted the smit/AnTeeAs branch November 17, 2020 23:24
@ajcvickers ajcvickers removed this from the 5.0.1 milestone Dec 11, 2020
@LarysaMatsirka
Copy link

LarysaMatsirka commented Jul 12, 2022

smitpatelsmitpatel is it possible to add the same fox for v.3.1.7? Initially bug was reported for this version (efcore/EFCore.SqlServer.HierarchyId#24)
Maybe there is the patch in place for 3.x?

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

Successfully merging this pull request may close these issues.

5 participants