diff --git a/src/FSharp.DynamoDB/Expression/ConditionalExpr.fs b/src/FSharp.DynamoDB/Expression/ConditionalExpr.fs index e909c5e..c1b084a 100644 --- a/src/FSharp.DynamoDB/Expression/ConditionalExpr.fs +++ b/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 diff --git a/src/FSharp.DynamoDB/Picklers/Pickler.fs b/src/FSharp.DynamoDB/Picklers/Pickler.fs index 8eb3364..837c8d5 100644 --- a/src/FSharp.DynamoDB/Picklers/Pickler.fs +++ b/src/FSharp.DynamoDB/Picklers/Pickler.fs @@ -30,6 +30,7 @@ type PicklerType = | Record = 03 | Union = 04 | Serialized = 05 + | Enum = 06 /// Untyped pickler base class [] @@ -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 = diff --git a/src/FSharp.DynamoDB/Picklers/PicklerResolver.fs b/src/FSharp.DynamoDB/Picklers/PicklerResolver.fs index 274822d..2c34f3d 100644 --- a/src/FSharp.DynamoDB/Picklers/PicklerResolver.fs +++ b/src/FSharp.DynamoDB/Picklers/PicklerResolver.fs @@ -44,8 +44,7 @@ module private ResolverImpl = s.Accept { new IEnumVisitor 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 { diff --git a/src/FSharp.DynamoDB/Picklers/PrimitivePicklers.fs b/src/FSharp.DynamoDB/Picklers/PrimitivePicklers.fs index 418aa33..360735a 100644 --- a/src/FSharp.DynamoDB/Picklers/PrimitivePicklers.fs +++ b/src/FSharp.DynamoDB/Picklers/PrimitivePicklers.fs @@ -23,6 +23,7 @@ type BoolPickler() = inherit StringRepresentablePickler() 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 () 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 () 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 () override __.PickleType = PickleType.Bytes @@ -120,6 +125,7 @@ type GuidPickler() = inherit StringRepresentablePickler () 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 () 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> () 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 diff --git a/tests/FSharp.DynamoDB.Tests/ConditionalExpressionTests.fs b/tests/FSharp.DynamoDB.Tests/ConditionalExpressionTests.fs index 88dd9db..c9934ee 100644 --- a/tests/FSharp.DynamoDB.Tests/ConditionalExpressionTests.fs +++ b/tests/FSharp.DynamoDB.Tests/ConditionalExpressionTests.fs @@ -11,7 +11,8 @@ open FSharp.DynamoDB [] module CondExprTypes = - type Enum = A = 0 | B = 1 | C = 2 + [] + 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 @> + [] + 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 @> + [] let ``Simple Scan Expression`` () = let hKey = guid() diff --git a/tests/FSharp.DynamoDB.Tests/UpdateExpressionTests.fs b/tests/FSharp.DynamoDB.Tests/UpdateExpressionTests.fs index 1914823..02ac8f2 100644 --- a/tests/FSharp.DynamoDB.Tests/UpdateExpressionTests.fs +++ b/tests/FSharp.DynamoDB.Tests/UpdateExpressionTests.fs @@ -11,7 +11,8 @@ open FSharp.DynamoDB [] module UpdateExprTypes = - type Enum = A = 0 | B = 1 | C = 2 + [] + type Enum = A = 1 | B = 2 | C = 4 type Nested = { NV : string ; NE : Enum }