graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Error when filtering or sorting on a field of a related object, where the field isn't a simple property access.

Open darren-clark opened this issue 1 year ago • 1 comments

Product

Hot Chocolate

Version

13.8.1

Link to minimal reproduction

https://github.com/darren-clark/graphql-platform/blob/sort_filter_fix/src/HotChocolate/Data/test/Data.Filters.SqlServer.Tests/QueryableFilterVisitorObjectTests.cs#L671

Steps to reproduce

Using EF, add a sort or filter field to an object that flattens a field of a related object.

e.g.

    public class BarFilterInput : FilterInputType<Bar>
    {
        protected override void Configure(IFilterInputTypeDescriptor<Bar> descriptor)
        {
            descriptor.Field(t => t.Foo.BarString)
                .Name("fooBarString");
        }
    }

Then on a type that has an object of this type as a field, use this input type:

    public class Baz
    {
        public int Id { get; set; }
        public Bar Bar { get; set; } = default!;
    }

    public class BazFilterInput : FilterInputType<Baz>
    {
        protected override void Configure(IFilterInputTypeDescriptor<Baz> descriptor)
        {
            descriptor.Field(b => b.Bar)
                .Type<BarFilterInput>();
        }
    }

Query Baz with a filter on bar.fooBarString such as

{ baz (where: { bar: { fooBarString: { eq: "testbtest"}} }) 
{bar { foo{ barString}}}}

What is expected?

SQL generated that filters on Foo.BarString

What is actually happening?

Exception thrown that in VisitLambda the replaced expression must be the same type as the original expression.

Relevant log output

No response

Additional context

The issue is that it is doing a replace on the whole LambdaExpression, including parameters, then extracting the replaced Body. This means the replacing expression must be a ParameterExpression, but in this case it is a MemberExpression.

Replacing the original ParameterExpression in the body works fine, and is correct. However replacing it the LamdaExpression.Parameters fails, and is unnecessary since the first thing done with this rewritten expression is to extract the body and discard parameters.

darren-clark avatar Jun 18 '24 04:06 darren-clark

I have a fix for this, but not sure how to PR it. I branched from the version that I'm using, which is 13.8.1.

I verified that this bug exists in 13.9 and 14.0, and the fix is the same.

When I tried to rebase onto main I started getting compiler errors, and if I try to PR my branch into main it drags along a bunch of 13.8 commits.

The diff is here: https://github.com/ChilliCream/graphql-platform/compare/13.8.1...darren-clark:sort_filter_fix?expand=1

Would be nice if this were backported to 13.8 and 13.9. If it's not going to be, please let me know so I can fixup project by replacing the default field handlers with copies with this fix.

darren-clark avatar Jun 18 '24 05:06 darren-clark