Return positions of parse errors found in JSON #10837
Conversation
…d only primitives and changed all of the parsers that throw QueryParsingExceptions which now mandates the passing of the error location info.
… all parsers that throw this exception type
|
|
||
| package org.elasticsearch.common.xcontent; | ||
|
|
||
| public class XContentLocation { |
There was a problem hiding this comment.
can we have some javadocs here and please make the class an all members final. I wonder if we should not even have getters here and make it a very simple struct
|
left some minor comments otherwise LGTM |
|
LGTM |
|
if we are changing all the places that call |
lets do that in a sep PR? |
sure (my thought was if we already change all this files now) |
|
If there's no objections I'll revise this PR with @kimchy 's suggestion |
|
ok sure go ahead @markharwood |
…r. This change simplified the parser classes that throw exceptions but ElasticsearchExceptionTests and BytesRestResonse tests needed tweaking to work around the lack of QueryParseContext available in unit tests. I had to introduce a subclass of QueryParsingException which doesn’t require QueryParseContext to work but can still report on line/column number errors.
|
The change to pass QueryParseContext to QueryParsingException made all parser code cleaner. |
| * This constructor is provided for use in unit tests where a | ||
| * {@link QueryParseContext} may not be available | ||
| */ | ||
| protected QueryParsingException(Index index, int line, int col, String msg, Throwable cause) { |
There was a problem hiding this comment.
does it need to be protected? or just package private?
|
I had a quick look and I really like it, having the |
…kage as base class but in src/test
|
Thanks, @javanna - I moved the test exception out into same package in test framework and changed visibilities as per your comments |
|
LGTM |
|
LGTM too |
|
Pushed to master 528f648 |
clintongormley
changed the title
clintongormley
added
:Search/Search
Extend SearchParseException and QueryParsingException to report position information in query JSON where errors were found. All query DSL parser classes that throw these exception types now pass the underlying position information (line and column number) at the point the error was found.
Closes #3303