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

Null propagation cannot be used for primitive type lists (string) #366

Closed
frankiDotNet opened this issue Apr 6, 2020 · 14 comments
Closed
Assignees
Labels

Comments

@frankiDotNet
Copy link

frankiDotNet commented Apr 6, 2020

I want to parse a null propagation expression for a list with a string:

Simple test scenario:

public class TestClass{
    public List<string> MyList {get; set;}
}
var dataSource = new TestClass();
// Let MyList null...

var expressionText = "np(myList.FirstOrDefault())"
LambdaExpression e = DynamicExpressionParser.ParseLambda(ParsingConfig.Default, dataSource.GetType(),
               typeof(object), expressionText, null);
            var result = (e.Compile()).DynamicInvoke(dataSource);

Here I get the exception : The 'np' (null-propagation) function requires the first argument to be a MemberExpression

Null propagation for a property works:

var expressionText = "np(myList.Count)"
LambdaExpression e = DynamicExpressionParser.ParseLambda(ParsingConfig.Default, dataSource.GetType(),
               typeof(object), expressionText, null);
            var result = (e.Compile()).DynamicInvoke(dataSource);
@StefH
Copy link
Collaborator

StefH commented Apr 8, 2020

Can you try MyGet version System.Linq.Dynamic.Core.1.0.23-ci-13135 ?

@frankiDotNet
Copy link
Author

frankiDotNet commented Apr 8, 2020

Looks good:

          var dataSource = new TestClass();
          
         var expressionText = "np(MyList.FirstOrDefault())";
         LambdaExpression e = DynamicExpressionParser.ParseLambda(ParsingConfig.Default, true, typeof(TestClass),
            typeof(object), expressionText, dataSource);
         var result = (e.Compile()).DynamicInvoke(dataSource);

{Param_0 => Convert(IIF(((Param_0 != null) AndAlso (Param_0.MyList != null)), Param_0.MyList .FirstOrDefault(), null))}

@StefH
Copy link
Collaborator

StefH commented Apr 8, 2020

#368

@StefH StefH self-assigned this Apr 8, 2020
@StefH StefH added bug feature and removed bug labels Apr 8, 2020
@StefH
Copy link
Collaborator

StefH commented Apr 8, 2020

@Franki1986 Can you do some more tests please? To make sure it works also on real database calls?

@frankiDotNet
Copy link
Author

Sure, I will do it tomorow. But I am using Linqtodb not ef core. But if the expression is right I think it will also work with ef..

@frankiDotNet
Copy link
Author

I am not sure what kind of test scenario I could do against a database? If I understand, I have to generate a methodExpression that could return null.

@StefH
Copy link
Collaborator

StefH commented Apr 9, 2020

Instead of using DynamicExpressionParser manually, you can just use a Linq Select:

Example:

var resultDynamic = DBContext.MyClasses.AsQueryable().Select("np(MyList.FirstOrDefault())");

@frankiDotNet
Copy link
Author

frankiDotNet commented Apr 9, 2020

I hope this is getting in the right direction, because it is the closest I can create with the database and LinqToDb, because a string list cannot be mapped directly to a database table and the expression then coould only be applied to the resulting data. So I created a parent and child class for 1 to n.


public class Parent{
    public int Id {get; set;}
    public List<Child>ChildList {get; set;}
}

public class Child{
    public int Id {get; set;}
}
       using (var db = new DbContext())
       {
            var query = db.GetTable<Parent>().LoadWith(g => g.ChildList);
            var resultA = query .Select(c => c.ChildList.FirstOrDefault() != null).ToList();

            var query2 = query .Select("r => np(r.ChildList.FirstOrDefault()) != null", null);
            var result2 = query2 .ToDynamicList();
       }

Resulting expression debug string:

"{Table(PARENT).LoadWith(g => g.ChildList).Select(Param_0 => (IIF(((Param_0 != null) AndAlso (Param_0.ChildList!= null)), Param_0.ChildList.FirstOrDefault(), null) != null))}";

The result list is the same..

@StefH
Copy link
Collaborator

StefH commented Apr 9, 2020

And if you get the Id and provide a default value, like:

var query2 = query .Select("np(r.ChildList.FirstOrDefault().Id, 42)");

@frankiDotNet
Copy link
Author

Sorry for the late response, I wasn't at the pc over the easter days.
So this

var query2 = qu.Select("np(ChildList.FirstOrDefault().Id, 42)", null);

produces the expression:

{Table(PARENT).LoadWith(g => g.ChildList).Select(Param_0 => IIF((((Param_0 != null) AndAlso (Param_0.ChildList != null)) AndAlso (Param_0.ChildList.FirstOrDefault() != null)), Param_0.ChildList.FirstOrDefault().Id, 42))}

which returns 42 for Ids that would return null.

Works!
Will there be a direct nuget release or do you put this change in a future release?

@StefH
Copy link
Collaborator

StefH commented Apr 15, 2020

@Franki1986 Thanks a lot for this test.
One last request : can you capture the real SQL which was produced?

@frankiDotNet
Copy link
Author

[Table("PARENT")]
public class Parent{
    [Column("ID")]
    public int Id {get; set;}
    [Association(ThisKey="ParentId", OtherKey="Id", CanBeNull=true, Relationship=Relationship.OneToMany, IsBackReference=true)]
    public List<Child>ChildList {get; set;}
}

[Table("CHILD")]
public class Child{
    [Column("ID")]
    public int Id {get; set;}
    [Column("PARENT_ID")]
    public int ParentId {get; set;}
}

LinqToDb didn't have eager loading till version 3 (pre release), so this is the SQL that is produced for a version under 3, where for each child a subselect is done:

SELECT
	t1.ID
FROM
	PARENT t1

DECLARE @p1 Integer -- Int32
SET     @p1 = 24

SELECT FIRST 1
        r.ID,
		r.PARENT_ID
FROM
        CHILD r
WHERE
        r.PARENT_ID = @p1

DECLARE @p1 Integer -- Int32
SET     @p1 = 25

SELECT FIRST 1
        r.ID,
		r.PARENT_ID
FROM
        CHILD r
WHERE
        r.PARENT_ID = @p1

and so on...

@frankiDotNet
Copy link
Author

Is this sufficent for you?

@StefH
Copy link
Collaborator

StefH commented Apr 16, 2020

Yes. I will merge PR and release new version.

@StefH StefH closed this as completed Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants