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 querying over primitive collections #30426

Closed
Tracked by #30731 ...
roji opened this issue Mar 7, 2023 · 5 comments · Fixed by #30738
Closed
Tracked by #30731 ...

Support querying over primitive collections #30426

roji opened this issue Mar 7, 2023 · 5 comments · Fixed by #30738
Assignees
Labels
area-primitive-collections area-query area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Mar 7, 2023

There are many query patterns involving arrays parameters which we don't translate (e.g. see #30425).

Doing this would involve a bit of common infrastructure in EF (e.g. support for table valued functions which aren't mapped to entity types). Then, on SQL Server specifically, we could transform the array to a JSON string on the client, and use OPENJSON to expand it out to a rowset; at that point it functions like a normal table, and we should be able to compose LINQ operators on it as usual (similar to #13617 (comment)).

Note that for collections which are inline in the query (e.g. new[] { 1, 2, 3 }), we can translate to SQL VALUES.

PS Pay special attention to the ordering guarantees of the rowset coming out of unnest; we probably need to add an ORDER BY to preserve it.
PPS npgsql/efcore.pg#2677 tracks doing this on PostgreSQL via the unnest function (without needing a JSON string intermediary)

@roji
Copy link
Member Author

roji commented Mar 8, 2023

Note: this relies on the ability to transform parameter values on the client-side before sending them (e.g. the array parameter is transformed into a JSON representation string of itself). General client-side parameter transformations are tracked by #28028.

The alternative would be to add a full type mapping for array types, value-converting them to their JSON representation. That has a much wider scope of change, and would also allow mapping primitve array properties to JSON columns (#29427)

@roji
Copy link
Member Author

roji commented Mar 8, 2023

Took a very quick look at this for PostgreSQL, npgsql/efcore.pg#2677 (AKA "it will be a fun little project!) and ran into the following limitations in EF:

  • This fails non-subquery translation because the argument to Count is a lambda, and RelationalSqlTranslatingExpressionVisitor.VisitLambda currently throws - we could change it to return NotTranslatedExpression instead.
  • However, even if we do that and go into subquery translation, QueryableMethodTranslatingEV only recognizes queryable operators, but in the above query they're enumerable. So that's not the right way forward.
  • For normal queries (e.g. ctx.Blogs.Where(b => b.Posts.Count(i => ...))), QueryableMethodNormalizingEV converts the enumerable chain to queryable; but is specifically refrains from doing so for closure variables. We can try removing that and do this even for enumerable closure variables.
  • However, NavigationExpandingExpressionVisitor converts the chain back to enumerable. We'd have to make changes to preserve the queryable chain there.

It's probably possible to fix all this, though we should think about the consequences. Doing all this means that all enumerable closure variables will start being treated as queryable.

@ajcvickers
Copy link
Member

Note from triage: this is the way.

@roji roji changed the title Translate array of scalars via OPENJSON Support queryable parameters (arrays of scalars) Mar 14, 2023
@roji roji self-assigned this Mar 14, 2023
@roji
Copy link
Member Author

roji commented Mar 27, 2023

We can also translate queryable constants, not just parameters:

_ = ctx.Blogs.Where(b => new[] { 1, 2 }.Count(i => i > b.Id))...

A constant table can be done in SQL via the VALUES expression; this seems universally supported in all databases, so it doesn't even require any sort of JSON packing/unpacking (as with parameters).

SQL Server

docs

SELECT * FROM (VALUES (1, 2), (3, 4)) AS x(a, b);

-- With explicit typing:
SELECT * FROM (VALUES (CAST(1 + 8 AS float), 2), (3, 4)) AS x(a, b);

SELECT a, b, SQL_VARIANT_PROPERTY(a, 'BaseType'), SQL_VARIANT_PROPERTY(b, 'BaseType')
FROM (VALUES (CAST(1 + 8 AS float), 2), (3, 4)) AS x(a, b); -- 1st column float, 2nd int

PostgreSQL

docs

SELECT * FROM (VALUES (1, 2), (3, 4)) AS x(a, b);

-- With explicit typing:
SELECT * FROM (VALUES (CAST(1 + 8 AS float), 2), (3, 4)) AS x(a, b);

SELECT a, b, pg_typeof(a), pg_typeof(b)
FROM (VALUES (CAST(1 + 8 AS float), 2), (3, 4)) AS x(a, b);

SQLite

docs

SELECT * FROM (VALUES (1, 2), (3, 4)) AS x;

-- With explicit typing:
SELECT * FROM (VALUES (CAST(1 + 8 AS text), 2), (3, 4)) AS x;

SQLite does not allow specifying column names in the VALUES expression, like the other databases do. The typical workaround is to wrap it in a SELECT (or CTE) to rename the auto-assigned column1, column2 to whatever is desired (link).

MariaDB

docs

SELECT * FROM (VALUES (1, 2), (3, 4)) AS x;

-- With explicit typing:
DROP TEMPORARY TABLE IF EXISTS tempp;
CREATE TEMPORARY TABLE tempp SELECT * FROM (VALUES (CAST(1 + 8 AS float), 2), (3, 4)) AS x;
DESC tempp;

MySQL

docs

Oracle

docs

@roji roji changed the title Support queryable parameters (arrays of scalars) Support querying over primitive collections Mar 30, 2023
@roji
Copy link
Member Author

roji commented Apr 4, 2023

As written above, SQLite/MySQL/MariaDB do not support column naming for VALUES; while SQLite just calls the column value, the naming in MySQL/MariaDB is even simply the value of the first row. Since we need to have a fixed/clear column name, the easiest is probably to split out the first row to a separate SELECT, and use UNION ALL: SELECT 1 AS foo UNION ALL VALUES (2), (3).

roji added a commit to roji/efcore that referenced this issue Apr 4, 2023
roji added a commit to roji/efcore that referenced this issue Apr 5, 2023
roji added a commit to roji/efcore that referenced this issue Apr 5, 2023
roji added a commit to roji/efcore that referenced this issue Apr 5, 2023
roji added a commit to roji/efcore that referenced this issue Apr 6, 2023
roji added a commit to roji/efcore that referenced this issue Apr 7, 2023
roji added a commit to roji/efcore that referenced this issue Apr 10, 2023
roji added a commit to roji/efcore that referenced this issue Apr 10, 2023
roji added a commit to roji/efcore that referenced this issue Apr 12, 2023
roji added a commit to roji/efcore that referenced this issue Apr 12, 2023
roji added a commit to roji/efcore that referenced this issue Apr 14, 2023
roji added a commit to roji/efcore that referenced this issue Apr 14, 2023
roji added a commit to roji/efcore that referenced this issue Apr 20, 2023
roji added a commit to roji/efcore that referenced this issue Apr 20, 2023
roji added a commit to roji/efcore that referenced this issue Apr 21, 2023
roji added a commit to roji/efcore that referenced this issue Apr 26, 2023
roji added a commit that referenced this issue Apr 26, 2023
@roji roji modified the milestones: Backlog, 8.0.0 Apr 26, 2023
roji added a commit to roji/efcore that referenced this issue Apr 27, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview4 Apr 27, 2023
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 27, 2023
roji added a commit that referenced this issue Apr 27, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview4, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-primitive-collections area-query area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants