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

Failing specification tests in SQL Server Compact (SQLCE) provider #6696

Closed
ErikEJ opened this issue Oct 6, 2016 · 10 comments
Closed

Failing specification tests in SQL Server Compact (SQLCE) provider #6696

ErikEJ opened this issue Oct 6, 2016 · 10 comments

Comments

@ErikEJ
Copy link
Contributor

ErikEJ commented Oct 6, 2016

Steps to reproduce

Run the relational specification tests against the SQL Server Compact database engine using the SQL CE EF Core provider

The issue

The tests listed below are failing with this error message:
Error is: System.Data.SqlServerCe.SqlCeException : There was an error parsing the query. [ Token line number = 5,Token line offset = 5,Token in error = SELECT ]

This is due to limitations in the SQL Server Compact engine, as discussed here:
#6145 (comment)

From AsyncQueryTestBase:
OrderBy_correlated_subquery_lol()
Where_query_composition()
Where_shadow_subquery_first()
SelectMany_primitive_select_subquery()

From ComplexNavigationsQueryTestBase:
Join_navigation_key_access_optional()
Join_navigation_in_inner_selector_translated_to_subquery()
Join_navigations_in_inner_selector_translated_to_multiple_subquery_without_collision()
Join_navigation_translated_to_subquery_non_key_join()
Join_navigation_translated_to_subquery_self_ref()
Where_navigation_property_to_collection2
Where_navigation_property_to_collection_of_original_entity_type
Correlated_subquery_doesnt_project_unnecessary_columns_in_top_level_join

From GearsOfWarQueryTestBase
Where_enum_has_flag_subquery
Where_count_subquery_without_collision
Where_subquery_boolean
Join_navigation_translated_to_subquery_composite_key

From IncludeTestBase
Include_collection_order_by_collection_column
Include_collection_order_by_subquery
Then_include_collection_order_by_collection_column

From QueryNavigationsTestBase
Project_single_scalar_value_subquery_is_properly_inlined
Select_collection_FirstOrDefault_project_single_column2
Select_collection_FirstOrDefault_project_single_column1
Select_count_plus_sum
Collection_orderby_nav_prop_count
Collection_orderby_nav_prop_count()
Collection_select_nav_prop_all()
Collection_select_nav_prop_any()
Collection_select_nav_prop_predicate()
Collection_select_nav_prop_count()
Collection_select_nav_prop_long_count()
Collection_select_nav_prop_sum()
Collection_where_nav_prop_count()
Collection_where_nav_prop_count_reverse()
Collection_where_nav_prop_sum()
Collection_where_nav_prop_sum_async()
Select_multiple_complex_projections()

From QueryTestBase
SelectMany_primitive_select_subquery
Where_query_composition
Where_query_composition_entity_equality_one_element_FirstOrDefault
Where_query_composition_entity_equality_no_elements_FirstOrDefault
Where_query_composition_entity_equality_multiple_elements_FirstOrDefault
Where_shadow_subquery_first
Select_Where_Subquery_Deep_First
OrderBy_any
OrderBy_correlated_subquery_lol()
OrderBy_correlated_subquery_lol2
Does_not_change_ordering_of_projection_with_complex_projections

Further technical details

EF Core version: 1.1.0-alpha1-22167
Operating system: Windows 10
Visual Studio version: (e.g. VS 2013 or n/a): VS 2015 Update 3

@smitpatel @divega - thanks for taking a second look at this, any help/advice/gist/PRs very much appreciated - I think I am not smart enough to figure this out without your help 😄

@divega
Copy link
Contributor

divega commented Oct 25, 2016

@ErikEJ some comments on this:

  1. One new place in which we are introducing scalar subqueries (i.e. SELECT statements that are supposed to return a single value) in the default relational SQL generation in EF Core 1.1 is to make the translation of OrderBy() work when applied to constants or uncorrelated parameters: We translate it to ORDER BY (SELECT 1) in DefaultQuerySqlGenerator.GenerateOrdering().
    Unlike the case for GenerateLimitOffset() in which we now use a similar scalar subquery in the SQL Server provider, we did this in the base relational provider because it solves a problem for most databases.
    But since SQL CE does not support scalar subqueries you could probably override the base provider and apply the same approach that you used for GenerateLimitOffset(): generate ORDER BY GETDATE().
    Note that the method GenerateOrdering() was recently introduced by @roji to support a different scenario in PostgreSQL, but if you need to refactor the base provider further to produce your own SQL in this case, please let us know or send a PR.

  2. There are a number of other places in which we generate scalar subqueries in other parts of the SQL statement. Some existed already in 1.0.0 but some may be new. As far as I remember there were places like conditions in the WHERE clause in which you were able to replace = comparisons to with IN( ) to make it work (see the conversation at Fix "Relational: Lift sub-queries in Where and OrderBy clauses" breaks SQLCE #2626 (comment)).

  3. Yet there are some other places in which scalar subqueries are used to obtain values rather than evaluate conditions and you cannot apply that same approach, e.g. if a query follows a pattern like this:

    select c.id, c.name
    from customers as c
    order by (select count(*) from orders where customerId = c.Id)

    I have been looking a bit at those and I am not sure they can be solved for all versions of SQL CE, but I think in theory any query with a scalar subquery could be rewritten to obtain the same results using the OUTER APPLY operator, which is supported in SQL CE 4.0. E.g. the query above becomes something like this:

    select c.id, c.name
    from customers as c
    outer apply (select count(*) as scalar from orders where customerid = c.id) as scalarquery
    order by scalar

    I have tried this transformation on a few queries with SQL CE 4.0 and it seems to work, even for the SELECT 1 case. In fact it is unfortunate that SQL CE 4.0 doesn't support scalar subqueries given that it appears to be shortcut syntax for OUTER APPLY used in this way. Not sure how to best leverage this solution but wanted to share it with you in case it helps as a starting point.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Oct 25, 2016

Hi @divega thanks for your comments.

Re 1: So you suggest I override "GenerateOrdering" and replace the SQL in this line https://github.com/aspnet/EntityFramework/blob/dev/src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs#L399 with for example:

  GETDATE()

Re 2: I think you misunderstood the previous conversation - I have not been smart enough to figure out how to implement the IN solution - would be very grateful for any suggestions/code samples to help me on the way! (I did test the IN solution with raw SQL, and that was what I reported as "working")

re 3: If OUTER APPLY works, how would I start using it - you seem to be very unsure?? Could you use it in EF.Relational ??

@smitpatel
Copy link
Contributor

@ErikEJ
Re 1: You can override GenerateOrdering function as whole or you can just process for the special case and call the base otherwise. For SQL CE, that would mean that for Constant or Parameter generate appropriate sql otherwise pass it to base as is.

Re 2:
https://github.com/aspnet/EntityFramework/blob/dev/src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs#L1141
The VisitBinary function would be covering most of the issues. So in that function, you need to check if either side is SelectExpression (since it is binary expression, presence of select is scalar subquery), then you need to generate SQL with IN operator instead of the regular operator flow. (you may need to flip left & right if left side is scalar subquery). That would take care of blocking case for CE, In all other cases just call base.

Re 3: For the case of outer apply, you will probably need to transform SelectExpression before SQL is generated.

ErikEJ pushed a commit to ErikEJ/EntityFramework.SqlServerCompact that referenced this issue Nov 9, 2016
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 10, 2016

Note to self: Overrdie "SqlTranslatingExpression" and force client eval, if Expression type is not supported

@smitpatel
Copy link
Contributor

With #6911 Now it is possible for providers to override SqlTranslatingExpressionVisitor. This is the main translator which convert fragments of query model into SQL. When it fails to convert to Sql, that tells our framework to do client evaluation for that part.
The main entry point of this class Visit method, generates result here.
https://github.com/aspnet/EntityFramework/blob/dev/src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/SqlTranslatingExpressionVisitor.cs#L124
SqlCE can override this method and call base one to get what is the translation of given node and then inspect the returned value and decide if that is valid sql fragment. If it is not then just drop the value and return null. Which would cause client evaluation for that part.
Also there are various other methods in this function which can be overridden to improve SQL translation.
e.g. In VisitBinary method, SqlCE can call base binary method, if the returned value is equality comparison with select expression, it can be converted to InExpression and passed along in rest of pipeline.
On higher level, what @divega suggested in part 3, when we have a subquery generated being used in order by or projection, we can convert it to a cross join query, in this derived class only, we also have access to select expression. So we take the scalar subquery, add a cross join with in main select expression and then use alias from that table reference wherever we needed scalar subquery.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 11, 2016

@smitpatel Thanks sooo much for all your help with this! As you can see above, we have made great progress already. Any idea when #6911 will be avilable in the aspnet-dev feed on myget?

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jan 14, 2017

I have added a DbContextOption that allows users to work around this on a case by case basis, so closing for now, thanks for help @divega and @smitpatel
https://github.com/ErikEJ/EntityFramework.SqlServerCompact/blob/master/src/Provider40/Query/ExpressionVisitors/SqlCeTranslatingExpressionVisitor.cs#L20

@ErikEJ ErikEJ closed this as completed Jan 14, 2017
@divega divega added closed-no-further-action The issue is closed and no further action is planned. and removed type-investigation labels May 8, 2017
@divega divega removed this from the 2.0.0-preview1 milestone May 8, 2017
@divega
Copy link
Contributor

divega commented Nov 28, 2018

Reopening so that we remember discussing in our next triage if there is anything we want to do for scalar subqueries (like I described in #6696 (comment)) in the future.

@divega divega reopened this Nov 28, 2018
@divega divega removed the closed-no-further-action The issue is closed and no further action is planned. label Nov 28, 2018
@ajcvickers
Copy link
Contributor

Pinging @divega

@ajcvickers
Copy link
Contributor

Discussed again in triage. We're still not going to do anything specific in the core code, but we could consider contributing to the CE provider under the right set of circumstances. We will follow up with @ErikEJ if we decide this makes sense.

@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
Projects
None yet
Development

No branches or pull requests

5 participants