guregu / dynamo

expressive DynamoDB library for Go
Other
1.31k stars 179 forks source link

Exposing condition builder publicly #235

Open alexwennerberg opened 5 months ago

alexwennerberg commented 5 months ago

Several operations (Put, Update, Delete) use a condition expression. This is represented externally as a string and variable interface{} parameters.

The If function is the public interface used to build a complex condition. However, the library does not allow for more complex condition building. The only options for If involve combining clauses with And, but the library does not expose its lower-level functions for building conditions, e.g. subExprN and other functions in that file.

How would you feel about exposing these functions publicly, and adding a method to Put, Update and Delete along the lines of SetCondition, so that users of this library can build a condition separately, then set it on the request, allowing for more flexibility and fine-grained control than If -- as well as the ability to reuse and combine conditions?

I'm still new to using this library, so let me know if this makes sense or if there are any substantial issues with this idea :)

guregu commented 5 months ago

Hello, it’s an interesting idea. Maybe some concrete use case examples could help flesh out a potential API. I don’t think there’s anything that you’re prevented from doing with the current API, but I could be missing some perspective.

For example, you can use OR expressions within If(…). Maybe an API for building ORs programmatically would be nice?

There is the ExpressionLiteral type which provides low-level control of what exactly gets substituted: https://pkg.go.dev/github.com/guregu/dynamo#ExpressionLiteral, this can be used as a building block for fancier stuff or experiments. It should be compatible with the AWS SDK’s expression builder thingy.

alexwennerberg commented 5 months ago

Basically, my use case is to build composable reusable conditions across queries. My issue is that I want a combination of And and Or conditions. If gives me access to AND, but in a sort of implicit manner. It may be useful to have an API that uses the AND/OR/NOT etc words directly. The function takes in an untyped string currently, and my understanding would also be that it’d be easy to accidentally create a query injection with unsafe input (although I may be missing some protections). I’m thinking along the lines of query builders that are common in e.g. SQL libraries, but it’s possible that i’m misapplying it to dynamodb! hope this made sense, typed it up on mobile in a bit of a rush. thanks again for this library!

guregu commented 5 months ago

I see, yeah. I can offer some ideas.

Parameterized queries are built into the DynamoDB API in that all values are always substituted for a placeholder in expressions, in this library they are represented by ?. There's no way to write a literal value in an expression. Attribute names can be literals or use placeholders, in this library they are $ or 'single quoted names'. $ can also be used for array indexing (as in .If("'Foo'.'Bar'[$]" = ?, 123, "abc")), and as a special case the ExpressionLiteral type.

For this reason you don't need to worry about SQL injection type of issues, as long as you use the placeholders for potentially untrusted input. It's the same idea as the stdlib database/sql package.

However I do understand it's annoying to generate the expression strings dynamically for doing OR and such. The stdlib sql package has the same issue with the user needing to generate the correct number of ?s for IN(?, ?, ?, ...) expressions for example.

The official SDK has this package which provides a builder for expressions, which I think is kinda what you're looking for: https://docs.aws.amazon.com/sdk-for-go/api/service/dynamodb/expression/

You can use this alongside dynamo.ExpressionLiteral type:

cond := expression.Name("Rating").LessThan(expression.Value(7))
expr, err := expression.NewBuilder().
    WithCondition(cond).
    Build()
lit := dynamo.ExpressionLiteral{Expression: expr.Condition(), AttributeNames: expr.Names(), AttributeValues: expr.Values()}
err := table.Put(item).If("$", lit).Run()

That is a lot of stuff to type out, maybe we should make it easier. I have considered adding our own expression builder thing but I'm not sure how we could improve over the official SDK's. I think it could be useful though.

Generally what I end up doing for really fancy queries is something like this:

type Filter interface{
    Apply(*dynamo.Query)
}

type OrFilter map[string]any // attribute name → value

func (of OrFilter) Apply(q *dynamo.Query) {
    var sb strings.Builder
    args := make([]any, 0, len(of)*2)
    for k, v := range of {
        if sb.Len() != 0 {
            sb.WriteString(" OR ")
        }
        sb.WriteString("($ = ?)")
        args = append(args, k, v)
    }
    q.Filter(sb.String(), args...)
}

But I don't think that is particularly elegant either (and of course is much less flexible than a full expression builder).

alexwennerberg commented 5 months ago

Interesting, I hadn't thought about using the go expression builder directly, it seems like that may be a reasonable approach. Thanks for thinking about this issue, I will look into your code samples a bit!