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

scripted_metric _agg parameter disappears if params are provided #19768

Closed
consulthys opened this issue Aug 3, 2016 · 1 comment · Fixed by #27159
Closed

scripted_metric _agg parameter disappears if params are provided #19768

consulthys opened this issue Aug 3, 2016 · 1 comment · Fixed by #27159

Comments

@consulthys
Copy link
Contributor

consulthys commented Aug 3, 2016

When running a scripted_metric aggregation (in 2.3.x and 5.0.0-alpha4), if the user needs to specify some parameters in the params section, the implicit _agg hashmap disappears.

When running the following dummy query:

POST index/type/_search
{
  "size": 0,
  "aggs": {
    "testAgg": {
      "scripted_metric": {
        "params": {
          "param1": 10,
          "param2": 20
        },
        "init_script": "_agg['max'] = []",
        "map_script": ";",
        "combine_script": ";",
        "reduce_script": ";"
      }
    }
  }
}

One gets an error stating No such property: _agg for class: b0b7ce0d2dbb5254a703a6f4d048654c3f84857b. There are obviously no errors without the params section.

In order for this to work, one needs to re-specify the _agg implicit parameter in the params section like this:

POST index/type/_search
{
  "size": 0,
  "aggs": {
    "testAgg": {
      "scripted_metric": {
        "params": {
          "_agg": {},
          "param1": 10,
          "param2": 20
        },
        "init_script": "_agg['max'] = []",
        "map_script": ";",
        "combine_script": ";",
        "reduce_script": ";"
      }
    }
  }
}

I'm not sure if this is done on purpose, but it feels a bit counterintuitive. The official documentation (see below) states that _agg is an implicit hashmap, but doesn't state that _agg needs to be explicitly re-defined if other parameters are defined in params .

If this is not specified, the default is the equivalent of providing:

"params" : {
   "_agg" : {}
}

I think this could be easily fixed by modifying ScriptedMetricAggregatorFactory.createInternal() like this:

    Map<String, Object> params = this.params;
    if (params != null) {
        params = deepCopyParams(params, context.searchContext());
    } else {
        params = new HashMap<>();
    }
    if (!params.containsKey("_agg")) {
        params.put("_agg", new HashMap<String, Object>());
    }

Note: If this is deemed worthy, I can gladly submit a PR.

consulthys added a commit to consulthys/elasticsearch that referenced this issue Aug 8, 2016
@consulthys
Copy link
Contributor Author

Issued PR #19863 to fix the above.

colings86 pushed a commit that referenced this issue Nov 8, 2017
)

* Fixes #19768: scripted_metric _agg parameter disappears if params are provided

* Test case for #19768

* Compare boolean to false instead of negating it

* Added mocked script in ScriptedMetricIT

* Fix test in ScriptedMetricIT for implicit _agg map
colings86 pushed a commit that referenced this issue Nov 8, 2017
)

* Fixes #19768: scripted_metric _agg parameter disappears if params are provided

* Test case for #19768

* Compare boolean to false instead of negating it

* Added mocked script in ScriptedMetricIT

* Fix test in ScriptedMetricIT for implicit _agg map
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants