Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Term positions in analyze API should start at 1, not 0 #10771

Closed

Conversation

Projects
None yet
4 participants
@johtani
Copy link
Member

commented Apr 24, 2015

Now, Analyze API return 1 as 1st token.
Analyze API should return 0 as 1st token, looks like TermVector.

@dakrone

This comment has been minimized.

Copy link
Member

commented Apr 24, 2015

LGTM

@clintongormley

This comment has been minimized.

Copy link
Member

commented Apr 26, 2015

@johtani Let's make this change only master - it's a breaking change.

@johtani

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2015

@clintongormley I see.
I add comment to breaking changes 2.0 doc.

int increment = posIncr.getPositionIncrement();
if (increment > 0) {
position = position + increment;
}
tokens.add(new AnalyzeResponse.AnalyzeToken(term.toString(), position, offset.startOffset(), offset.endOffset(), type.type()));

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 28, 2015

Contributor

This doesn't look correct to me: of course would be analyzer as a single token course at position 0 while it should be at position 1.

I think the right fix is to put back tokens.add where it was before and instead to initialize position at -1.

This comment has been minimized.

Copy link
@johtani

johtani Apr 28, 2015

Author Member

Good catch, thanks!

@johtani

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2015

@jpountz Fix your comment and add the test .

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2015

LGTM. Thanks @johtani !

@johtani johtani force-pushed the johtani:fix/wrong_position_returned_analyze_api branch 2 times, most recently from f8efba7 to ca3d941 Apr 28, 2015

@johtani johtani removed the review label Apr 28, 2015

@johtani johtani closed this Apr 28, 2015

@johtani johtani force-pushed the johtani:fix/wrong_position_returned_analyze_api branch from ca3d941 to 933edf7 Apr 28, 2015

@clintongormley clintongormley changed the title Analysis: Fix wrong position number by analyze API Term positions in analyze API should start at 1, not 0 Jun 6, 2015

@johtani johtani deleted the johtani:fix/wrong_position_returned_analyze_api branch Apr 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.