EfficientDynamoDb icon indicating copy to clipboard operation
EfficientDynamoDb copied to clipboard

Cannot create Querys with Global Secondary Indexes

Open wjax opened this issue 3 years ago • 5 comments

It does not seem to be able to query using a GSI. In fact there is no annotation available to distinguish a PK from a GSI-PK.

Is this something that is not implemented?

Thanks

wjax avatar Apr 21 '22 10:04 wjax

Hey @wjax. No extra annotations are needed to use GSIs. You need to use the fluent API method FromIndex("IndexName") and use the primary key of that GSI configured in DDB. There is an example in old ticket #168.

Seems like our docs are missing the section about GSI usage. I'll try to update it but can't provide an ETA yet.

firenero avatar Apr 21 '22 12:04 firenero

Thanks a lot. I missed that part.

However, there is still a small issue (not dircetly related). Tell me if I miss something again or if your would like me to open a new issue.

I get this:

EfficientDynamoDb.Exceptions.DdbException: Entity 'InterfaceDefinition' has no DynamoDbTable attribute applied

The doc says DynamoDbTable is not mandatory, however it seems it is for queries. Put item works without it, but not query. In my case, I prefer to use .WithTableName that is coming from AWS AppConfig settings rather than annotating the class.

If I annotate the class with DynamoDbTable("Dummy"), and overrides the query with .WithTableName, it works ok and the table in the With expression takes precedence. So I guess it is only a cosmetic problem (if any)

Thanks again

wjax avatar Apr 21 '22 12:04 wjax

Yeah, it definitely looks like a bug.

lezzi avatar Apr 21 '22 13:04 lezzi

It does not check if a TableNode has been added when executing the query, for example

public IAsyncEnumerable<IReadOnlyList<TEntity>> ToPagedAsyncEnumerable()
{
    var tableName = _context.Config.Metadata.GetOrAddClassInfo(typeof(TEntity)).GetTableName();
    return _context.QueryAsyncEnumerable<TEntity>(tableName, GetNode());
}

It should, either check if a TableNode has been added before GetTableName or do not check at all and let it fail.

I have however implemented a quick nasty fix. Added in ITableBuilder

public bool CheckTableName()
        {
            if (Node is null)
                return false;
            else
            {
                var node = Node;
                do
                {
                    if (node is TableNameNode)
                        return true;
                } while ((node = node.Next) != null);

                return false;
            }
        }

And then:

public async Task<IReadOnlyList<TEntity>> ToListAsync(CancellationToken cancellationToken = default)
        {
            string tableName = "";
            
            if ((this as ITableBuilder<IQueryEntityRequestBuilder<TEntity>>).CheckTableName() is false)
                tableName = _context.Config.Metadata.GetOrAddClassInfo(typeof(TEntity)).GetTableName();
            
            return await _context.QueryListAsync<TEntity>(tableName, GetNode(), cancellationToken).ConfigureAwait(false);
        }

wjax avatar Apr 21 '22 14:04 wjax

The existing HTTP builder code is already capable to handle the missing table names, we just accidentally forced it. I have removed unnecessary validation and published the new 0.9.13 stable version with some recent fixes.

lezzi avatar Apr 27 '22 12:04 lezzi