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

Throw exception when multiple field names are provided as part of query short syntax #19871

Merged
merged 12 commits into from
Aug 9, 2016

Conversation

javanna
Copy link
Member

@javanna javanna commented Aug 8, 2016

With #19791 we made sure that our query parsers throw error whenever they encounter multiple field names if only one is required. This PR covers the same situation for queries short syntax.

@javanna
Copy link
Member Author

javanna commented Aug 8, 2016

@tlrx this should address your comment from #19791 (comment) . Can you have a look please?

@@ -309,6 +309,10 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
}
}
} else {
if (fieldName != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be a method in AbstractQueryBuilder? Something like throwParseExceptionOnMultipleFields(String queryName, String fieldName, String currentName)?

@tlrx
Copy link
Member

tlrx commented Aug 9, 2016

Left a comment, thanks for doing this @javanna !

javanna added 4 commits August 9, 2016 09:50
…provided in a query that support one field only

This makes sure that error messages are unified, and makes us save a few lines of code too.
@javanna
Copy link
Member Author

javanna commented Aug 9, 2016

@tlrx I pushed some new commits that should address your comment

@tlrx
Copy link
Member

tlrx commented Aug 9, 2016

LGTM, thanks!

@javanna javanna merged commit af5fbcd into elastic:master Aug 9, 2016
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants