Add RelationalDbSet for raw query #1734

mikary · 2015-03-02T21:20:29Z

Add custom relational queries off of DbSet to provide functionality for #624
@ajcvickers @anpete @divega

anpete · 2015-03-02T21:24:59Z

src/EntityFramework.Core/Utilities/ServiceProviderExtensions.cs

@@ -11,7 +11,7 @@

 namespace Microsoft.Framework.DependencyInjection
 {
-    internal static class ServiceProviderExtensions
+    public static class ServiceProviderExtensions


If you need these, share them, rather than making this public.

anpete · 2015-03-02T21:25:51Z

Can you please describe what's in this change?

anpete · 2015-03-02T22:45:01Z

...rk.Relational/Query/ExpressionTreeVisitors/RelationalEntityQueryableExpressionTreeVisitor.cs

@@ -148,5 +158,82 @@ var keyFactory
                    Expression.Call(queryMethodInfo, queryMethodArguments),
                    _readerParameter));
        }
+
+        protected Expression VisitRelationalCustomQueryable(string sql, Type elementType)


Don't duplicate the code below. Refactor both into something that pivots on the source table expr.

Would we want an optional expression parameter so the existing code path can use the element type as it was but we still have a way to pass in the query string?

No, just extract a new method and call it from both places. It may be DRY'er to have the new method take a Func as a factory for the table expr. but I would see what it looks like to just build the table expr. externally first.

mikary · 2015-03-05T00:28:51Z

Updated based on PR feedback

maumar · 2015-03-05T01:24:26Z

src/EntityFramework.Core/Query/EntityQueryable`.cs

+
+        public override string ToString()
+        {
+            return base.ToString() + string.Join(", ", _annotatable.Value.Annotations.Select(annotation => annotation.Value));


fix the formatting - e.g. space between base.ToString() and annotations, maybe wrap annotations in some kind of parenthesis.

mikary · 2015-03-09T21:58:06Z

More updates based on PR input

divega · 2015-03-10T17:49:37Z

test/EntityFramework.SqlServer.FunctionalTests/FromSqlQuerySqlServerTest.cs

+
+SELECT [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[CustomerID], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM (
+    SELECT * FROM Customers WHERE Customers.City = 'Seattle'


If the raw SQL query contains multiple lines, will they all be indented the same?

There isn't any logic in place to do this right now, did we want to try?

Should be easy if you use a StringReader to read each line and append to the output.

Would we need to worry about literal line-breaks in quoted strings etc.?

I don't think you can do that - at least in TSQL.

Maybe it is not too bad, but the alternative is to not worry about it at all and not try to indent raw SQL.

divega · 2015-03-10T17:54:03Z

@mikary @anpete @maumar Is there any more reviewing happening on this or are we good with it? I think we should figure out handling parameters (unless I am missing something) but that could go in a different commit.

maumar · 2015-03-10T18:01:15Z

src/EntityFramework.Relational/Query/Expressions/RawSqlDerivedTableExpression.cs

+
+        public override string ToString()
+        {
+            return Sql + " " + Alias;


"(" + Sql + ") AS " + Alias?

That doesn't seem very DRY to me, since this would be guessing at the interpretation of the SQL generator that consumes this object.

The current implementation is consistent with the other TableExpressions, but we could revisit this if it's important / useful enough.

This is just for debugging so do whatever makes it clear.

Added #1795 to track this

mikary · 2015-03-10T20:31:26Z

Updated based on feedback

anpete · 2015-03-10T20:32:01Z

src/EntityFramework.Core/Query/EntityQueryable`.cs

+
+        public virtual Annotation AddAnnotation([NotNull] string annotationName, [NotNull] string value)
+        {
+            Check.NotNull(annotationName, nameof(annotationName));


That's a stronger constraint than we have on Annotatable itself, do we want to update that as well?

Yes, seems like an oversight.

mikary · 2015-03-12T00:13:11Z

Rebased, squashed, updated, and ready for another pass.
Of note:

Simplified extension method
DataStore now has an InSupported(IAnnotation) method that is checked when the query is compiled
Simplified Query Tree Visitor (added IAnnotatable parameter to constant expression and simplified implementation logic)

mikary · 2015-03-13T20:56:38Z

After talk with @anpete, looking at using QueryAnnotations to store the FromSql string and parameters.

divega · 2015-03-13T21:23:12Z

@mikary @anpete Are we going to check in what we have and then do that or just keep iterating on this PR?

anpete · 2015-03-13T21:28:39Z

This is mostly a rewrite so a new PR.

mikary · 2015-03-16T16:29:42Z

Closing PR, will make a new one for the next revision.

ghost added the cla-not-required label Mar 2, 2015

anpete reviewed Mar 2, 2015
View reviewed changes

mikary force-pushed the dbSetRawQuery branch from ee8c9ea to 71dd657 Compare March 5, 2015 00:26

maumar reviewed Mar 5, 2015
View reviewed changes

mikary force-pushed the dbSetRawQuery branch 3 times, most recently from 0f4b83e to 71fde95 Compare March 9, 2015 21:57

divega reviewed Mar 10, 2015
View reviewed changes

maumar reviewed Mar 10, 2015
View reviewed changes

mikary force-pushed the dbSetRawQuery branch from 71fde95 to 3efb451 Compare March 10, 2015 20:02

anpete reviewed Mar 10, 2015
View reviewed changes

mikary force-pushed the dbSetRawQuery branch from 3efb451 to fd0421c Compare March 12, 2015 00:09

mikary added 3 commits March 13, 2015 10:12

Add FromSql extension method for raw SQL queries on DbSet<TEntity>

391a3e4

Further updates based on PR input

af958b4

FromSql returns cloned DbSet to allow additional annotation extensions

e9ca28e

mikary force-pushed the dbSetRawQuery branch from d8e8518 to e9ca28e Compare March 13, 2015 18:20

mikary closed this Mar 16, 2015

mikary deleted the dbSetRawQuery branch March 19, 2015 00:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RelationalDbSet for raw query #1734

Add RelationalDbSet for raw query #1734

mikary commented Mar 2, 2015

anpete Mar 2, 2015

anpete commented Mar 2, 2015

anpete Mar 2, 2015

mikary Mar 2, 2015

anpete Mar 2, 2015

mikary commented Mar 5, 2015

maumar Mar 5, 2015

mikary commented Mar 9, 2015

divega Mar 10, 2015

mikary Mar 10, 2015

anpete Mar 10, 2015

mikary Mar 10, 2015

anpete Mar 10, 2015

divega Mar 10, 2015

divega commented Mar 10, 2015

maumar Mar 10, 2015

mikary Mar 10, 2015

anpete Mar 10, 2015

mikary Mar 10, 2015

mikary commented Mar 10, 2015

anpete Mar 10, 2015

mikary Mar 10, 2015

anpete Mar 10, 2015

mikary commented Mar 12, 2015

mikary commented Mar 13, 2015

divega commented Mar 13, 2015

anpete commented Mar 13, 2015

mikary commented Mar 16, 2015

Add RelationalDbSet for raw query #1734

Add RelationalDbSet for raw query #1734

Conversation

mikary commented Mar 2, 2015

Choose a reason for hiding this comment

anpete commented Mar 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikary commented Mar 5, 2015

Choose a reason for hiding this comment

mikary commented Mar 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divega commented Mar 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikary commented Mar 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikary commented Mar 12, 2015

mikary commented Mar 13, 2015

divega commented Mar 13, 2015

anpete commented Mar 13, 2015

mikary commented Mar 16, 2015