FromSql(): Support passing a preamble SQL string

[divega]

Currently whenever FromSql() detects a SQL string that does not start with the word SELECT, it treat it as a non-composable query and it evaluates the rest of the query on the client side.

Some SQL constructs such as Common Table Expressions are fully composable but cannot be treated like normal SELECT statements in that they cannot be simply pushed down inside FROM clauses to become derived tables for a new SELECT.

It should be possible to support composability with CTEs if they could be specified separately from the SELECT, e.g. consider the following LINQ query (based on issue #4780):

        var peamble = 
@"with postsCTE as (
   select Id, ReplyToPostId
   from Posts
   where Id = {0}
   union all
   select Parent.Id, Parent.ReplyToPostId
   from Posts Parent
     join postsCTE Son on Son.ReplyToPostId = Parent.Id)";

        var sql = 
@"select * 
from HierarchyPosts 
where RootId =
    (select top 1 Id 
    from PostsCTE 
    where ReplyToPostId is null)";

        var query = context.HierarchyPosts
            .FromSqlWithPreamble(preamble, query, postId)
            .OrderBy(x => x.ReplyToPostId ?? -1);

For it, we would need to generate SQL similar to this:

with postsCTE as (
   select Id, ReplyToPostId
   from Posts
   where Id = @p0
   union all
   select Parent.Id, Parent.ReplyToPostId
   from Posts Parent
     join postsCTE Son on Son.ReplyToPostId = Parent.Id)

SELECT *
FROM (select * 
    from HierarchyPosts 
    where RootId =
        (select top 1 Id 
        from PostsCTE 
        where ReplyToPostId is null)) AS q0
ORDER BY COALESCE(q0.ReplyToPostId, -1)

[bricelam]

Reading up on WITH, these outlandish recursive queries in the SQLite docs are pretty epic.

[amattar]

any update on this?

[bricelam]

If you're interested in contributing, start here in RelationalEntityQueryableExpressionVisitor.

[tjrobinson]

I've noticed a variation of this problem with simple queries (i.e. no CTE) where the query starts with a comment instead of the word SELECT.

In my example I'm reading the query as a string from an embedded resource then executing it with FromSql().

[roji]

With the decision to stop doing client evaluation (#14935), does it make sense to stop doing any checks on the SQL altogether? If the user provides a non-composable SQL, they'd simply get a database error, which seems like the right thing...

[khellang]

I'm hitting this as well. It would be nice to just stop validating SQL client-side and let the DB handle it, as @roji suggests...

[roji]

Clearing milestone so we can discuss in triage.

[divega]

@roji @khellang is the discussion to have in triage about the feature represented by this issue (enabling composability for SQL queries that require a preamble) or about not bothering trying to tell composable from non-composaable SQL. I see both as largely separate. Also I remember vaguely that @smitpatel was already tracking the latter somewhere else.

[roji]

@divega I think it's about the latter - removing any attempt to parse SQL and to identify composable vs. non-composable SQL. I took a quick look but couldn't find an issue tracking this - @smitpatel unless you have one I missed, I can open a new one.

[smitpatel]

• There is no tracking issue.
• It is fixed since in the absence of client eval we are not going to verify if we have to do client composition.

I am fine either way - tracking issue or not.

[roji]

OK, so I propose to close this issue and open another one tracking the removal of the composability check. Does that sound OK @divega @ajcvickers?

[smitpatel]

@roji - negative. Even if the composability check is removed, AFAIK the functionality being talked about in this issue is still not possible. Without the check, we will just put it inside subquery when being composed over. And I am not sure WITH inside a subquery works.

[roji]

Ah, I think I understand now (after reading this again) - sorry for the confusion. Returning to the backlog.

Am not sure that an issue for removing the SQL composability checks is needed, but opened #15392 to track that.

[smitpatel]

@roji - Thank you for contributing to my bucket of 3.0 issues. :trollface:

[roji]

Always a pleasure :trollface: :trollface: :trollface:

Although this one seems simple enough that I could do it too..

[divega]

At least we can close https://github.com/aspnet/EntityFrameworkCore/issues/15392 as fixed, presumably :smile:

[sdanyliv]

Maybe this documentation can help you in designing CTE API, it already works in production. https://linq2db.github.io/articles/sql/CTE.html I'm not a big expert in relinq, but I hope it is doable.

Also there can be more than one CTE in query, and they need to be properly ordered by topological sort, so just adding additional FromSql implementation may introduce new bugs after query optimization.