You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/05/25 12:15:57 UTC
[GitHub] [couchdb] hklarner opened a new pull request #3582: adds operators $sizeLte and $sizeGte
hklarner opened a new pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
- supports inequality matches for `$size`
- example: between 10 and 20 customers in array
- `"doc.customers": {"$sizeGte": 10}`
- `"doc.customers": {"$sizeLte": 20}`
## Testing recommendations
Tests are added to
- src/mango/test/03-operator-test.py
## Related Issues or Pull Requests
None
## Checklist
- [ ] Code is written and works correctly
- [ ] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] bessbd commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-852796440
@hklarner : https://github.com/apache/couchdb/pull/3568 is very close to a merge. Do you want me to hold off with it so that this can be merged before?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] garrensmith commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
garrensmith commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-849551130
@hklarner take a look at some of the other operators that handle nested values. Maybe $and and look at implementing something similar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] glynnbird edited a comment on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
glynnbird edited a comment on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-848792131
Thanks for the contribution @hklarner. On a broader question the new operators you are creating:
- `$sizeGte`
- `$sizeLte`
have no precedence in MongoDB which was the original inspiration for the MongoDB language. If I'm reading the MongoDB docs right, the MongoDB way of performing this query would be to allow the following syntax:
```
{
"selector": {
"customers": {
"$size:": { "$gte": 20}
}
}
}
```
The advantage of this approach is that
a) we already have a `$size` operator.
b) we have plenty of other comparison operators `$eq`, `$gte` etc etc
c) it wouldn't be creating new operators that differ from the "MongoDB way"
Note that currently the Mango query parser doesn't support this syntax, but I'm suggesting it could be modified to, rather than adding lots of new `$size...` operators.
What do you thinkg @hklarner & @garrensmith?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] glynnbird commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
glynnbird commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-848792131
Thanks for the contribution @hklarner. On a broader question the new operators you are creating:
- `$sizeGte`
- `$sizeLte`
have no precedence in MongoDB which was the original inspiration for the MongoDB language. If I'm reading the MongoDB docs right, the MongoDB way of performing this query would be to allow the following syntax:
```
{
"selector": {
"customers": {
"$size:": { "$gte": 20}
}
}
}
```
The advantage of this approach is that
a) we already have a `$size` operator.
b) we have plenty of other comparison operators `$eq`, `$gte` etc etc
c) it wouldn't be creating new operators that differ from the "MongoDB way"
What do you thinkg @hklarner & @garrensmith?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] hklarner edited a comment on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
hklarner edited a comment on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-850523672
@jjrodrig that's great! Hey, because of your answer on stackoverflow I opened this PR! Thanks for helping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] garrensmith commented on a change in pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#discussion_r639726181
##########
File path: src/mango/test/03-operator-test.py
##########
@@ -75,6 +75,21 @@ def test_keymap_match(self):
docs = self.db.find({"foo": {"$keyMapMatch": {"$eq": "aa"}}})
self.assertEqual(len(docs), 1)
+ def test_size(self):
Review comment:
Could you add another test that shows it handles the case where the `sizeGte` value is not an integer and another when it is acting on a field that is not an array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] hklarner commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
hklarner commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-848794736
@glynnbird I see. Agreed, the MongoDB consistency is preferable. But I don't know how to implement it :sweat_smile:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] hklarner commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
hklarner commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-850523672
@jjrodrig that's great! Hey, because of your answer on stackoverflow I opened the issue! Thanks for helping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] hklarner commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
hklarner commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-852811217
@bessbd no, because I won't be able to continue with this for at least another week. All I have to do to be up to date is pull before I continue, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] hklarner commented on a change in pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
hklarner commented on a change in pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#discussion_r639754182
##########
File path: src/mango/test/03-operator-test.py
##########
@@ -75,6 +75,21 @@ def test_keymap_match(self):
docs = self.db.find({"foo": {"$keyMapMatch": {"$eq": "aa"}}})
self.assertEqual(len(docs), 1)
+ def test_size(self):
Review comment:
I have added tests for failures but I don't know how to "assert failure" in this test script (need help).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] bessbd commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-853039319
> @bessbd no, because I won't be able to continue with this for at least another week. All I have to do to be up to date is pull before I continue, right?
That's pretty much it. In case you are changing files that are modified by the reformat, the conflict resolution may become a little difficult, but that shouldn't be a big deal.
In case you need help with anything, please let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jjrodrig commented on pull request #3582: adds operators $sizeLte and $sizeGte
Posted by GitBox <gi...@apache.org>.
jjrodrig commented on pull request #3582:
URL: https://github.com/apache/couchdb/pull/3582#issuecomment-850499211
Hi @hklarner,
I was checking this. It seems quite simple to introduce nesting the behaviour for the $size operator. I've been testing with these changes and seems to work.
```diff
--- a/src/mango/src/mango_selector.erl
+++ b/src/mango/src/mango_selector.erl
@@ -145,6 +145,8 @@ norm_ops({[{<<"$keyMapMatch">>, Arg}]}) ->
norm_ops({[{<<"$size">>, Arg}]}) when is_integer(Arg), Arg >= 0 ->
{[{<<"$size">>, Arg}]};
+norm_ops({[{<<"$size">>, {_}=Arg}]}) ->
+ {[{<<"$size">>, norm_ops(Arg)}]};
norm_ops({[{<<"$size">>, Arg}]}) ->
?MANGO_ERROR({bad_arg, '$size', Arg});
@@ -581,8 +583,10 @@ match({[{<<"$regex">>, Regex}]}, Value, _Cmp) when is_binary(Value) ->
match({[{<<"$regex">>, _}]}, _Value, _Cmp) ->
false;
-match({[{<<"$size">>, Arg}]}, Values, _Cmp) when is_list(Values) ->
+match({[{<<"$size">>, Arg}]}, Values, _Cmp) when is_list(Values), is_integer(Arg) ->
length(Values) == Arg;
+match({[{<<"$size">>, Arg}]}, Values, _Cmp) when is_list(Values) ->
+ match(Arg, length(Values), _Cmp);
match({[{<<"$size">>, _}]}, _Value, _Cmp) ->
false;
```
I hope this could help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org