-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
"did you mean" for ObjectParser with top named #51018
"did you mean" for ObjectParser with top named #51018
Conversation
When you declare an ObjectParser with top level named objects like we do with `significant_terms` we didn't support "did you mean". This fixes that. Relates elastic#50938
} | ||
if (false == entry.name.match(name, parser.getDeprecationHandler())) { | ||
/* Note that this shouldn't happen because we already looked up the entry using the names but we need to call `match` anyway | ||
* because it is responsible for logging deprecation warnings. */ | ||
throw new NamedObjectNotFoundException(parser.getTokenLocation(), | ||
throw new XContentParseException(parser.getTokenLocation(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the exceptions that I changed to XContentParseException
are really more development errors than usage errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And they don't have the critical candidates
set.
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Looking at other places that NamedObjectNotFoundException is used in the code, does this mean that we can rewrite AbstractQueryBuilder.parseInnerQueryBuilder
to use an ObjectParser and get 'did you mean' suggestions for misspelt queries?
I believe so. |
I think we'd more likely want to port the |
When you declare an ObjectParser with top level named objects like we do with `significant_terms` we didn't support "did you mean". This fixes that. Relates elastic#50938
When you declare an ObjectParser with top level named objects like we do with `significant_terms` we didn't support "did you mean". This fixes that. Relates elastic#50938
When you declare an ObjectParser with top level named objects like we do
with
significant_terms
we didn't support "did you mean". This fixesthat.
Relates #50938