changes to enumeration representation; improve comparison compatibili…

…ty checks in condition expressions

src/FSharp.DynamoDB/Expression/ConditionalExpr.fs

@@ -43,6 +43,11 @@ and Comparator =
     | GT
     | LE
     | GE
+with
+    member cmp.IsComparison =
+        match cmp with
+        | EQ | NE -> false
+        | _ -> true
 
 and Operand =
     | Undefined
@@ -215,12 +220,18 @@ let extractQueryExpr (recordInfo : RecordInfo) (expr : Expr) : ConditionalExpres
             match expr with
             | SpecificCall pat (None, _, args) ->
                 let pickler = args |> List.tryPick (|AttributeGet|_|) |> Option.map (fun attr -> attr.Pickler)
-                args |> List.map (extractOperand pickler) |> Some
+                let operands = args |> List.map (extractOperand pickler)
+                let pickler = match pickler with Some p -> p | None -> Pickler.resolveUntyped args.[0].Type
+                Some(pickler, operands)
 
             | _ -> None
 
-        let extractComparison (cmp : Comparator) (left : Operand) (right : Operand) =
-            if left = right then
+        let extractComparison (p : Pickler) (cmp : Comparator) (left : Operand) (right : Operand) =
+            if cmp.IsComparison && not p.IsComparable then
+                sprintf "Representation of type '%O' does not support comparisons." p.Type
+                |> invalidArg "expr"
+
+            elif left = right then
                 match cmp with
                 | LE | EQ | GE -> True
                 | LT | NE | GT -> False
@@ -257,12 +268,12 @@ let extractQueryExpr (recordInfo : RecordInfo) (expr : Expr) : ConditionalExpres
 
             | AttributeGet attr -> Compare(EQ, Attribute attr.Id, Value (wrap (AttributeValue(BOOL = true))))
 
-            | Comparison <@ (=) @> [left; right] -> extractComparison EQ left right
-            | Comparison <@ (<>) @> [left; right] -> extractComparison NE left right
-            | Comparison <@ (<) @> [left; right] -> extractComparison LT left right
-            | Comparison <@ (>) @> [left; right] -> extractComparison GT left right
-            | Comparison <@ (<=) @> [left; right] -> extractComparison LE left right
-            | Comparison <@ (>=) @> [left; right] -> extractComparison GE left right
+            | Comparison <@ (=) @> (p, [left; right])  -> extractComparison p EQ left right
+            | Comparison <@ (<>) @> (p, [left; right]) -> extractComparison p NE left right
+            | Comparison <@ (<) @> (p, [left; right]) -> extractComparison p LT left right
+            | Comparison <@ (>) @> (p, [left; right]) -> extractComparison p GT left right
+            | Comparison <@ (<=) @> (p, [left; right]) -> extractComparison p LE left right
+            | Comparison <@ (>=) @> (p, [left; right]) -> extractComparison p GE left right
 
             | SpecificCall2 <@ fun (x:string) y -> x.StartsWith y @> (Some (AttributeGet attr), _, _, [value]) ->
                 let op = extractOperand None value

src/FSharp.DynamoDB/Picklers/Pickler.fs

@@ -30,6 +30,7 @@ type PicklerType =
     | Record        = 03
     | Union         = 04
     | Serialized    = 05
+    | Enum          = 06
 
 /// Untyped pickler base class
 [<AbstractClass>]
@@ -51,6 +52,11 @@ type Pickler() =
     /// Pickle any object, making an effort to coerce it to current pickler type
     abstract PickleCoerced : obj -> AttributeValue option
     default __.PickleCoerced obj = __.PickleUntyped obj
+
+    /// True if DynamoDB representation preserves
+    /// comparison semantics for query expressions
+    abstract IsComparable : bool
+    default __.IsComparable = false
 
     /// True if scalar DynamoDB instance
     member __.IsScalar = 

src/FSharp.DynamoDB/Picklers/PicklerResolver.fs

@@ -44,8 +44,7 @@ module private ResolverImpl =
             s.Accept {
                 new IEnumVisitor<Pickler> with
                     member __.VisitEnum<'E, 'U when 'E : enum<'U>> () =
-                        let up = resolver.Resolve<'U>() :?> NumRepresentablePickler<'U>
-                        new EnumerationPickler<'E, 'U>(up) :> _ }
+                        new EnumerationPickler<'E, 'U>() :> _ }
 
         | ShapeNullable s ->
             s.Accept {

src/FSharp.DynamoDB/Picklers/PrimitivePicklers.fs

@@ -23,6 +23,7 @@ type BoolPickler() =
     inherit StringRepresentablePickler<bool>()
     override __.PickleType = PickleType.Bool
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.DefaultValue = false
     override __.Pickle b = AttributeValue(BOOL = b) |> Some
@@ -37,6 +38,7 @@ type StringPickler() =
     inherit StringRepresentablePickler<string> ()
     override __.PickleType = PickleType.String
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.DefaultValue = null
     override __.Pickle s =
@@ -64,6 +66,7 @@ let inline mkNumericalPickler< ^N when ^N : (static member Parse : string * IFor
     { new NumRepresentablePickler< ^N>() with
         member __.PickleType = PickleType.Number
         member __.PicklerType = PicklerType.Value
+        member __.IsComparable = true
 
         member __.Parse s = parseNum s
         member __.UnParse e = toString e
@@ -83,6 +86,7 @@ type ByteArrayPickler() =
     inherit StringRepresentablePickler<byte[]> ()
     override __.PickleType = PickleType.Bytes
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.Parse s = Convert.FromBase64String s
     override __.UnParse b = Convert.ToBase64String b
@@ -99,6 +103,7 @@ type ByteArrayPickler() =
         else
             invalidCast a
 
+
 type MemoryStreamPickler() =
     inherit Pickler<MemoryStream> ()
     override __.PickleType = PickleType.Bytes
@@ -120,6 +125,7 @@ type GuidPickler() =
     inherit StringRepresentablePickler<Guid> ()
     override __.PickleType = PickleType.String
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.DefaultValue = Guid.Empty
     override __.Pickle g = AttributeValue(string g) |> Some
@@ -138,6 +144,7 @@ type DateTimeOffsetPickler() =
 
     override __.PickleType = PickleType.String
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.DefaultValue = DateTimeOffset()
     override __.Parse s = parse s
@@ -148,10 +155,12 @@ type DateTimeOffsetPickler() =
         if not <| isNull a.S then parse a.S 
         else invalidCast a
 
+
 type TimeSpanPickler() =
     inherit NumRepresentablePickler<TimeSpan> ()
     override __.PickleType = PickleType.String
     override __.PicklerType = PicklerType.Value
+    override __.IsComparable = true
 
     override __.Parse s = TimeSpan.FromTicks(int64 s)
     override __.UnParse t = string t.Ticks
@@ -161,21 +170,26 @@ type TimeSpanPickler() =
         if not <| isNull a.N then TimeSpan.FromTicks(int64 a.N) 
         else invalidCast a
 
-type EnumerationPickler<'E, 'U when 'E : enum<'U>>(up : NumRepresentablePickler<'U>) =
-    inherit NumRepresentablePickler<'E> ()
-    override __.PickleType = PickleType.Number
-    override __.PicklerType = PicklerType.Value
+
+type EnumerationPickler<'E, 'U when 'E : enum<'U>>() =
+    inherit StringRepresentablePickler<'E> ()
+    override __.PickleType = PickleType.String
+    override __.PicklerType = PicklerType.Enum
 
     override __.DefaultValue = Unchecked.defaultof<'E>
-    override __.Pickle e = let u = EnumToValue<'E,'U> e in up.Pickle u
-    override __.UnPickle a = EnumOfValue<'U, 'E>(up.UnPickle a)
-    override __.Parse s = up.Parse s |> EnumOfValue<'U, 'E>
-    override __.UnParse e = EnumToValue<'E, 'U> e |> up.UnParse
+    override __.Pickle e = AttributeValue(S = e.ToString()) |> Some
+    override __.UnPickle a = 
+        if notNull a.S then Enum.Parse(typeof<'E>, a.S) :?> 'E
+        else invalidCast a
+
+    override __.Parse s = Enum.Parse(typeof<'E>, s) :?> 'E
+    override __.UnParse e = e.ToString()
 
 type NullablePickler<'T when 'T : (new : unit -> 'T) and 'T :> ValueType and 'T : struct>(tp : Pickler<'T>) =
     inherit Pickler<Nullable<'T>> ()
     override __.PickleType = tp.PickleType
     override __.PicklerType = PicklerType.Wrapper
+    override __.IsComparable = tp.IsComparable
     override __.DefaultValue = Nullable<'T>()
     override __.Pickle n = if n.HasValue then tp.Pickle n.Value else AttributeValue(NULL = true) |> Some
     override __.UnPickle a = if a.NULL then Nullable<'T> () else new Nullable<'T>(tp.UnPickle a)
@@ -184,6 +198,7 @@ type OptionPickler<'T>(tp : Pickler<'T>) =
     inherit Pickler<'T option> ()
     override __.PickleType = tp.PickleType
     override __.PicklerType = PicklerType.Wrapper
+    override __.IsComparable = tp.IsComparable
     override __.DefaultValue = None
     override __.Pickle topt = match topt with None -> None | Some t -> tp.Pickle t
     override __.UnPickle a = if a.NULL then None else Some(tp.UnPickle a)
@@ -193,6 +208,7 @@ type OptionPickler<'T>(tp : Pickler<'T>) =
         | :? ('T option) as topt -> __.Pickle topt
         | _ -> raise <| new InvalidCastException()
 
+
 type StringRepresentationPickler<'T>(ep : StringRepresentablePickler<'T>) =
     inherit Pickler<'T> ()
     override __.PickleType = PickleType.String

tests/FSharp.DynamoDB.Tests/ConditionalExpressionTests.fs

@@ -11,7 +11,8 @@ open FSharp.DynamoDB
 [<AutoOpen>]
 module CondExprTypes =
 
-    type Enum = A = 0 | B = 1 | C = 2
+    [<Flags>]
+    type Enum = A = 1 | B = 2 | C = 4
 
     type Nested = { NV : string ; NE : Enum }
 
@@ -469,6 +470,23 @@ type ``Conditional Expression Tests`` () =
         test false <@ fun r -> r.HashKey = "2" && r.Bool = true @>
         test false <@ fun r -> r.HashKey = "2" && BETWEEN 1L r.RangeKey 2L @>
 
+    [<Fact>]
+    let ``Detect incompatible comparisons`` () =
+        let test outcome q = 
+            let f () = table.Template.PrecomputeConditionalExpr(q)
+            if outcome then f () |> ignore
+            else shouldFailwith<_, ArgumentException> f |> ignore
+
+        test true <@ fun r -> r.Guid > Guid.Empty @>
+        test true <@ fun r -> r.Bool > false @>
+        test true <@ fun r -> r.Optional >= Some "1" @>
+        test false <@ fun r -> r.Map > Map.empty @>
+        test false <@ fun r -> r.Set > Set.empty @>
+        test false <@ fun r -> r.Ref > ref "12" @>
+        test false <@ fun r -> r.Serialized <= (1L, "32") @>
+        test false <@ fun r -> r.Tuple <= (1L, 2L) @>
+        test false <@ fun r -> r.Nested <= r.Nested @>
+
     [<Fact>]
     let ``Simple Scan Expression`` () =
         let hKey = guid()

tests/FSharp.DynamoDB.Tests/UpdateExpressionTests.fs

@@ -11,7 +11,8 @@ open FSharp.DynamoDB
 [<AutoOpen>]
 module UpdateExprTypes =
 
-    type Enum = A = 0 | B = 1 | C = 2
+    [<Flags>]
+    type Enum = A = 1 | B = 2 | C = 4
 
     type Nested = { NV : string ; NE : Enum }