You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "pgj (via GitHub)" <gi...@apache.org> on 2023/07/30 15:54:13 UTC
[GitHub] [couchdb] pgj opened a new pull request, #4710: feat(`mango`): strict index selection
pgj opened a new pull request, #4710:
URL: https://github.com/apache/couchdb/pull/4710
It is not always beneficial for the performance if the Mango query planner tries to assign an index to the selector. User-specified indexes may save the day, but since they are only hints for the planner, automated overrides can still happen.
Introduce the concept of "strict index selection" (h/t @willholley) which lets the user to request the exclusive use of a specific index. When it is not possible, give up on planning and return an HTTP 400 response right away. This way the user has the chance to learn about the missing index, request its creation and try again later.
The feature comes with a configuration toggle. By default, the feature is disabled to maintain backward compatibility, but the user may ask for this behavior via the new `use_index_strict` query parameter for a specific query. Note that, similarly, `use_index_strict` could be used to disable strict index selection temporarily when it is enabled on the server.
Fixes #4511
## Checklist for the review
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [x] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [x] Documentation changes were made in the `src/docs` folder
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731554533
Thank you all for the comments, it is much appreciated.
How about the following:
- Since changing the current semantics of `use_index` would break many existing integrations, even if it is indeed a bug, let us keep that for backwards compatibility and introduce `index_hint` for the current behavior (as an alias to `use_index`) and `force_index` for the desired behavior.
- Create another PR for the `index_hint` + `force_index` work and we leave this thread as the generic Mango discussion although it might be better to find a designated place for that.
- Spin off the parts about the suspected bug about `$regex` into the ticket and continue the related discussion there.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660312488
@janl we commented concurrently. I agree with your last. :)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on pull request #4710: feat(`mango`): strict index selection
Posted by "iilyak (via GitHub)" <gi...@apache.org>.
iilyak commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1661053944
> If we don't want that, then we need do nothing.
I don't have that strong of opinion. My only strong opinion is to involve Rich and his team in the design.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731412471
as for comparisons to calling `_view` and `_search` with no restrictions, I don't think that's true, and the details matter.
Querying a view without startkey/endkey can make sense and can be efficient as it's quite common for the view to be a subset of the database (any `if` clause on doc properties, say). And it could be a reduce=true or group=true and the view-wide reduce value might be valuable (and is efficiently calculated).
Querying a search index with `*:*` is also sometimes reasonable (as it might be a subset of the database as above) and will certainly be more efficient, Lucene _won't_ read the whole index to perform the query, it'll just return the first N results it finds (as a limit is not optional for Lucene as it is for views).
You can tell I am tired of dealing with users who have been let down by mango and its false promises.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1716142282
definitely `_find` will only work for indexes created with `_index` for the reasons you state.
I think you're right that, in practice, the index-level `_find` will have to allow the 'middle' cases. e.g an mr-backed index on two fields where the selector is only on the first first, so a wider start/endkey (`["foo"]` to `["foo",{}]`) and then filter out the non-matches.
That's still a lot better than db-level `_find`s failure modes. We _wouldn't_ allow a startkey/endkey that covers the entire view (or the `*:*` equivalent for search, though that at least doesn't return all the hits).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(`mango`): strict index selection [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj closed pull request #4710: feat(`mango`): strict index selection
URL: https://github.com/apache/couchdb/pull/4710
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660125334
> `/$dbname/_design/$ddoc/_find/$indexname`
sadly, we already have `use_index` as the in-JSON way to specify the index. afaict, this PR just makes it mandatory.
I have qualms with adding the URL-way on top of this and later possibly deprecating `use_index`, but this does not have to hold this particular PR for me.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735426001
@rnewson, @ricellis I like the proposed ideas and I am glad that we could utilize the original proposal but in a better way. Thanks for helping us with getting to this solution!
@big-r81 we need the backwards compatibility. Personally, I see at least two reasons for that:
- It is like that for about 7 years now, but there might be integrations that rely on this (good or bad, it does not matter) behavior. CouchDB already offers support for multi-version cluster upgrades for changes like that, that is basically mandatory.
- This is not a bug but a feature. The `use_index` parameter was introduced in 1b0426aa to force the index selection to a specific index. Unfortunately, this did not work out well in practice, so 743bd882 added a fallback mechanism to make it less brittle.
I do not have plans to remove the fallback at the moment. I do not see it an obvious bad thing but at the same time I understand that this has many disadvantages that may confuse or surprise the users. Actually, more "noise" around this in the documentation and in the configuration toggles could help to raise the awareness. I do not see that much associated extra work either (it is a small change if you check the current version), but I am willing to take on those, if needed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1737448106
@willholley what cluster-wide toggle? I'm talking about `allow_fallback: true|false` in the `_find` request body. Opt-in. And we had (momentarily, it seems) reached consensus on _not_ changing the default behaviour of `use_index`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] ricellis commented on pull request #4710: feat(`mango`): strict index selection
Posted by "ricellis (via GitHub)" <gi...@apache.org>.
ricellis commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735117227
I like this `allow_fallback` idea. I think it is a good compromise to allow `use_index` to behave like everyone expects without breaking backwards compatibility. Having purpose as an option beyond `use_index` cases makes it is more useful than adding a new toggle that only impacts `use_index`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] ricellis commented on pull request #4710: feat(`mango`): strict index selection
Posted by "ricellis (via GitHub)" <gi...@apache.org>.
ricellis commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731539141
>what's your opinion on mango considering an index usable for a selector for $regex operator?
That problem extends beyond `$regex` doesn't it? My undersanding is that (at least for json/view indexes) no (non-partial) index is more useful than `_all_docs` unless the selector has at least one "equality" operator (`$eq`, `$gt`, `$gte`, `$lt`, `$lte`) that can be used to narrow the set of docments to subsequently scan for the rest of the constraints. Or am I missing something particularly extra about regex?
>when a field is indexed but is only in the selector as a $regex. $regex is brute-forced, it doesn't use any indexed field
Does brute-forced in this context mean it uses the keys from the view or does it actually pull the docs?
I think the "covering" work only applied to avoiding fetches where the projected `fields` were already in the key, but potentially a "covering" style improvement to regex where the keys from the index can used to evaluate the regex _before_ fetching docs could make an index on the requested field more useful than an alternative.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1732661208
Submitted #4774 to fix the 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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735070059
another option occured to me. Similar to the initial proposal, we could introduce `allow_fallback: true|false` as a new option, defaulting to `true` to maintain current behaviour. We'd also update the documentation as in https://github.com/apache/couchdb/pull/4774 to clarify that `use_index` is a hint by default.
When `allow_fallback` is false, the behaviour changes as follows;
1) if `use_index` is set, and the index selected is not a match for the selector, we return a `400 Bad Request` instead of falling back to a different index or all_docs.
2) if `use_index is not set, and there is no index that can be used, we return a `400 Bad Request` instead of falling back to all_docs.
If this exists we can encourage users to always set `allow_fallback: false` in `_find` requests, whether they use `use_index` or not, so they can avoid the worst of the fallback behaviour.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1661123039
Agreed, would be good to hear from Rich et al
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660492011
removing `/dbname/_find` is only a possibility (that could only happen in a major version bump). I mention it as part of framing any improvement here. Adding the index-specific`_find` is the couchdb-shaped way to achieve "strict index selection", as that's how it's done for map/reduce and search indexes today. If, in the future, we find that `/dbname/_find` is much less used / actively avoided / a source of confusion not joy, we could deprecate it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] willholley commented on pull request #4710: feat(`mango`): strict index selection
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1661679783
Appreciating the difficulties with augmenting existing endpoints, I think
it's worth considering adding "selector" and "fields" support to the
existing _all_docs, view and search endpoints. This would be consistent
with the _changes and _find endpoints and provides most of the benefits of
a single-index _find query, albeit with a more explicit API.
It would also enable the filtering functionality for existing view/search
indexes whereas Mango makes assumptions about the index key structure that
hand-crafted indexes would need to replicate if _find we're to be supported
for them.
On Tue, 1 Aug 2023, 23:26 Robert Newson, ***@***.***> wrote:
> Agreed, would be good to hear from Rich et al
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/couchdb/pull/4710#issuecomment-1661123039>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAX365O3ZXNF4RRRIJZHXLXTFYBHANCNFSM6AAAAAA25IW4QM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on a diff in pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on code in PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#discussion_r1280488371
##########
src/docs/src/api/database/find.rst:
##########
@@ -1505,14 +1494,32 @@ it easier to take advantage of future improvements to query planning
"year",
"title"
],
- "partitioned": false
+ "mrargs": {
+ "include_docs": true,
+ "view_type": "map",
+ "reduce": false,
+ "partition": null,
+ "start_key": [
+ 2010
+ ],
+ "end_key": [
+ "<MAX>"
+ ],
+ "direction": "fwd",
+ "stable": false,
+ "update": true,
+ "conflicts": "undefined"
+ },
+ "covered": false
Review Comment:
the sort order changes here make this hard to review. If there were no semantic changes, I’d love to see that in a separate commit.
##########
src/docs/src/api/database/find.rst:
##########
@@ -1505,14 +1494,32 @@ it easier to take advantage of future improvements to query planning
"year",
"title"
],
- "partitioned": false
+ "mrargs": {
+ "include_docs": true,
+ "view_type": "map",
+ "reduce": false,
+ "partition": null,
+ "start_key": [
+ 2010
+ ],
+ "end_key": [
+ "<MAX>"
+ ],
+ "direction": "fwd",
+ "stable": false,
+ "update": true,
+ "conflicts": "undefined"
+ },
+ "covered": false
Review Comment:
the sort order changes here make this hard to review. If there were no semantic changes, I’d love to see that in a separate commit.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] mikerhodes commented on pull request #4710: feat(`mango`): strict index selection
Posted by "mikerhodes (via GitHub)" <gi...@apache.org>.
mikerhodes commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1727307260
> adding a note that for the new endpoints an index should not match a selector if it only has indexed fields where the selector does $regex or any other operator where neither startkey or endkey are changed from the defaults that cover the entire view.
I disagree that we should implement this restriction. The issue the user reported is "`use_index` doesn't do what it says". It wasn't "I don't like that the database does inefficient queries". Even if it was, I'd still disagree.
I do agree that a good way to solve this is to introduce the URL framing for selecting an index. But I think the only reason we'd return a 400 would be that the index in the URL would be disqualified using the standard Mango index selection logic for the given selector (eg, selector doesn't contain the fields in an "indexable" way).
Specifically, I don't think the user should be prevented from passing in a selector that passes the index selection logic, but passes values which mean the entire view is read.
I think this because:
1. I think this is confusing for a user --- both views and search allow this, so why have this restriction only for Mango?
2. I don't think "because the user can shoot themselves in the foot" is a good reason either, because it's not what I'd want for CouchDB to do on my behalf. Databases provide many, many ways to shoot oneself in the foot. And they should do, that's why they are powerful.
In summary, I agree the URL method is a good solution to this problem, which is that `use_index` is bad because it doesn't do what it says. I'd like to get use_index deprecated and a replacement into the codebase, and avoid trying to "fix" Mango in other ways at the same time.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on pull request #4710: feat(`mango`): strict index selection
Posted by "big-r81 (via GitHub)" <gi...@apache.org>.
big-r81 commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731483120
@rnewson Mhhh, then we should not output a result if a user specifies an index that does not exist or cannot be used. Then we should just issue a warning as a result and specify "Use the correct index or do not use `use_index` at all". Then the user can decide what to do. Of course, this would not be backward compatible behavior either.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(`mango`): strict index selection [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1747298830
Please note that #4792 has been opened as an alternative proposal in response to the comments in this conversation.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(`mango`): strict index selection [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-2422977093
I am closing this in favor of #4792.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on pull request #4710: feat(`mango`): strict index selection
Posted by "big-r81 (via GitHub)" <gi...@apache.org>.
big-r81 commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735273578
Do we really need backwards compatability here?
Let me give you a practical example:
There is a road with allowed max. speed of 50 km/h and a digital display board which shows your current speed and a 😄 if you drive less than or equal to 50 km/h and 😞 if you drive too fast. You drive this way for years and you drive always to fast because there are obviously no dangers. One day I got a speeding ticket because I was going too fast (like always)...
1. Should I declare to the fine office that I have always driven too fast up to now? (Who do you think will be proven right?) or
2. Should I just pay the ticket and drive only 50 km/h on this route from now on?
In this case - I think - we should fix `use_index` to use that index or fail if the index is not available / usable. No additional config setting. If we would introduce an additional config option for every functional change, we would
1. increase the complexity of the docs and
2. the complexity of the software in general and
3. (maybe) introduce additional dependencies between different settings and
4. is there a day when we would remove all deprecated and additional bc compatible settings?
So, just my 2 cents...
But, if we go the BC route, than `allow_fallback` is an elegant solution.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731612261
@pgj imo we either make `use_index` do what it says or we introduce the path-based `_find` endpoints that work like `_view` and `_search` in guaranteeing the request is served by that index (with the unique caveat that it's a `400` if the selector isn't covered by the index). I don't want to go the more confusing route of adding yet more parameters that mean 'really use the index'.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #4710: feat(`mango`): strict index selection
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731505119
Good point, @ricellis. This looks more like a bug fix than adding a new feature. Documentation agrees too.
Not sure about `$regex` maybe that falls into a separate category of `returning efficient results`. The index itself could look silly and just index everything equivalently to an `_all_docs` anyway, even without a `$regex` involved.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660274182
I see. But we should keep `use_index` around as it is now for a while for backward compatibility, 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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on a diff in pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on code in PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#discussion_r1280492718
##########
src/docs/src/api/database/find.rst:
##########
@@ -1505,14 +1494,32 @@ it easier to take advantage of future improvements to query planning
"year",
"title"
],
- "partitioned": false
+ "mrargs": {
+ "include_docs": true,
+ "view_type": "map",
+ "reduce": false,
+ "partition": null,
+ "start_key": [
+ 2010
+ ],
+ "end_key": [
+ "<MAX>"
+ ],
+ "direction": "fwd",
+ "stable": false,
+ "update": true,
+ "conflicts": "undefined"
+ },
+ "covered": false
Review Comment:
I added the `use_index_strict` field to the `_explain` output. But I can factor out the part about the refactoring and create a separate PR about it even.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660311391
@janl I don't see how this is a step towards "making the Mango-query-index-selection API look similar to JS views and search APIs". my suggestion of adding `/path/to/specific/index/_find` does that, but expanding `/dbname/_find` does not. Can you explain the step you see?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on pull request #4710: feat(`mango`): strict index selection
Posted by "iilyak (via GitHub)" <gi...@apache.org>.
iilyak commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660968293
I agree with Nick that we would not want to put ourselves in the corner by embedding assumption about single index into endpoint name.
I also agree with Bob and Jan that adding more option to existing endpoint is not the best choice in this particular case. Specifically because the option would be off by default for compatibility reasons. Also adding new options to existing API endpoints complicates maintaining of client libraries.
I think we should come up with a new endpoint name but shouldn't embed index name into it.
I am bad at naming and naming is hard.
`/{dbname}/_find_indexed`
`/{dbname}/_select` - // inspired by SQL
`/{dbname}/_find_select` - // find using selection of indexes
Let's ask help from @ricellis who has an extensive experience in designing APIs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on pull request #4710: feat(`mango`): strict index selection
Posted by "big-r81 (via GitHub)" <gi...@apache.org>.
big-r81 commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731457338
> You can tell I am tired of dealing with users who have been let down by mango and its false promises.
This is completely understandable and comprehensible and yes, you always answer the same questions and misunderstandings ;-). Maybe this is also one of the reasons that I have not used Mango much.
I think we shouldn't mix different things here though, nor personal inclinations / dislikes to various functions.
I think we can and should improve the current behavior as best as possible (in the sense of the user and operator) (and best of all, without introducing an additional configuration parameter).
If we want to preserve a backward-comatable behavior, can't we explicitly tell the user in the result if an index was used or not (more visible than just in _explain) and without interpreting the "execution statistics"?
Now I would also have no problem with marking this as backwards incompatible in the changlog! If we follow the documentation, then we just correct the behavior (which was wrong until now).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] willholley commented on pull request #4710: feat(`mango`): strict index selection
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731563627
`$regex` gets converted into an `$exists: true` predicate for index selection, so an index can be used to optimize it in some cases, though it's still going to be a range scan on whatever index is selected. I don't think there's a bug there, though perhaps the performance could be improved as per @ricellis's suggestion.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #4710: feat(`mango`): strict index selection
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660510003
More advanced query engines allow selecting _multiple_ indices to answer a query. It's not impossible, for Mango to be improved to do it as well. For instance, one pain point of Mango is it can't use indices for general disjunctions today; but it might be possible to do that by doing a merge on two indices. If the new Mango query API is to specify only one index, that API choice will work against that improvement.
Another, more pessimistic view, I guess is to say Mango is never going to be anything but a layer on top of an mrview index and we shouldn't pretend otherwise. Then selecting an index and forcing that there be an index defined is probably reasonable.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1687834953
perhaps it's more general, the goal being that query execution through these new options is always efficient (at least relative to reading the entire database or index). This is an opportunity to reject things that we previously had to accept.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1687830558
Okay, thanks for the note @rnewson.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] willholley commented on pull request #4710: feat(`mango`): strict index selection
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1715940637
> Rather than an additional request body parameter to /db/_find, perhaps we could add the index to the url path, but still end in _find? The request and response body stay in 'find' format (barring any fields that might be obsolete by the fact the index is known already).
This could work, though only for indexes created via the `_index` endpoint given we need the index definition in a format that allows `_find` to reason with the schema. For JS/other indexes we could return a `400` status code as we would for an invalid index.
> In this formulation I think 'execution stats' might not be emitted, since the number of rows returned would always match the number examined (as they've always done with _view).
I don't think this is true unless you're proposing to limit `_find` to start_key/end_key derivation only, which would represent a large step down in functionality from the database-level `_find`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660300445
the behaviour of `use_index` can't easily be changed (or, I think we'd all agree, _fixed_), short of a major version bump.
If there was a roadmap here for couchdb 4.0 (handwaving, but imagine that use_index as an option goes away entirely. you either use `/dbname/_find` and let couchdb choose, or you use `/path/to/specific/index/_find` to choose for yourself).
Without that, it's hard to decide whether adding more options is taking us somewhere better or just somewhere more confusing. As a user I would be legitimately baffled to be told I should use `use_index_strict` when I report that my `use_index` directive is being ignored.
I'm not sure if I'm -0 or -1 on this, I'm veering to -0 though. I don't like leaving `use_index` as-is (it should never have been merely a hint) and I don't like adding another argument to do what `use_index` should have done.
Hence I suggested having `_find` also reachable from a url that clearly is associated with a specific index resource, like we have for all the other kinds of index we have. That endpoint can return a 400 if the request parameters cannot be executed by the index.
That idea also gives users a migration path away from the flawed choosing algorithm. A future (major) release of CouchDB might then drop `/dbname/_find` entirely.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1715713111
@ricellis this is excellent feedback. I agree with your point about the several dimensions of confusion that we could introduce here, and our existing querying and indexing system is already confused. I don't want to make it worse.
It is no secret that I think mango was a mistake in its entirety and this colours my response to any attempt to change it. I dislike it, I think, for objective reasons. It promises much and delivers little, and every user of it that accidentally ends up doing full-database reads on every query because they, quite reasonably, believed they were using a query interface, is a user we have harmed.
It is in that spirit that I objected to the original idea of this PR, to add another request body option to mean "I _really_ mean you must use this index" because `use_index` is only advisory, despite its confident-sounding name.
However, adding selector as an option to `_view` and `_search` has problems, as you've noted.
So, I think we need another way.
I very much dislike the path of `_find` at the db level. The server side is insufficiently powerful today and, quite likely, will be forever, it is inappropriate to suggest that something clever happens on the server side. All that happens is the least-worst index for your selector is chosen and the rest is brute-forced. The diligence required by users to ensure they hit an index is high, and frequently not met.
Rather than an additional request body parameter to `/db/_find`, perhaps we could add the index to the url path, but still end in `_find`? The request and response body stay in 'find' format (barring any fields that might be obsolete by the fact the index is known already).
When called as `/$dbname/_design/$ddoc/_find/$indexname` we would only execute the query if the selector matches the index's indexed fields. a `400 Bad Request` is returned for anything else. No `_all_docs` fallback. Quite what we would decide on middle cases where a result can be served from the chosen index with some post-query filtering I'm not sure. I'd prefer to not allow that either.
In this formulation I think 'execution stats' might not be emitted, since the number of rows returned would always match the number examined (as they've always done with `_view`).
Smaller decisions like either prohibiting `use_index` in the request body or accepting it if it matches (but definitely 400 Bad Request if present and different) also need to be made.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735276846
@big-r81 the difference between your scenario and this one is that you know you are exceeding a limit. The problem with the fallback behaviour of `_find` is that you don't 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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] willholley commented on pull request #4710: feat(`mango`): strict index selection
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1737462585
@rnewson https://github.com/apache/couchdb/pull/4710#issuecomment-1735317072.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1737715766
@willholley aha! I missed it the first time. I agree that those two suggested toggles just complicate things. I suggest it's just the opt-in in the `_find` request body for now. couchdb could flip the default in code in a future major release (but I think it's unlikely).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660248244
I disagree. The mistake when use_index was added, where it was allowed to not use the index requested, is embedded now. Having a "I really mean it" option, that is optional, and off by default, makes this interface worse, not better.
We should either fix the associated issue (prevent use_index from not using any index other than the one requested) or not. An optional fudge is worse.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660314166
noting `use_index` should be prohibited (400 Bad Request) for the `/path/to/index/_find` variant
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660424954
A revisited version of `/{db}/find` may refuse to run unindexed queries, it does not have to be necessarily removed because currently it makes that possible. Or am I missing something here?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660412655
> Why to drop /{db}/find entirely?
it was a mistake to allow unindexed queries. We are trying to reverse that mistake now :)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] ricellis commented on pull request #4710: feat(`mango`): strict index selection
Posted by "ricellis (via GitHub)" <gi...@apache.org>.
ricellis commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1715549181
Apologies for the delay in commenting here, I missed it initially in vacation season.
>Appreciating the difficulties with augmenting existing endpoints, I think
it's worth considering adding "selector" and "fields" support to the
existing _all_docs, view and search endpoints.
I can see the appeal of this suggestion as a neat way to direct mango-style queries to a specific index without the need to add additional endpoints. However, the problem I see is that the response structure for view-like things, searches and mango queries are all a bit different.
I'd be completely opposed to different response structures being returned depending on the request properties. For example IMO a call to `_all_docs` with a `selector` should definitely not return an array of `"docs":[{…},…]` like `_find` does because it does not conform with the normally expected pattern for `_all_docs` repsonses of `"rows":[{…"doc":{…}},…]`.
That kind of response structure toggling makes using endpoints unwieldy and confusing for users and also makes them hard to describe accurately and correctly in tooling and documentation.
To go down that route I'd say the responses would need to be transformed (or maybe it is actually _not_ transformed) from the underlying index response to retain the existing structures used by the endpoints being augmented. That might in turn result in a loss of fidelity for some things and although additional response properties could be added relatively painlessly a proliferation of additional response properties that are only populated under certain request conditions is also not very user friendly (e.g. adding a `bookmark` that is only populated with a `selector` but not on a normal `_all_docs`, or `execution_stats` etc).
Further I think consideration of the impact on the request options is also needed, like how combinations could (or could _appear_ to) operate. For example:
what would it mean to provide a `selector` to a view-like endpoint with `include_docs=false`?
or request `fields=["foo","bar"]&include_docs=true` on a normal view-like request?
The more APIs are dual-purposed and additional options bolted onto existing endpoints the more potential combinations surface that seem to make sense to users, but are invalid in the implementation. Again this leads to APIs that are harder to document and use. It isn't great for usability to have to fathom a giant matrix of option combinations from the manual. Sure validation can be added, but even then potentially making multiple requests getting `400` responses before landing on a valid combination isn't a good experience.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731464823
@ricellis I agree that `use_index` should mean what it says and we should fix that.
@big-r81 how do you you propose to "explicitly tell the user in the result if an index was used"? We already send a warning in every `_find` response if it was executed inefficiently, and absolutely no one notices this. How could they? They get their results, though slower than if they had used a better index (and, for small datasets, not even that. The problem only appears when they add more data).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731669017
Curiously, `use_index` has always been considered a hint in the sources, see the comments in `mango_cursor` where the warnings are generated:
https://github.com/apache/couchdb/blob/435db51a4f412a66f3cf3deffbc84b9da522a3d6/src/mango/src/mango_cursor.erl#L475-L509
Is this a bug in the _documentation_ itself?
https://github.com/apache/couchdb/blob/83e39b6d14c500e9a1978287de3058465db5c1ec/src/docs/src/api/database/find.rst?plain=1#L45-L47
The use of "instruct" there might be a poor choice which causes so much pain and misunderstanding. Personally, I can see the intention behind the wording but I can also understand that it is misleading at the first sight. (Being instructed does not necessarily warrant that one will obey. But I am not either a native speaker or a linguist.)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] ricellis commented on pull request #4710: feat(`mango`): strict index selection
Posted by "ricellis (via GitHub)" <gi...@apache.org>.
ricellis commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731431936
Controversial I'm sure, but can we go back to basics here?
Does anyone actually think `use_index` should be a hint?
The [documentation](https://docs.couchdb.org/en/stable/api/database/find.html) says:
> Instruct a query to use a specific index.
That doesn't even seem that ambiguous, given computers follow instructions.
Honestly it just seems like a long standing bug to me. Every user I've interacted with who has tried to use this option (including past me) does not think this instruction is optional for the server. I can't think of a good reason why it would ever be a "hint". If you want auto-selection, don't say `use_index` if you want it to use a specific index then specify `use_index`. 🤷
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731470877
@ricellis separately, what's your opinion on mango considering an index usable for a selector for $regex operator? I consider that a bug also. The index _isn't_ usable for that query, it's no better than `all_docs` unless it was a partial index.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on a diff in pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on code in PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#discussion_r1280597853
##########
src/docs/src/api/database/find.rst:
##########
@@ -1505,14 +1494,32 @@ it easier to take advantage of future improvements to query planning
"year",
"title"
],
- "partitioned": false
+ "mrargs": {
+ "include_docs": true,
+ "view_type": "map",
+ "reduce": false,
+ "partition": null,
+ "start_key": [
+ 2010
+ ],
+ "end_key": [
+ "<MAX>"
+ ],
+ "direction": "fwd",
+ "stable": false,
+ "update": true,
+ "conflicts": "undefined"
+ },
+ "covered": false
Review Comment:
Moved the unrelated documentation changes to #4713.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660310854
In general, for API migrations we loosely agreed on: introduce a better API, eventually retire the old one, maybe.
In that light, adding the REST-URL-selects-index route is adding the better API and we can later remove raw `_find` when we think the time is right.
That is indeed cleaner than adding another toggle to change the behaviour of `use_index`.
I have no problems with that approach.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660402380
> That idea also gives users a migration path away from the flawed choosing algorithm. A future (major) release of CouchDB might then drop `/dbname/_find` entirely.
Why to drop `/{db}/find` entirely? Why not to fix the index selection algorithm instead? I understand that it might be a nuisance for the users that a less optimal index is selected, but that is why I started to work on #4662 for example.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1661765225
oooh I like that.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660283983
> I see. But we should keep use_index around as it is now for a while for backward compatibility, right?
Yes.
> An optional fudge is worse.
I wouldn't frame it these strict terms. I see this PR as a step towards making the Mango-query-index-selection API look similar to JS views and search APIs. IF this PR doesn't get us the whole step there, that’s fine by me as long as we arrive there eventually. IF we want to fix this all in one go, that’s fine by me, but for BC reasons we will need a toggle of some sort for a while and time for folks to migrate off of the no longer preferred APIs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660133410
I agree with @janl. I would be happy to work on making the Mango interface to be consistent with that of Search (I like the idea), but not in the scope of this PR.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660982289
The stated purpose of this PR is to allow a user to run a selector against a specific individual index _or_ receive an error stating that the index is not compatible, rather than fallback to a less efficient index.
Let us focus on the best way to achieve that specific goal, and whether to then do so.
Speculation about the future of `/dbname/_find` has muddied the waters. I am sorry for my part in that.
For map/reduce views and search indexes (either dreyfus/clouseau or nouveau) this is done having the path to the index in the url.
Inventing a third mechanism (as Ilya proposes) for this is not acceptable to me; our API is already complex enough.
If we want a facility to run a selector against a user-selected index, then I only see one way: `/db/_design/design-doc/_find/index-name`.
If we don't want that, then we need do nothing.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660474939
I am not sure I got you. In my understanding, `/{db}/find` provides the possibility of letting CouchDB choose the index for the query. Removing this endpoint would lose this feature. It is hard for me to compare this with explicitly naming the index in the URL — there is no "better" or "worse" API in that sense, I believe. These are two different answers to the same problem: manual vs. automated index selection.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] janl commented on pull request #4710: feat(`mango`): strict index selection
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1660439181
sure, but why leave it if we have a better API in place?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1727221556
adding a note that for the new endpoints an index should _not_ match a selector if it only has indexed fields where the selector does `$regex` or any other operator where neither startkey or endkey are changed from the defaults that cover the entire view.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1727514495
@mikerhodes I'm not sure we're disagreeing. The example I cite is when a field is indexed but is only in the selector as a $regex. $regex is brute-forced, it doesn't use any indexed field. aka, the field used for a $regex should not count, since whether it is indexed or not is irrelevant to evaluation.
whatever the formula currently is for `/db/_find` should remain untouched, sure, but we should take the opportunity to remove foot guns where we can.
`_find`_ is _not_ powerful. It just pretends to be.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1731981572
Instruct
> to [order](https://dictionary.cambridge.org/dictionary/english/order) or [tell](https://dictionary.cambridge.org/dictionary/english/tell) someone to do something, [especially](https://dictionary.cambridge.org/dictionary/english/especially) in a [formal](https://dictionary.cambridge.org/dictionary/english/formal) way:
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on pull request #4710: feat(`mango`): strict index selection
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1687827392
one further note here. when, in the future, you pass a selector to `_view` or _`search` we should send a `400 Bad Request` if mango would do a startkey/endkey that covers the entire index (for `_view` that's when it does `"start_key": [], "end_key": ["<MAX>"],`.)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] pgj commented on pull request #4710: feat(`mango`): strict index selection
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1734105026
Created #4775 for the problem with the `$regex` operator.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] ricellis commented on pull request #4710: feat(`mango`): strict index selection
Posted by "ricellis (via GitHub)" <gi...@apache.org>.
ricellis commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1735317072
Perhaps some administrative config for both:
* the default setting of `allow_fallback`
* and whether to honor `allow_fallback` from requests
would provide enough control to allow admins to enforce no fallbacks when they have no compatibility concerns. That would also give a mechanism of flipping the default config value on the major version boundary.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] willholley commented on pull request #4710: feat(`mango`): strict index selection
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #4710:
URL: https://github.com/apache/couchdb/pull/4710#issuecomment-1737168382
I agree with changing the behavior of `use_index`, but I'm less keen on overloading it to removing the use of the `_all_docs` index. There are legitimate reasons to use `_all_docs` and it would be a breaking change for some clients (thinking particularly things like Loopback which fetch docs by `_id` using `_find`). Making that breaking change a cluster-wide toggle then makes portability between CouchDB deployments difficult / unpredictable.
It may also be useful context that Mango did originally prevent the `_all_docs` fallback unless the query contained a clause that would require it. What we saw, certainly in Cloudant, was that lots of users just blindly added a `{ _id: { "$gt": 0 } }` clause to their selectors to work around the restriction. I think if we want to limit the overhead of index / db scans (accidental or otherwise), there are better options available that would be less breaking (e.g. [check the magnitude of the scan before executing the query](https://github.com/apache/couchdb/blob/b6ea325ccb1e3de45f8541ca4fde225ac75f7d54/src/mango/src/mango_cursor_view.erl#L305-L307)).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org