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

Support client evaluation when store evaluation is not appropriate #10265

Closed
ajcvickers opened this issue Nov 10, 2017 · 8 comments
Closed

Support client evaluation when store evaluation is not appropriate #10265

ajcvickers opened this issue Nov 10, 2017 · 8 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement

Comments

@ajcvickers
Copy link
Member

This has come up a lot with discussion of value conversions (#242) but is also an issue with the SQLite ADO.NET provider (e.g. #10249 and #10198) and will be an issue for Oracle since it has a limited number of column types just like SQLite.

Things to consider:

  • Developers may want:
    • The sort order to be as it would be if run in .NET on the client
    • The sort order to be as it would be in the server, understanding that this is different due to
      • Conversion of values or value format
      • Different semantics on the server (e.g. collation)
    • Any sort order as long as it is deterministic
      • Even if it is less common for developers to want this, EF still often needs this
  • Forcing sorting on the client can have a big perf impact
  • Anything done to change this by default would be a breaking change--especially important for SQLite cases
  • In some cases it is possible to change the conversions to be order-preserving, but this often results in unnatural data in the database
  • Even if it cannot be guaranteed that order will be preserved, it could be that ordering is preserved for the subset of values actually used--consider positive/negative number spaces when converting to strings or byte arrays.

Things we could do:

  • Warn when this may be an issue
  • Allow developers to pick type mappings/conversions that preserve order
    • Decide whether this is a must or a preference. That is, if EF can't ensure order is preserved, then either:
      • Continue with a warning and use non-order preserving mapping with store ordering
      • Continue with a warning, but force ordering to be done client side
      • Throw and require that the developer make an explicit choice of how to proceed by configuring differently
  • Allow developers to use client-side ordering. This could be
    • Opt-in for any ordering
    • Automatically opted in when EF can't guarantee that order is preserved, but with a way to opt-out
@ajcvickers
Copy link
Member Author

ajcvickers commented Nov 20, 2017

Additional note from #10198: .NET ordering of GUIDs is different from SQL Server ordering of GUIDs.

@ajcvickers
Copy link
Member Author

Notes from triage:

  • Since there are already cases of order not being preserved even without any value conversions, it means that this is not an issue limited to value conversion and likely needs a solution in general type mapping rather than just in value conversion.
  • There are different things we could do, but it's not clear exactly when we should:
    • Force client eval
    • Use unnatural but order-preserving converters
    • Use store-side conversions, similar to what has been discussed for spatial types
    • Generate warnings or errors
  • There are other similar issues such as equality comparisons which may need similar treatment

Base on this, we will not build any of this explicitly into value converters and we are moving this issue to the backlog to consider it holistically in a future release.

@ajcvickers ajcvickers added this to the Backlog milestone Nov 21, 2017
@ajcvickers ajcvickers changed the title Support client-side ordering when store ordering would return different results Support client evaluation when store evaluation is not appropriate Dec 13, 2017
@ajcvickers
Copy link
Member Author

ajcvickers commented Dec 13, 2017

Also consider #10534.

@divega
Copy link
Contributor

divega commented Nov 17, 2018

Note for triage: TL;DR: Maybe it is just me, but it seems this issue has become too general and it includes some things we should keep in mind for 3.0.

Removing it from the backlog milestone so that we can char about it in triage.

Some of the things I have seen linked to this issue:

@ajcvickers
Copy link
Member Author

Added reference to this issue from guiding principles (#12795) issue.

@smitpatel
Copy link
Member

This would require from input from TypeMapping on ordering, (which should also take care of converters). For now, I believe query should treat them as if they are correct ordering with warning perhaps.

@ajcvickers
Copy link
Member Author

Notes from planning:

  • We will add ordering metadata to value conversions, which will then be available on the type mapping
  • If conversion is not order-preserving, then we throw if an order-based operation is used.
    • For example less than, greater than, min, max.
    • This could include order-by, but note, that commonly any fixed ordering can fine.
  • Add API to allow these operations anyway

@ajcvickers ajcvickers self-assigned this Jan 29, 2019
@ajcvickers ajcvickers removed their assignment May 10, 2019
@smitpatel smitpatel removed this from the 3.0.0 milestone Jun 5, 2019
@ajcvickers
Copy link
Member Author

Closing this in favor of #15979 and #15978 since we are not doing client evaluation (except in final projection) in 3.0

@ajcvickers ajcvickers added the closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. label Mar 10, 2022
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement
Projects
None yet
Development

No branches or pull requests

3 participants