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

Convert BucketScript to static parser #44385

Merged

Conversation

polyfractal
Copy link
Contributor

I was wanting to make some changes to BucketScript to support "empty" buckets, but the old style parser was getting in the way. This converts it over to the new(er) static parser. It also adds a test for GapPolicy enum serialization since that was lacking.

BucketScript was using the old-style parser and could easily be
converted over to the newer static parser.

Also adds a test for GapPolicy enum serialization
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@polyfractal
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polyfractal polyfractal merged commit 40bcee7 into elastic:master Jul 17, 2019
@polyfractal polyfractal deleted the bucketscript_gappolicy_cleanup branch July 17, 2019 14:22
polyfractal added a commit that referenced this pull request Jul 17, 2019
BucketScript was using the old-style parser and could easily be
converted over to the newer static parser.

Also adds a test for GapPolicy enum serialization
@tylersmalley
Copy link
Contributor

We're seeing issues with Kibana that we have tracked back to this commit.

{
  "error": {
    "root_cause": [
      {
        "type": "x_content_parse_exception",
        "reason": "[61:33] [bucket_script] buckets_path doesn't support values of type: VALUE_STRING"
      }
    ],
    "type": "x_content_parse_exception",
    "reason": "[61:33] [bucket_script] buckets_path doesn't support values of type: VALUE_STRING"
  },
  "status": 400
}

Here is an example of a query throwing the exception:

POST /_all/_search?ignore_throttled=true&timeout=30000ms
{
  "query": {
    "bool": {
      "must": [
        {
          "range": {
            "@timestamp": {
              "gte": "2019-07-20T02:38:31.637Z",
              "lte": "2019-07-20T02:53:31.637Z",
              "format": "strict_date_optional_time"
            }
          }
        }
      ],
      "filter": {
        "bool": {
          "must": [],
          "filter": [
            {
              "match_all": {}
            }
          ],
          "should": [],
          "must_not": []
        }
      }
    }
  },
  "aggs": {
    "q": {
      "meta": {
        "type": "split"
      },
      "filters": {
        "filters": {
          "*": {
            "query_string": {
              "query": "*"
            }
          }
        }
      },
      "aggs": {
        "time_buckets": {
          "meta": {
            "type": "time_buckets"
          },
          "date_histogram": {
            "field": "@timestamp",
            "interval": "1s",
            "time_zone": "America/Los_Angeles",
            "extended_bounds": {
              "min": 1563590311637,
              "max": 1563591211637
            },
            "min_doc_count": 0
          },
          "aggs": {
            "count": {
              "bucket_script": {
                "buckets_path": "_count",
                "script": {
                  "source": "_value",
                  "lang": "expression"
                }
              }
            }
          }
        }
      }
    }
  },
  "size": 0
}

Is there an intentional breaking change here, or something we should be doing differently in this Timelion request?

@iverase
Copy link
Contributor

iverase commented Jul 22, 2019

This is not an intentional break. I have opened a PR toads back the possibility of defining buckets_path as a string or as an array.

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

Successfully merging this pull request may close these issues.

None yet

5 participants