Add support for SQL Server 2017 graph processing

[WiredUK]

This PR relates to #8527. I wanted to submit this PR as soon as possible as it's quite large (for me at least!) and wanted to make sure I was on the right track.

It could possible do with some more tests but I'd like your guidance before continuing.

[smitpatel]

Changing base branch to master

[ajcvickers]

@WiredUK Thanks for submitting this PR. We're definitely interested in pursuing this, but right now we are focused on other areas, such as locking down for the 2.2 release. Therefore, it might take us a while to get to a full review.

[WiredUK]

@ajcvickers No problem at all, I assumed this was too late for 2.2 anyway and if it does get merged that it would be 3 at the earliest, particularly as there's a couple of minor breaking API changes. Thanks for letting me know though, and I'll be here when you're ready!

[jvveldhuizen]

What's an expected timeline? Currently we are developing a new application and we want to use the new SQL graph features. To start simple we need a table as node and a relation as edge. We don't need full support for advanced queries right now, but what's the best way to be prepared? We prefer to start with the node and edge creation already and implement advanced queries later. Is it possible to start already? Or is it simply not supported at all yet?

We are willing to start as beta customer and are available to discuss our needs!

Beforehand thanks for your response,

Johan

[WiredUK]

What's an expected timeline?

@jvveldhuizen I wouldn't hold your breath for this one any time soon. Core 3 is likely to take a significant amount of time. In the meantime though, you could pull this PR down and see how it works for you. I'd certainly appreciate someone giving it a try and testing out my changes.

[jdeprato]

@WiredUK We're working on some new development with SQL Server 2019 and the graph features. Our team is going to pull down the code and take a look. Maybe we can start working on this together on this as we have a direct need for this solution for our project.

[WiredUK]

@WiredUK We're working on some new development with SQL Server 2019 and the graph features. Our team is going to pull down the code and take a look. Maybe we can start working on this together on this as we have a direct need for this solution for our project.

@jdeprato I'd be happy to work with you and your team on this. I'd be very interesting in how you get on with it either way.

[jdeprato]

@WiredUK It looks like your changes were mostly focused on code first database support. We're actually using database first design. Have you looked at the Scaffold-DbContext functionality? We're running into an issue there where the method that pulls the indexes from tables fails because it returns a null for column. We haven't dug deep enough yet, but we're suspecting it's because of the $node_id etc syntax that's being used now to define those corresponding columns in the graph tables.

[WiredUK]

@WiredUK It looks like your changes were mostly focused on code first database support. We're actually using database first design. Have you looked at the Scaffold-DbContext functionality? We're running into an issue there where the method that pulls the indexes from tables fails because it returns a null for column. We haven't dug deep enough yet, but we're suspecting it's because of the $node_id etc syntax that's being used now to define those corresponding columns in the graph tables.

@jdeprato Yeah, I was initially interested in getting the querying side working. I did spend a little time looking at the scaffolding component, but it was a little awkward and wanted to push up this PR to make sure I was on the right track before I went too far down the rabbit hole.

I can possibly still do some work on that though, I've got some time over the holidays. Is there anything in particular that you would like me to look at?

[jdeprato]

@WiredUK Our biggest blocker right now is trying to figure out how to step through the code that is called by Scaffold-DbContext. It's not very clear how you set it up for debugging it. Once we get that working, I think we can make progress on changes we'll need in order to generate the appropriate classes etc from the existing database. Then we need to look at adding support for edge constraints. That will be a huge component in making this work correctly and successfully.

[WiredUK]

@WiredUK Our biggest blocker right now is trying to figure out how to step through the code that is called by Scaffold-DbContext. It's not very clear how you set it up for debugging it. Once we get that working, I think we can make progress on changes we'll need in order to generate the appropriate classes etc from the existing database. Then we need to look at adding support for edge constraints. That will be a huge component in making this work correctly and successfully.

@jdeprato I'm curious how you see edge constraints being surfaced in EF. It's not something I've looked into with SQL2019 only being in a CTP release right now (I don't even think SSMS support it properly in preview)

Regarding scaffolding though, there is some support in there already. Do you have an example DB that you could supply that I could test against? My (admittedly simple) tests here seems to work well enough. What command line do you run to scaffold? I'm testing directly with the dotnet ef dbcontext scaffold ... command.

[jdeprato]

@WiredUK Regarding edge constraints, look at this example regarding foreign keys: https://docs.microsoft.com/en-us/ef/core/modeling/relationships, without the constraint it would be similar to a foreign key, you wouldn't know that this relationship actually exists without actually writing the query itself or manually defining that relationship in code for the example: Node-(edge)->Node. As far as the scaffolding goes we were getting an error when trying to generate the model from a defined graph schema that we created. I can't share our specific schema, but I'll try and come up with something we can share. Where should we post something like that so you can see it? Also we're working with Microsoft's SQL product team with regards to the graph features. Here is the system view that should help derive the relationships for mapping the model for ef core purposes: https://docs.microsoft.com/en-us/sql/relational-databases/system-catalog-views/sys-edge-constraints-transact-sql?view=sql-server-ver15

Here is a more refined query for better understanding of how you can use the system view:

SELECT OBJECT_NAME(object_id) as ConstraintName,
OBJECT_NAME(from_object_id) as NodeFrom,
OBJECT_NAME(to_object_id) as NodeTo,
clause_number
FROM sys.edge_constraint_clauses

[WiredUK]

@WiredUK Regarding edge constraints, look at this example regarding foreign keys: https://docs.microsoft.com/en-us/ef/core/modeling/relationships, without the constraint it would be similar to a foreign key, you wouldn't know that this relationship actually exists without actually writing the query itself or manually defining that relationship in code for the example: Node-(edge)->Node. As far as the scaffolding goes we were getting an error when trying to generate the model from a defined graph schema that we created. I can't share our specific schema, but I'll try and come up with something we can share. Where should we post something like that so you can see it? Also we're working with Microsoft's SQL product team with regards to the graph features, so I would say the constraint feature is pretty well set it in stone as far as getting in the final release. Here is the system view that should help derive the relationships for mapping the model for ef core purposes: https://docs.microsoft.com/en-us/sql/relational-databases/system-catalog-views/sys-edge-constraints-transact-sql?view=sql-server-ver15

@jdeprato Sorry for the confusion, I know what edge constraints are going to be, I was asking how you think they might show up in EF when scaffolding. EF doesn't check constraints or indexes, it relies on the backing store to enforce those rules. So the only real benefit I can see would be that edge constraints would be created in the DB when running a migration in a new database from the scaffolded context. Does that make sense?

An example schema is all I need to play around with, what people on Stack Overflow call a minimum, complete, verifiable example :) To get it to me, you can email me directly (address in my profile) or use a site like pastebin.com.

[jdeprato]

Here is a simple example:

IF OBJECT_ID('dbo.Edge1') IS NOT NULL
BEGIN
	DROP TABLE dbo.Edge1
END
IF OBJECT_ID('dbo.Node1') IS NOT NULL
BEGIN
	DROP TABLE dbo.Node1
END
IF OBJECT_ID('dbo.Node2') IS NOT NULL
BEGIN
	DROP TABLE dbo.Node2
END
IF OBJECT_ID('dbo.Node3') IS NOT NULL
BEGIN
	DROP TABLE dbo.Node3
END
GO
CREATE TABLE dbo.Node1
(
	Id INT IDENTITY(1,1) PRIMARY KEY,
	Name VARCHAR(50),
    INDEX ix_graphid UNIQUE ($node_id)
)
AS NODE
GO
CREATE TABLE dbo.Node2
(
	Id INT IDENTITY(1,1) PRIMARY KEY,
	Name VARCHAR(50),
    INDEX ix_graphid UNIQUE ($node_id)
)
AS NODE
GO
CREATE TABLE dbo.Node3
(
	Id INT IDENTITY(1,1) PRIMARY KEY,
	Name VARCHAR(50),
    INDEX ix_graphid UNIQUE ($node_id)
)
AS NODE
GO
CREATE TABLE dbo.Edge1
(
    Id INT IDENTITY(1,1) PRIMARY KEY,
    Name VARCHAR(50),
    CONSTRAINT EC_EDGE1 CONNECTION (Node1 TO Node2, Node2 TO Node3),
    INDEX ix_graphid UNIQUE ($edge_id),
    INDEX ix_fromid ($from_id, $to_id),
    INDEX ix_toid ($to_id, $from_id)
)
AS EDGE
GO

[jdeprato]

@WiredUK Still having issues. I'm going to step away for a bit and try again later.

[WiredUK]

@jdeprato Hehe no problem. I'm happy to take a look at your test project if you like. Otherwise let me know if you want me to provide a complete repo for you to test against. At least we might be able to compare test projects.

[jdeprato]

@WiredUK Thanks for all of your help. I finally got it working. Do you happen to know why it's requiring the use of .NET Core 2.2 preview 3? I didn't see where I could change the targeting platform. I would prefer to use a fully released target framework like 2.2.101 if you happen to know where it's referencing the preview one I can change it and see if that causes any negative impacts.

[WiredUK]

@jdeprato Er no, that makes no sense to me. The framework version shouldn't have any effect at all. I'm using 2.2 here and it's fine.

[jdeprato]

@WiredUK I'm not sure why but I couldn't get the ef scaffold command to work without installing the preview version of .net core 2.2.101. I had install 2.2.100 for some reason.

[WiredUK]

@jdeprato 2.2.100 isn't a preview, it was the initial release of 2.2.

[jdeprato]

@WiredUK Okay that's weird. I couldn't get it to run until I installed preview 3. When I had only 2.2.101 installed it wouldn't run for some reason. I can post the solution if you want to look at it.

[jdeprato]

@WiredUK Here is the error message I received and that's why I installed the preview version and that allowed the scaffolding command to work finally. The first time running the command returns the error and the second time running the command completed successfully after installing the preview version:

C:\..\..\..\..\EFCore\src\TestConsoleApp>dotnet ef dbcontext scaffold "<my connection string>" Microsoft.EntityFrameworkCore.SqlServer -o ScaffoldedContext
It was not possible to find any compatible framework version
The specified framework 'Microsoft.NETCore.App', version '2.2.0-preview3-27001-02' was not found.
  - Check application dependencies and target a framework version installed at:
      C:\Program Files\dotnet\
  - Installing .NET Core prerequisites might help resolve this problem:
      http://go.microsoft.com/fwlink/?LinkID=798306&clcid=0x409
  - The .NET Core framework and SDK can be installed from:
      https://aka.ms/dotnet-download
  - The following versions are installed:
      1.0.5 at [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      1.1.2 at [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      2.0.0 at [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      2.1.6 at [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      2.2.0 at [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

C:\..\..\..\..\EFCore\src\TestConsoleApp>dotnet ef dbcontext scaffold "<my connection string>" Microsoft.EntityFrameworkCore.SqlServer -o ScaffoldedContext

I did search through all of the projects and I didn't find any references to the preview version so I don't really understand why it was complaining about not having it. To force a specific version as for as I know you have set the global.json file and override the live version with the forced preview version. You can see in the error I had 2.2.0 installed, but in fact I had 2.2.101 installed (I'm going to guess it just generically refers to it instead of referring to the minor version). I've uninstalled the preview version and then received the same error message you see above. The reason I made the assumption that .NET Core 2.2.100 was a preview is based on what I saw in the in "programs and features" on my machine. I am posting a screen shot so you can see what's installed and when it was installed. You can see I had 2.2.101 installed before installing 2.2.100 preview 3 (which shows todays date now because I uninstalled and reinstalled it to test to make sure I could reproduce the error) and like I said I couldn't get it working without the preview version.

[WiredUK]

@jdeprato I'd be interested in what the output of dotnet --info was when run from inside the EFCore source directory, especially if you only have the RTM version of the SDK installed and no preview. Another thing I would check is to run a clean solution and clear the Nuget package cache too.

[WiredUK]

@jdeprato I've also just noticed that there is a file called version.props in the root that seems to point at the preview version of the SDK. You may want to try editing that file to get it to compile. Note that when I was working on this PR, 2.2 wasn't released yet.

[smitpatel]

These changes introduces QuerySourceReferenceExpression which is relinq specific in our SQL tree. We don't have QSRE as part of our SQL tree since there is no meaning of printing it. Also the way Function printing and delimiting is happening is also pretty ad-hoc. I believe these changes should not be added until new query pipeline due to significant changes would happen in the area.
Plus, there are no tests for Match function.

[WiredUK]

@smitpatel Thanks for reviewing this, I have a couple of question/comments.

1. Not sure what you mean by "introduces QuerySourceReferenceExpression", all I did was move that code into a virtual method so it can be overidden in the derived class. This is needed because the Match method takes entities as parameters, not entity properties. I couldn't see a better way to achieve this?
2. Delimiting may look ad-hoc, but it's a requirement as (for some reason) the SQL Server dev team decided that the new psuedo columns (i.e. $edge_id and $from_id) won't work if you surround them with [...]. So SELECT [$edge_id] FROM EdgeTable will tell you Invalid column name '$edge_id' but SELECT $edge_id FROM EdgeTable works fine.
3. Completely agree it needs more tests, I just wanted some guidance before I carried on.

I assumed that since v3 is likely to bring significant change that this entire PR would get closed. I'm happy to look at re-doing it when the internals have been changed if you think that would be worthwhile?

[smitpatel]

Answers as follows:

1. QuerySource in QueryModel gets converted to TableExpression in SQL tree. While it is true that Match takes entities but they are integrated in SQL tree as TableExpression rather than QSRE. (That's what translation does)
2. If the new expression is "psuedo-column" which requires different processing than regular column then it is better to have a new kind of Expression rather than trying to fit on existing one which requires changes to default delimitation process. Especially when all the changes are for SqlServer only rather than across different providers. A good example of that would RowNumberExpression.
3. No worries.

I was just evaluating the PR for team to determine are we any close to get this merged. But it is better to re-visit after internals have changed.

[jvveldhuizen]

A simple but useful post is this: https://www.red-gate.com/simple-talk/sql/sql-development/sql-server-graph-databases-part-1-introduction/ As far as I can see it’s an internal variable. So you should not add quotes around it. A sample insert statement from that blog: INSERT INTO Posts ($from_id, $to_id) VALUES ( (SELECT $node_id FROM FishLover WHERE FishLoverID = 1), (SELECT $node_id FROM FishPost WHERE PostID = 3)); Hope that helps you to continue! From: espray <notifications@github.com> Sent: woensdag 13 februari 2019 23:37 To: aspnet/EntityFrameworkCore <EntityFrameworkCore@noreply.github.com> Cc: jvveldhuizen <johan@van-veldhuizen.eu>; Mention <mention@noreply.github.com> Subject: Re: [aspnet/EntityFrameworkCore] Add support for SQL Server 2017 graph processing (#13804) @WiredUK <https://github.com/WiredUK> I working on a variation of this PR and when I try to load a Node I get this exception "Invalid column name '$node_id'.". You can see from the screen snip, when SQL created the Node table it added a guid to the end of the $node_id column. Any ideas how to avoid the guid postfix on the $node_id column? <https://user-images.githubusercontent.com/5826764/52748537-5b0d7600-2fac-11e9-804c-6bb24c07ef05.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13804 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AJQQlG5xKV01T5Tv05fZJnblFA3xNRptks5vNJONgaJpZM4YA82P> .

[espray]

@jvveldhuizen looks like that is is. Here is the SQL statement EF generated

SELECT TOP(1) [c].[ProviderId], [c].[ProviderKey], [c].[Name], [c].[$node_id]
FROM [Customers] AS [c]

@WiredUK what part of the PR resolved this? Is it the includeDelimiters in SqlServerSqlGenerationHelper.cs? I am just looking to support a SELECT

[WiredUK]

@jvveldhuizen looks like that is is. Here is the SQL statement EF generated

SELECT TOP(1) [c].[ProviderId], [c].[ProviderKey], [c].[Name], [c].[$node_id]
FROM [Customers] AS [c]

@WiredUK what part of the PR resolved this? I am just looking to support a SELECT

It shouldn't be generating SQL like that. What did your C# code look like to create that? The psuedo columns aren't supposed to be escaped like this.

To be honest, I need to take another look at this to make the changes suggested previously by Smit, specifically to add a new column expression object for these psuedo columns.

[espray]

@WiredUK Its my own implementation derived from this PR.

It looks like its the includeDelimiters in SqlServerSqlGenerationHelper.cs, is that correct?

[WiredUK]

@WiredUK Its my own implementation derived from this PR.

It looks like its the includeDelimiters in SqlServerSqlGenerationHelper.cs, is that correct?

Off the top of my head because I'm not sat in front a my dev PC, yes that looks right, and why I need to replace the whole delimiter parameter with a new column expression.

[espray]

Yes that was the correct spot.

FYI: I created an override for SqlServerQuerySqlGenerator.VisitColumn() and used the below code

    Sql.Append(SqlGenerator.DelimitIdentifier(columnExpression.Table.Alias))
        .Append(".");

    if (includeDelimiters)
    {
        Sql.Append(SqlGenerator.DelimitIdentifier(columnExpression.Name));
    }
    else
    {
        Sql.Append(columnExpression.Name);
    }

FYI: for my implementation I am using DbContextOptionsBuilder.ReplaceService(), rather than modifying the SqlProvider it self.

[mikethibault]

This is great work! Are there any plans to support graph processing in EF Core 3?

[jdeprato]

This is great work! Are there any plans to support graph processing in EF Core 3?

We've been in contact with Microsoft about it, but they didn't have a timeline for when they were going to get to it. They're making a lot of changes with 3.0, so it's going to be a while I think.

[mikethibault]

I'm trying to play around with this myself. How does one ensure that the EF Core Tool commands you'd like to run (Update-Database, Scaffold-DbContext, etc) call the pull request code instead of the standard EF Core Tools that lack Graph Processing support?

[bvdwalt]

I'm trying to play around with this myself. How does one ensure that the EF Core Tool commands you'd like to run (Update-Database, Scaffold-DbContext, etc) call the pull request code instead of the standard EF Core Tools that lack Graph Processing support?

From what I've read, it seems that you need to clone the repository, build the code, retrieve the .nupkg file from the build output and upload that to a local nuget feed and then use that package in your project.
I have however been unsuccessful in getting the .nupkg file and uploading to my local nuget feed thus far.

Edit:
I have since been able to build the code, obtain the nupkg files I needed, upload them to my local Nuget feed and include them in my test project. I'll see how far I get

[mikethibault]

I found my migrations would not be correctly generated unless I modified ColumnList in EFCore.Relational/Migrations/MigrationsSqlGenerator.cs like this:

protected virtual string ColumnList([NotNull] string[] columns)
{

List<string> pseudoColumnNames = new List<string> { "$edge_id", "$node_id", "$from_id", "$to_id" };

return string.Join(", ", columns.Select(c => Dependencies.SqlGenerationHelper.DelimitIdentifier(c, !pseudoColumnNames.Contains(c))));

}

The pseudo-column references were still being delimited with square brackets until I modified this; I'm thinking there may be a better way of doing this, somewhere where the pseudo-column attribute is available, but it works.

[mikethibault]

I have since been able to build the code, obtain the nupkg files I needed, upload them to my local Nuget feed and include them in my test project. I'll see how far I get

@bvdwalt any luck in getting it to work? I am successfully referencing the custom EF project in my migrations, but in my application it is always defaulting to standard EF assemblies.

[bvdwalt]

I have since been able to build the code, obtain the nupkg files I needed, upload them to my local Nuget feed and include them in my test project. I'll see how far I get

@bvdwalt any luck in getting it to work? I am successfully referencing the custom EF project in my migrations, but in my application it is always defaulting to standard EF assemblies.

@mikethibault I have somewhat left this alone for the time being. Will possibly get back to it sometime soon.

[smitpatel]

@WiredUK & others who are watching and were working on this PR in past. Thank you for all your efforts. We apologize for not be able to process this in time. Other higher priority features & architectural changes for 3.0 release kept us heavily occupied.
There has been quite a lot of changes in underlying codebase which requires changes in this PR. Since there is no active person working on this PR right now, I will close this for now. It does not mean that we don't want contribution. You are still encouraged to use this PR as starting point (or start afresh whatever you prefer) against latest master branch. It would also be good to discuss design beforehand with the team in the tracking issue.