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

LINQ Provider Rewrite #2518

Draft
wants to merge 60 commits into
base: main
Choose a base branch
from
Draft

LINQ Provider Rewrite #2518

wants to merge 60 commits into from

Conversation

joakim-olsson
Copy link
Collaborator

@joakim-olsson joakim-olsson commented Jul 12, 2021

Description

I'm experimenting with ways to rewrite the way the LINQ provider works to a more object-oriented structure. This is just a rough sketch so this is not planned to be merged and might not even be close to the end product.

The idea is to have it as objects so that it can be easily transferred to a JSON structure if that is the case. E.g a query like:
var query = _realm.All<Person>().Where(p => p.Score == 5 && p.FirstName == "Test" && p.LastName == "Testersson");

should give the ability to create a JSON structure like:

{
    "WhereClause":
    [
        {
            "property": "Score",
            "value": 5,
            "op": "eq"
        },
        {
            "property": "FirstName",
            "value": "Test",
            "op": "eq"
        },
        {
            "property": "LastName",
            "value": "Testersson",
            "op": "eq"
            
        }
    ]
}

Realm/Realm/Linq/QueryObjects.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryObjects.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryObjects.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor.cs Outdated Show resolved Hide resolved
Tests/Tests.XamarinMac/Tests.XamarinMac.csproj Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@papafe papafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments!

Realm/Realm/Linq/QueryObjects.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/SortClause.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryModel.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryModel.cs Show resolved Hide resolved
Realm/Realm/Linq/QueryModel.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryModel.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/QueryModel.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
{
Visit(node.Arguments[0]);
var whereClause = new WhereClauseVisitor(_metadata);
var lambda = (LambdaExpression)StripQuotes(node.Arguments[1]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at this comment


namespace Realms
{
internal class SortClauseVisitor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make a base class, something called "ClauseVisitor", that also has the strip method

Realm/Realm/Linq/RealmResultsVisitor2.cs Outdated Show resolved Hide resolved
Realm/Realm/Linq/WhereClauseVisitor.cs Outdated Show resolved Hide resolved
throw new NotSupportedException($"The operator '{binaryExpression.NodeType}' is not supported");
}

if (binaryExpression.Left is MemberExpression binaryLeftMemberExpression)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a lot of repeated code here, try to avoid this. There is no difference between left and right here

{
if (node.Expression != null && node.Expression.NodeType == ExpressionType.Parameter)
{
ComparisonNode comparisonNode = new EqualityNode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boolean statements with no equality sign (e.g .Where(p => !p.IsInteresting))


private static string GetKind(object valueType)
{
if (valueType == typeof(float))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this with a switch statement? Also, does valueType need to be object? Can we be more specific with the type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some research on this and it does not seem like there is a good way to do this, we can discuss this later

Tests/Realm.Tests/Database/AAAATests.cs Outdated Show resolved Hide resolved
Tests/Realm.Tests/Database/AAAATests.cs Outdated Show resolved Hide resolved
Tests/Tests.XamarinMac/Tests.XamarinMac.csproj Outdated Show resolved Hide resolved
@papafe papafe changed the title Jo/rewrite LINQ Provider Rewrite Aug 23, 2021
@papafe
Copy link
Contributor

papafe commented Sep 2, 2021

This is still a draft, but I've cleaned it out a little, so it should be readable enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants