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

Painless: BWC breaking parsing change in 6.3.0 onwards #33193

Closed
spinscale opened this issue Aug 28, 2018 · 4 comments
Closed

Painless: BWC breaking parsing change in 6.3.0 onwards #33193

spinscale opened this issue Aug 28, 2018 · 4 comments
Labels
blocker >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >regression

Comments

@spinscale
Copy link
Contributor

Elasticsearch version (bin/elasticsearch --version): 6.4.0/6.3.2 (works in 6.2.4)

Description of the problem including expected versus actual behavior:

Parsing of scripts seems to have changed from 6.2 onwards, meaning that requests and watches that worked on 6.2 will suddenly break in 6.3 and above.

Steps to reproduce:

Run this script field with 6.3 and you will get

GET _search
{
  "script_fields": {
    "test": {
      "script": {
        "source" : "def x = 1; if (x == 1) return true"
      }
    }
  }
}

This is the response

{
  "error": {
    "root_cause": [
      {
        "type": "script_exception",
        "reason": "compile error",
        "script_stack": [
          "... ; if (x == 1) return true",
          "                             ^---- HERE"
        ],
        "script": "def x = 1; if (x == 1) return true",
        "lang": "painless"
      }
    ],
    "type": "search_phase_execution_exception",
    "reason": "all shards failed",
    "phase": "query",
    "grouped": true,
    "failed_shards": [
      {
        "shard": 0,
        "index": ".kibana",
        "node": "R2u0OqZuReaW5_cuIiso7w",
        "reason": {
          "type": "script_exception",
          "reason": "compile error",
          "script_stack": [
            "... ; if (x == 1) return true",
            "                             ^---- HERE"
          ],
          "script": "def x = 1; if (x == 1) return true",
          "lang": "painless",
          "caused_by": {
            "type": "illegal_argument_exception",
            "reason": "unexpected token ['<EOF>'] was expecting one of [';']."
          }
        }
      }
    ]
  },
  "status": 500
}

This above search will work fine under 6.2.4 though. If you append a semicolon at the end of the script, this also works in 6.3.2/6.4.0.

@jdconrad can you take a look please? This is breaking BWC compat in a minor release thus I think it warrants being a blocker for now.

@spinscale spinscale added >bug >regression blocker :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Aug 28, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@spinscale spinscale changed the title Painless: BWC breaking parsing change in 6.3.0 Painless: BWC breaking parsing change in 6.3.0 onwards Aug 28, 2018
@jdconrad
Copy link
Contributor

@spinscale Will take a look soon.

@jdconrad
Copy link
Contributor

Posted a fix (#33212)

jdconrad added a commit that referenced this issue Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this issue Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this issue Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this issue Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
@jdconrad
Copy link
Contributor

This is fixed.

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

No branches or pull requests

3 participants