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

Make `_source` parsing in `top_hits` aggregation consistent with the search api #6997

Merged
merged 1 commit into from Jul 24, 2014

Conversation

Projects
None yet
5 participants
@martijnvg
Copy link
Member

martijnvg commented Jul 23, 2014

No description provided.

@martijnvg martijnvg added bug and removed v1.3.0 labels Jul 23, 2014

@jpountz

View changes

src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsParser.java Outdated
@@ -121,6 +124,9 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se
case "fielddata_fields":
fieldDataFieldsParseElement.parse(parser, topHitsContext);
break;
case "_source":
sourceParseElement.parse(parser, topHitsContext);
break;

This comment has been minimized.

Copy link
@jpountz

jpountz Jul 24, 2014

Contributor

Since it can work on values and objects, maybe we should just have an: else if "_source".equals(currentFieldName) condition right before else if (token.isValues()) ? This could help avoid duplicate the parsing?

@jpountz

This comment has been minimized.

Copy link
Contributor

jpountz commented Jul 24, 2014

Left one comment but the change looks good to me. Feel free to push if it is not applicable.

@martijnvg martijnvg added the v1.3.1 label Jul 24, 2014

@pierrre

This comment has been minimized.

Copy link

pierrre commented Jul 24, 2014

"_source": false doesn't work in top_hits aggregation.
Is it related?

My fix:

            "_source": {
              "exclude": [
                "*"
              ]
            }
Made `_source` parsing in `top_hits` aggregation consistent with regu…
…lar `_source` parsing in search api.

Closes #6997

@martijnvg martijnvg merged commit 73f7f42 into elastic:master Jul 24, 2014

martijnvg added a commit that referenced this pull request Jul 24, 2014

martijnvg added a commit that referenced this pull request Jul 24, 2014

Made `_source` parsing in `top_hits` aggregation consistent with regu…
…lar `_source` parsing in search api.

Closes #6997
@martijnvg

This comment has been minimized.

Copy link
Member Author

martijnvg commented Jul 24, 2014

@pierrre Yes it is related, this PR fixes this.

@pierrre

This comment has been minimized.

Copy link

pierrre commented Jul 24, 2014

@martijnvg thank you :)

@clintongormley clintongormley changed the title Make `_source` parsing in `top_hits` aggregation and search api consistent Aggregations: Make `_source` parsing in `top_hits` aggregation and search api consistent Jul 28, 2014

@martijnvg martijnvg deleted the martijnvg:bugs/top_hits__source_parsing branch May 18, 2015

@clintongormley clintongormley changed the title Aggregations: Make `_source` parsing in `top_hits` aggregation and search api consistent Make `_source` parsing in `top_hits` aggregation consistent with the search api Jun 7, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.