Skip to content

ValueOption equality on expression trees causes crash #13370

Closed
Closed
ValueOption equality on expression trees causes crash#13370
Labels
Area-QueriesQuery expressions and library implementationBugImpact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone
Backlog

Description

@Thorium
Thorium
opened on Jun 23, 2022

I'm trying to have an expression tree to compare a struct with the same item, and it fails?

open System
open System.Linq.Expressions
//let tester = Some "a" //works
//let tester = Guid.NewGuid() //works
let tester = ValueSome "b" //crash

let equalExpr = Expression.Equal(Expression.Constant(tester), Expression.Constant(tester))

//System.InvalidOperationException: The binary operator Equal is not defined for the types 
// 'Microsoft.FSharp.Core.FSharpValueOption`1[System.String]' and 
// 'Microsoft.FSharp.Core.FSharpValueOption`1[System.String]'.
//   at System.Linq.Expressions.Expression.GetEqualityComparisonOperator(ExpressionType binaryType, String opName, Expression left, Expression right, Boolean liftToNull)
//   at System.Linq.Expressions.Expression.Equal(Expression left, Expression right, Boolean liftToNull, MethodInfo method)

Activity

  1. Thorium
    added
    Bug
    on Jun 23, 2022

  2. Thorium commented on Jun 23, 2022

    @Thorium
    Thorium
    on Jun 23, 2022
    ContributorAuthor

    Ok,
    I found a workaround that is enough good for me: I lie the types to the expression tree:

    open System
open System.Linq.Expressions
//let tester = Some "a" //works
//let tester = Guid.NewGuid() //works
let tester = ValueSome "b"
let ty = typeof<obj>

let equalExpr = Expression.Equal(Expression.Constant(tester, ty), Expression.Constant(tester, ty))

    Edit: Not very good solution if your expressions are not Constant.
    An example of full stack is here:

    System.InvalidOperationException: The binary operator Equal is not defined for the types 'Microsoft.FSharp.Core.FSharpValueOption`1[System.String]' and 'Microsoft.FSharp.Core.FSharpValueOption`1[System.String]'.
   at System.Linq.Expressions.Expression.GetEqualityComparisonOperator(ExpressionType binaryType, String opName, Expression left, Expression right, Boolean liftToNull)
   at System.Linq.Expressions.Expression.Equal(Expression left, Expression right, Boolean liftToNull, MethodInfo method)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext@364.Invoke(Tuple`2 tupledArg)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 364
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 599
   at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList`1 cons, FSharpFunc`2 f, FSharpList`1 x) in D:\a\_work\1\s\src\fsharp\FSharp.Core\local.fs:line 237
   at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc`2 mapping, FSharpList`1 x) in D:\a\_work\1\s\src\fsharp\FSharp.Core\local.fs:line 248
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprsToLinq(ConvEnv env, FSharpList`1 es) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 689
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 507
   at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc`2 mapping, FSharpList`1 x) in D:\a\_work\1\s\src\fsharp\FSharp.Core\local.fs:line 246
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprsToLinq(ConvEnv env, FSharpList`1 es) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 689
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 507
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 599
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.EvaluateQuotation(FSharpExpr e) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Linq.fs:line 713
   at Microsoft.FSharp.Linq.QueryModule.EvalNonNestedInner(CanEliminate canElim, FSharpExpr queryProducingSequence) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Query.fs:line 1823
   at Microsoft.FSharp.Linq.QueryModule.clo@1926-18.Microsoft.FSharp.Linq.ForwardDeclarations.IQueryMethods.Execute[a,b](FSharpExpr`1 q) in D:\a\_work\1\s\src\fsharp\FSharp.Core\Query.fs:line 1928
  3. Thorium
    closed this as completedon Jun 23, 2022
  4. Thorium
    reopened this on Jun 24, 2022

  5. Thorium commented on Jun 24, 2022

    @Thorium
    Thorium
    on Jun 24, 2022
    ContributorAuthor

    More practical example:

    open System.Linq
let datasource1 = [1; 2].AsQueryable() // primary key
let datasource2 = [ValueSome 1; ValueSome 2].AsQueryable() //foreign key
query { 
   for d1 in datasource1 do
   for d2 in datasource2 do
   where (ValueSome(d1) = d2)
   select (d1, d2)
} |> Seq.toList;;

// System.InvalidOperationException: The binary operator Equal is not defined for the types 'Microsoft.FSharp.Core.FSharpValueOption`1[System.Int32]' and 'Microsoft.FSharp.Core.FSharpValueOption`1[System.Int32]
  6. vzarytovskii
    added this to F# Compiler and Toolingon Jul 7, 2022
  7. vzarytovskii
    moved this to Not Planned in F# Compiler and Toolingon Jul 7, 2022
  8. vzarytovskii
    added this to the Backlog milestone on Jul 7, 2022
  9. dsyme
    added
    Area-QueriesQuery expressions and library implementation
    Impact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.
    on Jul 8, 2022

  10. Thorium commented on Jul 12, 2022

    @Thorium
    Thorium
    on Jul 12, 2022
    ContributorAuthor

    The original issue can be reproduced with custom types:

    open System
open System.Linq.Expressions

[<StructuralEquality; StructuralComparison>]
[<Struct>]
type Person(name:string, age:int) =
  member x.Name = name
  member x.Age = age

let a = Person("Joe", 42)
let a2 = Person("Joe", 42)
let equalExpr = Expression.Equal(Expression.Constant(a), Expression.Constant(a2))
// System.InvalidOperationException

    In the case of LINQ it's often maintainability over performance, so I'd prefer some extra-boxing or whatever just to make things work correctly.

    Edit: I move the another issue to separate issue

  11. Happypig375 commented on Jul 12, 2022

    @Happypig375
    Happypig375
    on Jul 12, 2022
    Member

    For this you should provide the F# equality operator as a custom operator into the Expression.Equals constructor. F#'s equality is different from C#'s equality and System.Linq.Expressions uses C# equality by default.

  12. Happypig375 commented on Jul 12, 2022

    @Happypig375
    Happypig375
    on Jul 12, 2022
    Member
    let makeEq (left: System.Linq.Expressions.Expression) (right: System.Linq.Expressions.Expression) =
    System.Linq.Expressions.Expression.Equal(left, right, false, typeof<_ option>.Assembly.GetType("Microsoft.FSharp.Core.Operators").GetMethod("op_Equality").MakeGenericMethod left.Type)
let a = ValueSome 1
printfn $"🌄{System.Linq.Expressions.Expression.Lambda(makeEq (System.Linq.Expressions.Expression.Constant a) (System.Linq.Expressions.Expression.Constant a), [||]).Compile().DynamicInvoke[||]}"

  13. Happypig375 commented on Jul 12, 2022

    @Happypig375
    Happypig375
    on Jul 12, 2022
    Member

    This issue can be closed as by design

  14. Thorium commented on Jul 12, 2022

    @Thorium
    Thorium
    on Jul 12, 2022
    ContributorAuthor

    @Happypig375 will the Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter be using this custom operator constructor when you do F# Linq query?

  15. Happypig375 commented on Jul 12, 2022

    @Happypig375
    Happypig375
    on Jul 12, 2022
    Member

    @Thorium Only after merging #11681. Currently, no.

  16. T-Gro commented on Dec 29, 2022

    @T-Gro
    T-Gro
    on Dec 29, 2022
    Member

    #11681 is merged, this can be closed (by design, solution exists)

  17. T-Gro
    closed this as completedon Dec 29, 2022
  18. github-project-automation
    moved this from Not Planned to Done in F# Compiler and Toolingon Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-QueriesQuery expressions and library implementationBugImpact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.

    Type

    No type

    Projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions