Skip to content

Conversation

@tonysun83
Copy link
Contributor

@tonysun83 tonysun83 commented Aug 31, 2020

Overview

Previously, we passed in the unpacked version of the bookmark with
the cursor inside the options field. This worked fine for _find because
we didn't need to return it to the user. But for _explain, we return
the value back as unpacked tuple instead of a string and jiffy:encode/1
complains. Now we correctly extract the bookmark out of options, unpack
it, and then pass it separately in it's own field. This way options
retains it's original string form for the user so that invalid_ejson
is not thrown.

Testing recommendations

Regression tests should pass. If you create a text index, simply use _explain with a bookmark and verify the results.

Related Issues or Pull Requests

Checklist

Previously, we passed in the unpacked version of the bookmark with
the cursor inside the options field. This worked fine for _find because
we didn't need to return it to the user. But for _explain, we return
the value back as unpacked tuple instead of a string and jiffy:encode/1
complains. Now we correctly extract the bookmark out of options, unpack
it, and then pass it separately in it's own field. This way options
retains it's original string form for the user so that invalid_ejson
is not thrown.
@tonysun83 tonysun83 force-pushed the fix-explain-text-indexes branch from 49ade88 to c14569c Compare August 31, 2020 17:57
@tonysun83
Copy link
Contributor Author

text index tests also pass with full regression

Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@tonysun83 tonysun83 merged commit 0c3c4b6 into master Aug 31, 2020
@tonysun83 tonysun83 deleted the fix-explain-text-indexes branch August 31, 2020 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants