Create new AttributeFilter .OnKey<T>(T key)

[devtekve]

I am proposing the creation of a filter .OnKey<T>(T key) to support more generic operations, so the caller doesn't need to explicitly know the distribution of the entity. Since the entity needs to have one property flagged [DynamoDbProperty(nameof(xx), DynamoDbAttributeType.PartitionKey)], if the entity is properly configured, should be trivial for us to know what to query.

If the entity is not properly configured, no query would work anyway :D

[firenero]

If the entity is not properly configured, no query would work anyway

In fact, Query operations will work without correctly specified primary and sort keys. It happens because we translate the code into DDB query syntax without checking the primary key correctness. It's done this way because it's possible to write a query against an index that will have unmarked primary key. That said, it's more of a side effect and we don't "officially" support incorrectly configured entities.

Can you write an example of what this .OnKey<T>(T key) method is supposed to be used because I'm not fully following the idea so far.

[devtekve]

That's interesting. I didn't try it passing it wrong :D, but yeah that sounds like something we might want to fix in the future. But since it's not proper flow, I don't think it's urgent.

For the example... Right now a query looks like this:

await context.Query<TEntity>().WithKeyExpression(cond => cond.On(x => x.RecordId).EqualTo(id))

And I'd like to be able to do it like this:

await context.Query<TEntity>().WithKeyExpression(cond => cond.OnKey(id))

and also

await context.Query<TEntity>().WithKeyExpression(cond => cond.OnKey().Contains(id))
await context.Query<TEntity>().WithKeyExpression(cond => cond.OnKey().[...](id)) 

[firenero]

Theoretically it's possible but the API needs to be changed a bit. We need to have separate methods like OnPartitionKey and OnSortKey to make it work.

For example:

await context.Query<TEntity>().WithKeyExpression(cond => cond
    .OnPartitionKey().EqualTo(id)
    .OnSortKey().LessThan(someValue)
);

However there is a flaw that prevents this approach from working with indexes. As I previously mentioned, there are no specific attributes for primary keys on GSIs so we don't know which fields to use with this approach. It's not easy to make it work because we'll need to have separate attributes tied to specific indexes.

Something like this:

[DynamoDbTable("table")]
public class Entity
{
  [DynamoDbProperty("pk", DynamoDbAttributeType.PartitionKey)]
  public string PartitionKey { get; set; }

  [DynamoDbProperty("gsi_pk", DynamoDbAttributeType.PartitionKey, "index_name")]
  public string SomeIndexPartitionKey { get; set; }
}

Theoretically it's possible but it's a larger chunk of work that can be harder to integrate into current flow. I also don't want to break the current approach because it will force everyone to change all existing queries.

Also, with OnPartitionKey and OnSortKey we can potentially limit builder methods to operations that possible on these keys. E.g. OnPartitionKey() can only be followed with EqualTo(...) because no other operations are allowed for partition keys in DynamoDB.

---

However, there is a massive downside -- OnPartitionKey() and OnSortKey() will be visible for WithFilterExpression(...) method but they can't be applied there. That's a quite confusing behavior that I would like to avoid if possible.

[devtekve]

Yeah, it adds some complexity... I dont know if something like this would be possible, but what if we were to do it on

await context.Query<TEntity>(IAttributeFilter<T> keyFilter? key, IAttributeFilter<T> keyFilter? sort)

And if that overload is used, we remove the .FromIndex() ? (we'd have to do a different return type for it)

[firenero]

It should work but I can't say that I like that approach. It makes this specific overload completely different to all other ways to use the library that can cause confusion and adds additional contract we can't simply remove if needed. In general, I'd try to avoid introducing another API to the library that doesn't add significant benefits but requires us to support it for a long time.

That said, I'm not against the original suggestion but I'd like to put more thoughts into it to get thee best API possible.