Expression mapping changes parameterizable member access expressions to non-parameterizable constants

[csboling]

The problem

To fix #79, a condition was added that creates constant expressions for the values of local variables. However, this changes the node type from MemberAccess to Constant. Adding some assertions to one of the tests for this behavior can reveal this:

Expression<Func<GarageModel, bool>> filter = m => m.Color == garage.Color;

//Act
var mappedfilter = mapper.MapExpression<Expression<Func<Garage, bool>>>(filter);

//Assert
Assert.NotNull(mappedfilter);
var rhs = ((BinaryExpression)filter.Body).Right;
Assert.Equal(ExpressionType.MemberAccess, rhs.NodeType); // passes
var mappedrhs = ((BinaryExpression)mappedfilter.Body).Right;
Assert.Equal(ExpressionType.MemberAccess, mappedrhs.NodeType); // fails -- the node type is now Constant

The problem with turning these nodes into ConstantExpressions is that constant nodes are inlined into the expression tree, and query providers such as Entity Framework will treat values like this as e.g. part of the SQL query text, rather than lifting them to SQL parameters. This causes each new value of mappedrhs to be treated as a completely separate query and compiled separately by the query provider, even though the structure of the expression tree to evaluate is identical, and only the input value has changed. Since EF Core caches each compiled query, this causes memory usage to grow as different end-user-provided values are given as input for dynamic query configuration, and impacts performance because the query compiler has to repeatedly compile nearly-identical expressions.

Implementation thoughts

Ways I have seen to build expressions that preserve EF's ability to extract parameters from the expression tree are basically:

Expression<Func<T>> lift = () => constantValue;
var memberAccessExpression = lift.Body;

and

var memberAccessExpression = Expression.Property(
    Expression.Constant(new { constantValue }),
    nameof(constantValue));

Both of these require providing enough type information when constructing the memberAccessExpression to have the right Type rather than just an opaque object.

For my own purposes, just commenting out the whole if (baseExpression?.NodeType == ExpressionType.Constant) conditional works for the types of expressions I'm trying to map, base.VisitMember(node) is able to handle them. I don't know exactly the right way to preserve the node type in a way that preserves the desired behavior for local variables in e.g. #79. I'm willing to take a stab at a PR but I may need some guidance, my early attempts at changing this have not been successful at getting all the local variable related tests to pass.

[BlaiseD]

I think the problem solved in PR #80 was "reference type constant expressions not mapped", so just excluding literals from that logic should solve the parameterization problem. Maybe:

?