-
Notifications
You must be signed in to change notification settings - Fork 25.2k
feature/querystringquery-nestedqueries #11339
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
feature/querystringquery-nestedqueries #11339
Conversation
I have signed the CLA but it is not appearing here yet. Just a heads up |
Hi @tuespetre Thanks for the PR. I marked your issue as "discuss" yesterday because I'm not at all sure that this is something we want to do. Personally I don't like the idea of further overloading the already fragile query string syntax with more syntax, but I'd like to hear what others think first. We're in the process of wrapping up a few big features, so it'll probably take a while to get any feedback on this. Hang in there and we'll come back to you in due course :) thanks |
Thanks @clintongormley And if there is anything particular you are thinking of with the query string syntax as fragile I would be glad to help, of course if that means the syntax itself is what's fragile and not the code supporting it, there's not much anyone could do 👅 |
@tuespetre yeah, that's exactly what i mean. it's a strict syntax, so you really have to understand it to use it properly. It's definitely not for exposing to your average user. The |
I fully understand what you mean. I wrote a gist here the other day (https://gist.github.com./tuespetre/f6951bb665c79abbb7c8) that helps to create user interfaces that build query string queries in the fashion of Github's issue tracker, precisely because we can't expect all of our employees/users to memorize the syntax. But for those who are not intimidated, it's a very powerful tool, and it's so much less boilerplate in the backend to be able to just pass a query string to Elasticsearch. We have use cases that require matching on nested documents ("accounts with a nested item containing this code and that ip address", for instance) so I did feel personally motivated to do this; plus it was very fun to try out Java! I am tweaking the gist this morning to support the 'nested query string' syntax so we can just break it down in the back end and pass it to Elasticsearch in the appropriate way to accomplish the same thing with minimal cruft. So we won't necessarily be missing anything if this doesn't become incorporated. I am just grateful for your consideration. Best of luck with your work and in the meantime if there is anything else I might be able to contribute, feel free to point me there. |
@tuespetre this PR looks like the issue it refers to is closed already. Closing this now, feel free to reopen if I was mistaken. |
Implements #11322, both the basic proposal and the features listed under 'other considerations.' Includes a new test and added assertions to another test.
mvn test Dtests.slow=true
passes all tests.This is my first Java commit so please be kind :)