Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update #'rethinkdb.query/filter to allow optargs #43

Merged
merged 6 commits into from
Jun 30, 2015

Conversation

yurrriq
Copy link
Contributor

@yurrriq yurrriq commented Jun 19, 2015

Per the docstring:

By default RethinkDB will ignore documents where a specified field is missing.
Passing {:default (r/error)} as an optional argument will cause any
non-existence error to raise an exception.

Currently, something like...

(r/filter sq predicate {:default true})

...throws an exception:

Wrong number of args (3) passed to: query/filter

@danielcompton
Copy link
Collaborator

@yurriq could you add tests to cover this new feature as well? (Reminds me to add tests to my open PRs)

@yurrriq
Copy link
Contributor Author

yurrriq commented Jun 19, 2015

For sure.

@yurrriq
Copy link
Contributor Author

yurrriq commented Jun 19, 2015

I added a basic test that fails on 80f7da0 and passes on yurrriq/clj-rethinkdb@7927c6c, but I'm not thrilled with it...

Any ideas for improvement are most welcome.

@danielcompton
Copy link
Collaborator

Hey sorry for the delay on this, I've been away. I'll try and get to it in the next few days.

@yurrriq
Copy link
Contributor Author

yurrriq commented Jun 29, 2015

Sounds good.

@danielcompton
Copy link
Collaborator

This all looks pretty good, there's a few more things this needs before it's ready, but I'm happy to help/do the work on these:

  • Update the docstring to match http://rethinkdb.com/api/javascript/filter/, e.g.

    • If default is set to true, documents with missing fields will be returned rather than skipped.
    • If default is set to r.error(), an RqlRuntimeError will be thrown when a document with a missing field is tested.
    • If default is set to false (the default), documents with missing fields will be skipped.
  • Add test cases for all three defaults

  • Modify existing test case to not use :bool true, and to filter on something different. To someone who wasn't familiar with filter, it looks like we're setting a default value (is :bool is not found, it equals true), rather than default behaviour (skipping/including record)

yurrriq added 4 commits June 30, 2015 01:39
* commit '95ea74e1b26da9abd1ccd909cc959e1eb847b435':
  Add gitter.im webhooks for CircleCI
  Move Gitter badge
  Add new arity for table to use default database
  Added Gitter badge
@yurrriq
Copy link
Contributor Author

yurrriq commented Jun 30, 2015

I merged in master, updated the docstring and modified/added tests. Let me know what you think.

@danielcompton
Copy link
Collaborator

Hey this looks great! Thanks so much.

danielcompton added a commit that referenced this pull request Jun 30, 2015
Update #'rethinkdb.query/filter to allow optargs
@danielcompton danielcompton merged commit 383967a into apa512:master Jun 30, 2015
@yurrriq
Copy link
Contributor Author

yurrriq commented Jun 30, 2015

Cheers!

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

Successfully merging this pull request may close these issues.

2 participants