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

SearchRequestBuilder.setHighlighterBoundaryChars doesn't work as expected #15792

Closed
julamb opened this issue Jan 6, 2016 · 5 comments
Closed

SearchRequestBuilder.setHighlighterBoundaryChars doesn't work as expected #15792

julamb opened this issue Jan 6, 2016 · 5 comments
Assignees

Comments

@julamb
Copy link

@julamb julamb commented Jan 6, 2016

Using the java API, I'm trying to use setHighlighterBoundaryChars to change the boundary chars of the fast vector highlighter, but it seems that it is broken.

...
char[] chars = new char[]{'\n', '.'};
request.setHighlighterBoundaryChars(chars);
System.out.println(request.toString());
...

prints

...
  "highlight" : {
    "boundary_max_scan" : 200,
    "boundary_chars" : "[C@550ec6a1",
...

According to the doc, the "boundary_chars" field should be a string containing all the boundary chars.

Possible fix

Looking at the code of HighlightBuilder.java:368 in 2.2:

// Current
builder.field("boundary_chars", boundaryChars); // boundaryChars is a char array

// Fixed
builder.field("boundary_chars", new String(boundaryChars)); // might work better
@nik9000

This comment has been minimized.

Copy link
Contributor

@nik9000 nik9000 commented Jan 6, 2016

Confirmed the bug. It looks like your fix will work too. This is not a problem in master though. I'll open up a PR to fix it once the test suite finishes running.

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Jan 6, 2016
@julamb

This comment has been minimized.

Copy link
Author

@julamb julamb commented Jan 6, 2016

Cool! Actually I just noticed that there's another use of the boundaryChars char array at HighlightBuilder.java:430.

@nik9000

This comment has been minimized.

Copy link
Contributor

@nik9000 nik9000 commented Jan 6, 2016

Cool! Actually I just noticed that there's another use of the boundaryChars char array at HighlightBuilder.java:430.

Good catch. I'll fix that one too.

@nik9000

This comment has been minimized.

Copy link
Contributor

@nik9000 nik9000 commented Jan 6, 2016

Good catch. I'll fix that one too.

Done and waiting in the PR.

@clintongormley

This comment has been minimized.

Copy link
Member

@clintongormley clintongormley commented Jan 10, 2016

Fixed by #15795

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