Skip to content

DateHistogramAggregationBuilder#toString yields unexpected error string #25648

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

Closed
yuthura opened this issue Jul 11, 2017 · 5 comments
Closed
Assignees

Comments

@yuthura
Copy link

yuthura commented Jul 11, 2017

Elasticsearch version: 5.3.2

Plugins installed: []

JVM version: 1.8.0_121

OS version: Windows 10

Description of the problem including expected versus actual behavior:

Using toString() on a DateHistogramAggregationBuilder (and most likely other aggregation builders as well) yields the following result:

{ "error" : "JsonGenerationException[Can not write a field name, expecting a value]"}

Expected result is something along the lines of:

{ "my_aggr" : { "date_histogram" : { ... } } }

Steps to reproduce:

Run the following class (adding more calls to the builder continues to yield the same result):

import org.elasticsearch.search.aggregations.AggregationBuilders;

public class Test {
    public static void main(String[] args) {
        System.out.println(
            AggregationBuilders.dateHistogram("revenueHistogram")
        );
    }
}

This is a follow-up on an existing (closed) issue for the following pull request (as request by @cbuescher ): #22280

@cbuescher
Copy link
Member

cbuescher commented Jul 11, 2017

@yuthura yes, thanks for pointing this out. The problem is that ToXContentToBytes#toString(), which is used internally, assumes that the object that should be printed renders to a self-contained json (or yaml, smile...) object, which aggregation builders generally don't do. This is not as easy to fix as in the SuggestionBuilder case you refer to, because AggregationBuilders are all fragments of other json structures.
Note that one way to avoid this is to use XContentHelper.toString(AggregationBuilders.dateHistogram("revenueHistogram") in this example.

@javanna
Copy link
Member

javanna commented Jul 12, 2017

@cbuescher Should we make ToXContentToBytes#toString() call XContentHelper#toString or at least behave the same, so that it checks isFragment? I think we may have the problem of breaking output from classes that haven't ben migrated yet to XContentObject although they output a whole object.

@colings86
Copy link
Contributor

@javanna - @cbuescher and I were just saying the same thing. I think that would be the ideal solution here but we need to migrate the remaining classes to XContentObject otherwise we risk fixing this but breaking other classes.

@yuthura
Copy link
Author

yuthura commented Jul 12, 2017

@cbuescher thanks for the tip on XContentHelper, this solves my use-case for the foreseeable future. :)

@colings86
Copy link
Contributor

This bug is related to #16347 so I'm going to cloase this in favour of that issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants