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

Query: Don't read default value from databases #21668

Merged
merged 1 commit into from
Jul 18, 2020

Conversation

smitpatel
Copy link
Member

In 3.x when reading values from database, we started returning default value of expected type if the value was null in the database. That means if we needed to read int then database value was null then we got 0 back. Which is not correct as we should throw exception for the case.

The fix here is,

  • All value read from databases are of nullable type as database can return null.
  • Since all the bindings with database are nullable now,
    • When doing member access we add null check and return nullable of return type
    • When doing method call we add null check for instance. All parameters we convert to original types.
    • All other expressions, where they need to match the type to reconstruct the original tree, adds convert nodes.
  • For translation of Single/SingleOrDefault (and friends)
    • For scalar subquery
      • We add coalesce with default value for SingleOrDefault
      • We don't add coalesce for Single case and assume there will be non-null value coming out of database which we will try to assign.
    • For non-scalar subquery
      • We add default value in client side in shaper for SingleOrDefault
      • We throw exception Sequence contains no elements for Single case
    • Above rules are only when not returning entity type. When returning entity type, we don't have enough information to differentiate the case. Also above behavior is just side-effect of assigning values to properties which cannot take null values.
  • When doing DefaultIfEmpty over a non-reference type, navigation expansion will add client side coalesce so that we can move Selector after DefaultIfEmpty

Resolves #20633
Resolves #20959
Resolves #21002

@smitpatel smitpatel requested a review from maumar July 17, 2020 14:56
@smitpatel
Copy link
Member Author

smitpatel commented Jul 17, 2020

Pending TODOs (all done)

  • Still working on replicating same for InMemory/Cosmos
  • Update or remove tests which were incorrect due to 3.x behavior
  • Add exception messages in resources file.

In 3.x when reading values from database, we started returning default value of expected type if the value was null in the database. That means if we needed to read int then database value was null then we got 0 back. Which is not correct as we should throw exception for the case.

The fix here is,
- All value read from databases are of nullable type as database can return null.
- Since all the bindings with database are nullable now,
  - When doing member access we add null check for value types and return nullable of return type
  - When doing method call we add null check for instance. All parameters we convert to original types.
  - All other expressions, where they need to match the type to reconstruct the original tree, adds convert nodes.
- For translation of Single/SingleOrDefault (and friends)
  - For scalar subquery
    - We add coalesce with default value for SingleOrDefault
    - We don't add coalesce for Single case and assume there will be non-null value coming out of database which we will try to assign.
  - For non-scalar subquery
    - We add default value in client side in shaper for SingleOrDefault
    - We throw exception Sequence contains no elements for Single case
  - Above rules are only when not returning entity type. When returning entity type, we don't have enough information to differentiate the case. Also above behavior is just side-effect of assigning values to properties which cannot take null values.
- When doing DefaultIfEmpty over a non-reference type, navigation expansion will add client side coalesce so that we can move Selector after DefaultIfEmpty

Resolves #20633
Resolves #20959
Resolves #21002
@smitpatel smitpatel merged commit 3ebfde7 into release/5.0-preview8 Jul 18, 2020
@smitpatel smitpatel deleted the smit/ComplicatedMess branch July 18, 2020 12:40
@smitpatel smitpatel mentioned this pull request Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants