fsprojects / FSharp.AWS.DynamoDB

F# wrapper API for AWS DynamoDB
MIT License
58 stars 18 forks source link

Added support for Array.contains and List.contains #67

Closed faldor20 closed 1 year ago

faldor20 commented 1 year ago

I have completed the partially complete support for the "IN" keyword. Array.contains and List.contains can both compare a list of local values against a single value from the table They generate query code like the following:

a.tableVal IN("val1","val2")

I also added support for those same Array.contains and List.contains operators to work against table values This would generate code like:

contains(a.tableList,"b")
samritchie commented 1 year ago

Whitespace in general here is a little off - is it worth having a look at putting the standards in the .editorconfig and automatically applying (or checking)? Does @fsprojects have any sort of standard formatting?

bartelink commented 1 year ago

I forgot to say: nice feature, great that the README and all is included, kudos!

Does @fsprojects have any sort of standard formatting?

Doubt it. If one comes about, I'll be stealing stuff to put in https://github.com/jet/equinox/blob/master/.editorconfig (maybe that makes sense as a base - it's not exactly extensive, but IME the things I highlighted are the main things that tend to bother me, and doing a cleanup to make the line endings/training whitespace consistent before this PR would be excellent given how much it affects this PR).

faldor20 commented 1 year ago

Thanks! Sorry about the messy formatting, It's just not something I've ever really noticed. 😅

faldor20 commented 1 year ago

Okay @bartelink I implemented the most performant solution available. I had to replace the nice pattern matching writeops, but it's now as fast and memory efficient as it can be.

samritchie commented 1 year ago

Sigh, more fake-related breakages. It looks like there’s a straightforward fsi-related fix, I’ll have a go at this.

samritchie commented 1 year ago

@faldor20 I’ve fixed the fake build with #69 if you want to merge that into this PR - thanks

faldor20 commented 1 year ago

@samritchie @bartelink If one of you could approve this that would be great :)