michaellavelle / spring-data-dynamodb

Simplifies the development of creating an Amazon DynamoDB-based Java data access layer
https://derjust.github.io/spring-data-dynamodb/
Apache License 2.0
169 stars 284 forks source link

Unable to query by attribute when attribute is used as hashkey of more than one global secondary index #21

Closed rshan2 closed 10 years ago

rshan2 commented 10 years ago

I had a table with a global secondary index with hash key 'type' and range key 'timestamp' and was able to perform queries like this:

List<Resource> resources = findByType(type);

I then added another global secondary index with the same hash key and a range key 'name'. Consequently my queries fail with RuntimeException("Don't know which index name to use");

The associated code in AbstractDynamoDBQueryCriteria.getGlobalSecondaryIndexName is:

if (getAttributeName(indexNamesEntry.getKey()).equals(singleAttributeConditions.getKey())) {
    String[] indexNames = indexNamesEntry.getValue();
    if (indexNames.length > 1) {
        throw new RuntimeException("Don't know which index name to use");
    }
    String newIndexName = indexNames[0];

    ....

Would it be acceptable to simply select the first index in this case? I realize that the order of results depend on the selected index, but in this case I'm not interested in order. When order is important, then I'll use a query such as findByTypeAndTimestamp.

rshan2 commented 10 years ago

Actually, I see there is more to this issue. I created a query like

findByTypeAndTimestampLessThan

and in the same area of code the exception RuntimeException("Already using a different indexName:" + indexName) is thrown. I haven't ascertained if the issue is rooted generally in multiple global secondary indexes or more specifically multiple global secondary indexes that share the same hash key.

rshan2 commented 10 years ago

OK, with the following change, my queries are now working:

String indexName = null;
for (Entry<String, List<Condition>> singleAttributeConditions : attributeConditions.entrySet()) {
    if(this.entityInformation.isGlobalIndexHashKeyProperty(singleAttributeConditions.getKey()) && attributeConditions.size() > 1) {
        continue;
    }

    for (Map.Entry<String, String[]> indexNamesEntry : entityInformation.getGlobalSecondaryIndexNamesByPropertyName()
                .entrySet()) {
    if (getAttributeName(indexNamesEntry.getKey()).equals(singleAttributeConditions.getKey())) {
         String[] indexNames = indexNamesEntry.getValue();
         String newIndexName = indexNames[0];

         if (indexName != null) {
             if (indexName.equals(newIndexName)) {
             } else {
                throw new RuntimeException("Already using a different indexName:" + indexName);
             } else {
                 indexName = newIndexName;
             }
        }
    }
}
return indexName;
michaellavelle commented 10 years ago

Thanks for raising this Bob. I've refactored the existing getGlobalSecondaryIndexName to make it easier to understancd, but also so the method now attempts to determine the best index to use in the event that multiple indexes span the properties specified in a search critiera, rather than the first matching index encountered.

For example, if searching by global secondary index hash key, if there exists an index on this property only, and also a second index on this property and some other secondary range-key property, the method will need to select the hash-key index for efficiency reasons. Also, users may wish to have control over which index is used in the event of a tie-break between matching multi-property indexes for global secondary hash key searches - now in such a scenario the method should return the first declared index on the annotation for the global secondary hash key.

Would you mind checking your use-cases to see if this has resolved the issue when you get chance?

Many thanks,

Michael

rshan2 commented 10 years ago

Michael, nice job. Handles all of my current use cases and getGlobalSecondaryIndexName is indeed much easier to understand. Thanks for the quick turnaround.

-Bob