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

StackOverflowError running query script and agg script #7487

Closed
markharwood opened this Issue Aug 27, 2014 · 4 comments

Comments

Projects
None yet
6 participants
@markharwood
Copy link
Contributor

commented Aug 27, 2014

This query was generated by a benchmarking framework that creates random combinations of clauses and this combo of scripted agg and script_score that references "_score" causes a StackOverflowError

curl -XPUT "http://localhost:9200/test?pretty=true" -d'
{
  "mappings": {
    "car": {
      "properties": {
        "make": {
          "type": "string",
          "index": "not_analyzed"
        },
        "model": {
          "type": "string",
          "index": "not_analyzed"
        },
        "mileage": {
          "type": "integer"
        }
      }
    }
  }
}'
curl -XPOST "http://localhost:9200/test/car/1?pretty=true" -d'
{
  "make": "bmw",
  "model": "m3",
  "mileage": 30000
}'  
curl -XPOST "http://localhost:9200/test/car/_search?pretty" -d'
{
   "query": {
      "function_score": {
         "query": {
            "term": {
               "model": "m3"
            }
         },
         "script_score": {
            "script": "_score * doc[\"mileage\"].value "
         },
         "boost_mode": "replace"
      }
   },
   "aggs": {
      "makes": {
         "terms": {
            "script": "doc[\"make\"].value"
         }
      }
   }
}'  

@markharwood markharwood added the bug label Aug 27, 2014

@clintongormley clintongormley assigned jpountz and colings86 and unassigned jpountz Sep 6, 2014

@colings86

This comment has been minimized.

Copy link
Member

commented Sep 10, 2014

So this is caused by the following:

The aggregation framework registers the agg script as scorer aware. When setScorer is called it eventually delegates down to DocLookup.setScorer(). DocLookup is global though so the agg script overwrites the original scorer (the one that the score script needs to use) and replaces it with the Score script. So now when the score script evaluates _score instead of using the original scorer it tries to use itself causing a recursive loop.

The solution should be to replace the global DocLookup with a DocLookup for each context (e.g. one for aggs, one for query_score, etc.)

@brwe

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2014

The solution should be to replace the global DocLookup with a DocLookup for each context (e.g. one for aggs, one for query_score, etc.)

I believe it might be sufficient to just remove the dependence of DocLookup and scorer. The scorer should be set for each script explicitly instead of having the script score set the scorer in DocLookup which is then in turn looked up by the script when it is executed.
Here is what I mean: brwe@5e142fe

brwe added a commit to brwe/elasticsearch that referenced this issue Sep 22, 2014

script with _score: remove dependency of DocLookup and scorer
As pointed out in elastic#7487 DocLookup is a variable that is accessible by all scripts
for one doc while the query is executed. But the _score and therfore the scorer
depends on the current context, that is, which part of query is currently executed.
Instead of setting the scorer for DocLookup
and have Script access the DocLookup for getting the score, the Scorer should just
be explicitely set for each script.
DocLookup should not have any reference to a scorer.
This was similarly discussed in elastic#7043.

This dependency caused a stackoverflow when running script score in combination with an
aggregation on _score. Also the wrong scorer was called when nesting several script scores.

closes elastic#7487

@s1monw s1monw removed the v1.3.3 label Sep 23, 2014

@clintongormley

This comment has been minimized.

Copy link
Member

commented Sep 25, 2014

@brwe @markharwood any progress on this one?

@brwe

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2014

@clintongormley See my PR above, needs a reviewer

@brwe brwe closed this in 7feb742 Sep 26, 2014

brwe added a commit that referenced this issue Sep 26, 2014

script with _score: remove dependency of DocLookup and scorer
As pointed out in #7487 DocLookup is a variable that is accessible by all scripts
for one doc while the query is executed. But the _score and therfore the scorer
depends on the current context, that is, which part of query is currently executed.
Instead of setting the scorer for DocLookup
and have Script access the DocLookup for getting the score, the Scorer should just
be explicitely set for each script.
DocLookup should not have any reference to a scorer.
This was similarly discussed in #7043.

This dependency caused a stackoverflow when running script score in combination with an
aggregation on _score. Also the wrong scorer was called when nesting several script scores.

closes #7487
closes #7819

brwe added a commit that referenced this issue Sep 26, 2014

script with _score: remove dependency of DocLookup and scorer
As pointed out in #7487 DocLookup is a variable that is accessible by all scripts
for one doc while the query is executed. But the _score and therfore the scorer
depends on the current context, that is, which part of query is currently executed.
Instead of setting the scorer for DocLookup
and have Script access the DocLookup for getting the score, the Scorer should just
be explicitely set for each script.
DocLookup should not have any reference to a scorer.
This was similarly discussed in #7043.

This dependency caused a stackoverflow when running script score in combination with an
aggregation on _score. Also the wrong scorer was called when nesting several script scores.

closes #7487
closes #7819

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

script with _score: remove dependency of DocLookup and scorer
As pointed out in elastic#7487 DocLookup is a variable that is accessible by all scripts
for one doc while the query is executed. But the _score and therfore the scorer
depends on the current context, that is, which part of query is currently executed.
Instead of setting the scorer for DocLookup
and have Script access the DocLookup for getting the score, the Scorer should just
be explicitely set for each script.
DocLookup should not have any reference to a scorer.
This was similarly discussed in elastic#7043.

This dependency caused a stackoverflow when running script score in combination with an
aggregation on _score. Also the wrong scorer was called when nesting several script scores.

closes elastic#7487
closes elastic#7819
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.