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 2026/01/25 20:44:51 UTC
[PR] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
pgj opened a new pull request, #5865:
URL: https://github.com/apache/couchdb/pull/5865
In the Explain output, bookmarks for queries on `text` (Search) indexes are added in their unpacked format to the options. This leads to at least two problems:
- it is inconsistent with how empty bookmarks are rendered for `json` indexes, where the string `"nil"` is used,
- for non-trivial bookmarks, contents cannot be translated by `jiffy:encode/1` to a JSON representation so that it will crash.
Mitigate this issue by capturing these unpacked bookmarks in the query options and pack them before sending them for building the JSON response.
Thanks @mojito317 for reporting this issue!
## Testing recommendations
The change comes with tests that may be run as follows.
```
make elixir EXUNIT_OPTS="test/elixir/test/mango/02_basic_find_test.exs test/elixir/test/partition_mango_test.exs"
```
and
```
make elixir-search EXUNIT_OPTS="test/elixir/test/mango/06_basic_text_test.exs test/elixir/test/partition_mango_test.exs"
```
Otherwise just try to start a `_find` query on a `text` index, grab the bookmark, and feed it into an `_explain` request with the same query. It should not crash any more. When running a `_explain` query on a `text` with no bookmark provided, the `opts` field in the body should show `nil` instead of an empty array as it did previously.
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3805343697
> mango won't choose a search index if search is down
Yes, I also noticed this one. I am inclined to believe that it is a regression from the Elixir conversion (hence I am cc'ing @janl here) because the Python tests bailed out _before_ any Search-based Mango tests could be run while the Elixir tests do not seem to care about that.
A mitigation of the weird selection logic might be utilizing the `allow_fallback=false` configuration where both `_find` and `_explain` would be coerced to use that single `text` 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
Re: [PR] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3804969678
> _explain works for me without this patch
Hrm, so may it be that simply `dreyfus_bookmark:unpack/1` is not called on some of the branches?
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3804950800
aha, cloudant's dev/run wipes out my data, resetting my environment and removing my indexes. how very naughty.
more review comments after re-testing
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3805304592
It seems to me that we unpack the provided bookmark too early, in the `create` function. It is then unpacked when `explain` returns. The patch below moves the `unpack` logic to the `execute` function.
```
diff --git i/src/mango/src/mango_cursor_text.erl w/src/mango/src/mango_cursor_text.erl
index 0fc92092d..a1cc018cc 100644
--- i/src/mango/src/mango_cursor_text.erl
+++ w/src/mango/src/mango_cursor_text.erl
@@ -38,7 +38,7 @@
documents_seen
}).
-create(Db, {Indexes, Trace}, Selector, Opts0) ->
+create(Db, {Indexes, Trace}, Selector, Opts) ->
Index =
case Indexes of
[Index0] ->
@@ -48,7 +48,6 @@ create(Db, {Indexes, Trace}, Selector, Opts0) ->
end,
DbName = couch_db:name(Db),
- Opts = unpack_bookmark(DbName, Opts0),
Stats = mango_execution_stats:stats_init(DbName),
DreyfusLimit = get_dreyfus_limit(),
@@ -77,7 +76,8 @@ explain(Cursor) ->
[
{query, mango_selector_text:convert(Selector)},
{partition, get_partition(Opts, null)},
- {sort, sort_query(Opts, Selector)}
+ {sort, sort_query(Opts, Selector)},
+ {bookmark, dreyfus_bookmark:pack(get_bookmark(Opts))}
].
execute(Cursor, UserFun, UserAcc) ->
@@ -87,9 +87,12 @@ execute(Cursor, UserFun, UserAcc) ->
limit = Limit,
skip = Skip,
selector = Selector,
- opts = Opts,
+ opts = Opts0,
execution_stats = Stats
} = Cursor,
+
+ DbName = couch_db:name(Db),
+ Opts = unpack_bookmark(DbName, Opts0),
Query = mango_selector_text:convert(Selector),
QueryArgs = #index_query_args{
q = Query,
```
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3806302287
I'm using the `--with-clouseau --clouseau-dir=../clouseau` so, if necessary, I can modify clouseau.
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3805324562
> we unpack the provided bookmark too early
Excellent. I was suspecting that there may be other places where the missing conversion can be injected, but I was not sure. Thanks for checking.
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3804933092
_explain works for me without this patch;
```
"opts": {
"use_index": [],
"bookmark": "g2wAAAABaANkAA9ub2RlMUAxMjcuMC4wLjFsAAAAAmEAYn____9qaAJGP9OjegAAAABhAGo",
"limit": 25,
"skip": 0,
"sort": {},
"fields": [],
```
I _do_ get this sometimes;
```
"error": "invalid_ejson",
"reason": "{{shard,<<\"shards/00000000-7fffffff/db1.1769515564\">>,'node1@127.0.0.1',\n <<\"db1\">>,\n [0,2147483647],\n undefined,[]},\n {0.3068528175354004,0}}",
"ref": 1564355139
```
which is a different bug. attempting to jiffy:encode a shard record.
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3805310636
(`_explain` worked for me _briefly_ with dev/run because mango won't choose a search index if search is down, and it now takes longer to start clouseau, so there's a 15-20 second window after dev/run where the query falls back to `_all_docs`).
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3804934719
also the fixup code, if any is needed, should be in mango_cursor_text module, not the generic mango_cursor module.
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3805519286
> it now takes longer to start clouseau, so there's a 15-20 second window after dev/run
I did not have problems with that. For the reference, once `dev/run` starts up, the `search` feature is immediately reported.
```
% dev/run -a adm:pass -n 1 --with-clouseau --no-eval "curl -s http://127.0.0.1:15984/ | jq -r '.features'"
[ * ] Setup log dir: /Users/gaborpali/projects/github/couchdb/dev/logs ... ok
[ * ] Ensure CouchDB is built ... ok
[ * ] Ensure Erlang boot script exists ... ok
[ * ] Prepare configuration files ... ok
[ * ] Start node node1 ... ok
[ * ] Start Clouseau node clouseau1 ... ok
[ * ] Check node at http://127.0.0.1:15984/ ... ok
[ * ] Check Clouseau node clouseau1 ... ok
[ * ] Running cluster setup ... ok
[ * ] Exec command curl -s http://127.0.0.1:15984/ | jq -r '.features' ... [
"search",
"quickjs",
"access-ready",
"partitioned",
"pluggable-storage-engines",
"reshard",
"scheduler"
]
```
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on PR #5865:
URL: https://github.com/apache/couchdb/pull/5865#issuecomment-3820380256
> I'm using the --with-clouseau --clouseau-dir=../clouseau so, if necessary, I can modify clouseau.
Well, if Clouseau is run from source (basically) then it could indeed take more time because SBT will be called instead of running the JAR file immediately. That is the same for Clouseau 2.x too, but with Maven.
--
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] fix(`mango`): formatting of `text` bookmarks in `_explain` output [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson merged PR #5865:
URL: https://github.com/apache/couchdb/pull/5865
--
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