gadelkareem / sails-dynamodb

Amazon DynamoDB adapter for Waterline / Sails.js
http://sailsjs.org
46 stars 22 forks source link

Sort not working #34

Open arthurquerou opened 7 years ago

arthurquerou commented 7 years ago

It seems that sort conditions are not taken into account.

In index.js in the Sort section you have this code:

var sort = _.keys(options.sort)[0];
      if (sort) {
          query.ascending();
        }
        else if (sort == -1) {
          query.descending();
        }

It can't even work as sort equals the key and not the value.

Are there any plan to update this behaviour ?

ferrants commented 7 years ago

Trying to work this out myself as well. According this library: https://github.com/ryanfitz/vogels, For non-scanning queries, sort requires indicating an Index to use if you're not using the primary index of the table. You can't or shouldn't be able to sort on an arbitrary field using dynamo unless you're using scan. The ascending/descending should respect the range field of the index you're using.

Looks like the index is set around line 546: https://github.com/gadelkareem/sails-dynamodb/blob/master/index.js#L546

          if (indexName && indexName != 'primary') {
            query.usingIndex(indexName);
          }

Regardless of this, the lines you posted don't make sense for the handling of the sort. It gets the first key in the sort dictionary and expects that to be -1 for descending or 1 for ascending. The number -1 can not be the key of a dictionary, so you can never have descending. sort: {-1: something} doesn't make sense. I think it should be:

sort: {indexName: -1}

to sort that index in a descending way.

I made https://github.com/gadelkareem/sails-dynamodb/pull/35 to see if that works

ferrants commented 7 years ago

You can use sort: "-1" to descendingly sort based on whatever index it picks up. The sort variable ends up being { '-1': 1 }, which seems to work. My Pr didn't work.

ferrants commented 7 years ago

I made https://github.com/gadelkareem/sails-dynamodb/pull/36 for a documentation update to show how this feature works for this adapter

jkeczan commented 7 years ago

When I sort by a -1, I am getting query.descending not a function defined on line 897.

Any thoughts?

TypeError: query.descending is not a function
    at Object._searchCondition (/Users/jkeczan/Projects/web/external/straingauge-web/node_modules/sails-dynamodb/index.js:897:17)
ferrants commented 7 years ago

My thoughts are that you may not have an index on what you are searching by or ordering by so it's trying to do it in scan mode instead of query mode.

ggarcia92 commented 6 years ago

Hi, I'm trying to sort by updatedAt but when I define an index to that attribute on model and lunch local dynamodb throws this error "Cannot read property 'dynamoType' of undefined". Is this a bug?

ferrants commented 6 years ago

Doesn't sound like a bug or this open issue. Sounds like a configuration issue.