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

Include user parameters in painless reduce script context #42046

Closed
nbrahms opened this issue May 9, 2019 · 6 comments
Closed

Include user parameters in painless reduce script context #42046

nbrahms opened this issue May 9, 2019 · 6 comments
Assignees
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache

Comments

@nbrahms
Copy link

nbrahms commented May 9, 2019

Hello,

I'd like to request that script parameters be available in scripted metric reduce scripts.

Here's a toy example:

GET foo/_search
{
  "query" : {
    "match_all": {}
  },
  "size": 0,
  "aggregations": {
    "foo": {
      "scripted_metric": {
        "params": {
          "foo": 2
        },
        "init_script": "state.bla = 0",
        "map_script": "state.bla += 2",
        "combine_script": "return state.bla",
        "reduce_script": "int r = 0; for (s in states) r += s; return r * params.foo"
      }
    }
  }
}

which returns:

{
  "error": {
    "root_cause": [],
    "type": "search_phase_execution_exception",
    "reason": "",
    "phase": "fetch",
    "grouped": true,
    "failed_shards": [],
    "caused_by": {
      "type": "script_exception",
      "reason": "runtime error",
      "script_stack": [
        "return r * params.foo",
        "                 ^---- HERE"
      ],
      "script": "int r = 0; for (s in states) r += s; return r * params.foo",
      "lang": "painless",
      "caused_by": {
        "type": "null_pointer_exception",
        "reason": null
      }
    }
  },
  "status": 503
}

IMO it should be pretty common to parameterize reduction (in my case, I want to control a final sort order). Can we throw this on the roadmap?

@nbrahms
Copy link
Author

nbrahms commented May 9, 2019

For what it's worth, this is the current workaround I'm using:

{
  "query" : {
    "match_all": {}
  },
  "size": 0,
  "aggregations": {
    "foo": {
      "scripted_metric": {
        "params": {
          "foo": 2
        },
        "init_script": "state.bla = 0; state.foo = params.foo",
        "map_script": "state.bla += 2",
        "combine_script": "return state",
        "reduce_script": "int r = 0; for (s in states) r += s.bla; return r * states[0].foo"
      }
    }
  }
}

@jakelandis jakelandis added the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label May 10, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jdconrad jdconrad self-assigned this May 10, 2019
@jdconrad jdconrad added the >bug label May 10, 2019
@jdconrad
Copy link
Contributor

Seems like a bug. Will look into this soon.

@nbrahms
Copy link
Author

nbrahms commented May 15, 2019

Awesome, thanks for the fast response!

@jdconrad
Copy link
Contributor

This fix is now in master for 8.0 and will be backported to 7.3 as well.

@booster96
Copy link

booster96 commented Dec 19, 2019

how can I use some aggregated value(for ex sum of any column) which is outside scripted_metric and then use that value in params.foo inside scripted_metric. Please advice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache
Projects
None yet
Development

No branches or pull requests

5 participants