find() should use `ZREVRANGEBYSCORE` instead of `ZRANGEBYSCORE` when min>max. #73

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

Hello,
I faced problem with find() when min>max.

Here's the source code.

  models.newsfeed_status.find(
      {
        'written_date': {
          min: +since,
          max: '-inf',
          limit: 10
        }
      },
      function(err, ids) {
        console.log(ids);
      }
  );

On the current version, It results [].
I figured out that nohm uses only ZRANGEBYSCORE, and in this case, I think the proper action is using ZREVRANGEBYSCORE.
And, to be fully functional, I implemented endpoint option on find()'s search parameter.

models.newsfeed_status.find(
      {
        'written_date': {
          min: +since,
          max: '-inf',
          endpoint: '(]', 
          limit: 10
        }
      },
      function(err, ids) { 
        console.log(ids);
      }
  );

It results properly in descending order.

endpoint is similar to mathematical expression of interval. I think it is fairly straight-forward.
default value is [].

Also I added a test case in /test/findTests.js for your convenience.
Please review this.

Owner

maritz commented Jan 3, 2013

Thanks a lot. 👍

I've merged it. However I renamed endpoint to endpoints because I'm pedantic like that. :D

I also fixed some offset/limit issues that were left.

maritz closed this Jan 3, 2013

Owner

maritz commented Jan 3, 2013

Next thing is adding this to the docs, which I like to have before publishing it as a new version on npm. Would you be up for that?

@maritz It would be my pleasure. How do you maintain the docs? I couldn't find doc files at nohm repository. Should I add changes to the html docs and give it to you?

Oh, ;)
Thank you

Owner

maritz commented Jan 3, 2013

Do note: When I talk about the docs I mean the index.md, not the api/* stuff. I haven't updated the API docs in forever. :/

Owner

maritz commented Jan 13, 2013

@interruptz Any update on this? :)

Owner

maritz commented Mar 25, 2013

Reopening for docs, otherwise this will be forgotten.

maritz reopened this Mar 25, 2013

Works incorrectly when options.min=3 and options.max is not set. Because 3 > '+inf'

Hm, you mean '3' > '+inf', right? In that case it's a simple matter of making sure that if options[min/max] is parseInt'd if it's neither '-inf' nor '+inf'.

Or is there something else I'm missing?

Owner

maritz commented Aug 22, 2013

Interesting. Can you provide a test case so it's super easy for me to fix? :-)

Owner

maritz commented Sep 5, 2014

Okay, I included some more type conversions/checks and added some documentation.

I also changed the behaviour a tiny bit. But if you used full interval definitions before ("()", "[)" or "(]") instead of just "(" or ")", you should be fine.

Owner

maritz commented Sep 5, 2014

Finally published. Sorry for the delay.

maritz closed this Sep 5, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment