Add support for same-attribute if-not-exists updates #76
Conversation
…alue")` and `r with Attribute = defaultArg (Some r.Attribute) "value"` for if_not_exists support. Fixes #74
| @@ -295,7 +295,9 @@ let extractUpdateOps (exprs: IntermediateUpdateExprs) = | |||
| | SpecificCall2 <@ List.append @> (None, _, _, [ left; right ]) -> | |||
| UpdateValue.EList_Append (extractOperand pickler left) (extractOperand pickler right) | |||
|
|
|||
| | SpecificCall2 <@ defaultArg @> (None, _, _, [ AttributeGet attr; operand ]) -> | |||
| | SpecificCall2 <@ defaultArg @> (None, _, _, [ AttributeGet attr; operand ]) | |||
| | Some'(SpecificCall2 <@ defaultArg @> (None, _, _, [ AttributeGet attr; operand ])) | |||
There was a problem hiding this comment.
this looks very cryptic - maybe NewSome or OptionSome or SomeCaseCtor naming instead?
| match e with | ||
| | NewUnionCase(uci, [ v ]) -> | ||
| let dt = uci.DeclaringType | ||
| if dt.IsGenericType && dt.GetGenericTypeDefinition() = typedefof<_ option> && uci.Name = "Some" then |
There was a problem hiding this comment.
| if dt.IsGenericType && dt.GetGenericTypeDefinition() = typedefof<_ option> && uci.Name = "Some" then | |
| if dt.IsGenericType && dt.GetGenericTypeDefinition() = typedefof<_ option> && uci.Name = (nameof Some) then |
| let item = mkItem () | ||
| let key = table.PutItem item | ||
| clearAttribute table key "String" | ||
| let item' = table.UpdateItem(key, <@ fun (r: R) -> { r with String = defaultArg (Some r.String) "<undefined>" } @>) |
There was a problem hiding this comment.
was about to comment that this is the same as L272 as the clearAttributedid not jump out at me as being the important bit
is there any way that 263 could do let key = table.PutItem { item with .... } or is it important tha the clear is a separate call?
There was a problem hiding this comment.
It’s a non-optional field, it can’t be set to None on the record. I’m not convinced we need if-not-exists support for these non-optionals, it’s only likely to come about as the result of schema changes, but supporting it shouldn’t break anything important
There was a problem hiding this comment.
tbh I have very little idea what the behavior and/or schema is - from my perspective it's a small piece of expression handling with supporting tests that will be used so it meets the bar regardless of whether I even get how it works ;)
| AttributeUpdates = Dictionary(Map.ofSeq [ attribute, new AttributeValueUpdate(Action = AttributeAction.DELETE) ]) | ||
| ) | ||
| ) | ||
| |> Async.AwaitTask |
There was a problem hiding this comment.
could make the test async rather than baking this in?
There was a problem hiding this comment.
In that case pretty much all the tests should be async. It’s probably a good idea but I’ll do this separately
Fixes #74. Needs some additional tests to make sure all scenarios work properly.