-
Notifications
You must be signed in to change notification settings - Fork 40
One request mapping #111
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
base: master
Are you sure you want to change the base?
One request mapping #111
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| //! Joining queries | ||
|
|
||
| use ::json::ShouldSkip; | ||
| use ::serde_json::Value; | ||
|
|
||
| use super::{ScoreMode, Query}; | ||
|
|
||
|
|
@@ -50,14 +51,17 @@ impl NestedQuery { | |
| /// Has Child query | ||
| #[derive(Debug, Default, Serialize)] | ||
| pub struct HasChildQuery { | ||
| #[serde(rename="type")] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for this :) it must have been missing for quite a while |
||
| doc_type: String, | ||
| query: Query, | ||
| #[serde(skip_serializing_if="ShouldSkip::should_skip")] | ||
| score_mode: Option<ScoreMode>, | ||
| #[serde(skip_serializing_if="ShouldSkip::should_skip")] | ||
| min_children: Option<u64>, | ||
| #[serde(skip_serializing_if="ShouldSkip::should_skip")] | ||
| max_children: Option<u64> | ||
| max_children: Option<u64>, | ||
| #[serde(skip_serializing_if="ShouldSkip::should_skip")] | ||
| inner_hits: Option<Value> | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preferably this would be a concrete type rather than
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The link I sent you in the other comment is similar to this one - Although I agree that a concrete type would be way preferable, I find not using a
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of Inner Hits potentially returning more than one query? Yes, that's true. We could have a hybrid option as there is some predictable structure. E.g. the
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we also add the same to the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it actually? https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-has-parent-query.html I can't find it here but in case sure, I can add
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I've ever needed it on a |
||
| } | ||
|
|
||
| /// Has Parent query | ||
|
|
@@ -95,6 +99,7 @@ impl HasChildQuery { | |
| add_field!(with_score_mode, score_mode, ScoreMode); | ||
| add_field!(with_min_children, min_children, u64); | ||
| add_field!(with_max_children, max_children, u64); | ||
| add_field!(with_inner_hits, inner_hits, Value); | ||
|
|
||
| build!(HasChild); | ||
| } | ||
|
|
||
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.
Is the
Valueneeded due to the potential of more-than-one different type coming back in the results? In which case its probably unavoidable.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.
Indeed - You can have a look at this as reference: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-inner-hits.html