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

Some multi-bucket aggs cannot be used in pipeline aggs #53215

Closed
jetnet opened this issue Mar 6, 2020 · 4 comments · Fixed by #54161
Closed

Some multi-bucket aggs cannot be used in pipeline aggs #53215

jetnet opened this issue Mar 6, 2020 · 4 comments · Fixed by #54161
Assignees

Comments

@jetnet
Copy link

jetnet commented Mar 6, 2020

Elasticsearch version: 7.6
Plugins installed: [ingest-attachment, ingest-opennl]

JVM version : openjdk version "13.0.2" 2020-01-14

OS version : Windows 10

Description of the problem including expected versus actual behavior:
Filter Agg and IP Range Agg cannot be used as a source for Pipeline Aggs

Steps to reproduce:
Example with the ip_range agg:

GET an_ECS_index/_search
{
  "size": 0,
  "aggs": {
    "1-bucket": {
      "ip_range": {
        "field": "destination.ip",
        "ranges": [
          {
            "mask": "192.168.0.0/16"
          },
          {
            "mask": "10.10.0.0/16"
          }
        ],
        "keyed": false
      }, 
      "aggs": {
        "1-metric": {
          "sum": {
            "field": "source.bytes"
          }
        }
      }
    },
    "1": {
      "sum_bucket": {
        "buckets_path": "1-bucket>1-metric"
      }
    }
  }
}

Error:

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "The first aggregation in buckets_path must be a multi-bucket aggregation for aggregation [1] found :org.elasticsearch.search.aggregations.bucket.range.IpRangeAggregationBuilder for buckets path: 1-bucket>1-metric"
      }

The similar error occurs when using filter agg.

@martijnvg
Copy link
Member

This originated from a discuss topic.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@nik9000 nik9000 self-assigned this Mar 9, 2020
@polyfractal
Copy link
Contributor

Also includes filter agg (#14600), or really any bucketing agg that doesn't directly extend MultiBucket

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Mar 25, 2020
Pipeline aggregations like `stats_bucket`, `sum_bucket`, and
`percentiles_bucket` only operate on buckets that have multiple buckets.
This adds support for those aggregations to `geo_distance`, `ip_range`,
`auto_date_histogram`, and `rare_terms`.

This all happened because we used a marker interface to mark compatible
aggs, `MultiBucketAggregationBuilder` and it was fairly easy to forget
to implement the interface.

This replaces the marker interface with an abstract method in
`AggregationBuilder`, `bucketCardinality` which makes you return `NONE`,
`ONE`, or `MANY`. The `bucket` aggregations can check for `MANY`. At
this point `ONE` and `NONE` amount to about the same thing, but I
suspect that'll be a useful distinction when validating bucket sorts.

Closes elastic#53215
@nik9000
Copy link
Member

nik9000 commented Mar 25, 2020

``filter` intentionally isn't a multi-bucket agg - it is a "single bucket" agg. #54161 just fixes the aggs that is looks like we "forgot". If we want to be able to operate on single bucket aggs we certainly can, but I don't want to sneak that change in with a fix for this.

nik9000 added a commit that referenced this issue Mar 28, 2020
Pipeline aggregations like `stats_bucket`, `sum_bucket`, and
`percentiles_bucket` only operate on buckets that have multiple buckets.
This adds support for those aggregations to `geo_distance`, `ip_range`,
`auto_date_histogram`, and `rare_terms`.

This all happened because we used a marker interface to mark compatible
aggs, `MultiBucketAggregationBuilder` and it was fairly easy to forget
to implement the interface.

This replaces the marker interface with an abstract method in
`AggregationBuilder`, `bucketCardinality` which makes you return `NONE`,
`ONE`, or `MANY`. The `bucket` aggregations can check for `MANY`. At
this point `ONE` and `NONE` amount to about the same thing, but I
suspect that'll be a useful distinction when validating bucket sorts.

Closes #53215
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Mar 28, 2020
…c#54161)

Pipeline aggregations like `stats_bucket`, `sum_bucket`, and
`percentiles_bucket` only operate on buckets that have multiple buckets.
This adds support for those aggregations to `geo_distance`, `ip_range`,
`auto_date_histogram`, and `rare_terms`.

This all happened because we used a marker interface to mark compatible
aggs, `MultiBucketAggregationBuilder` and it was fairly easy to forget
to implement the interface.

This replaces the marker interface with an abstract method in
`AggregationBuilder`, `bucketCardinality` which makes you return `NONE`,
`ONE`, or `MANY`. The `bucket` aggregations can check for `MANY`. At
this point `ONE` and `NONE` amount to about the same thing, but I
suspect that'll be a useful distinction when validating bucket sorts.

Closes elastic#53215
nik9000 added a commit that referenced this issue Mar 30, 2020
#54379)

Pipeline aggregations like `stats_bucket`, `sum_bucket`, and
`percentiles_bucket` only operate on buckets that have multiple buckets.
This adds support for those aggregations to `geo_distance`, `ip_range`,
`auto_date_histogram`, and `rare_terms`.

This all happened because we used a marker interface to mark compatible
aggs, `MultiBucketAggregationBuilder` and it was fairly easy to forget
to implement the interface.

This replaces the marker interface with an abstract method in
`AggregationBuilder`, `bucketCardinality` which makes you return `NONE`,
`ONE`, or `MANY`. The `bucket` aggregations can check for `MANY`. At
this point `ONE` and `NONE` amount to about the same thing, but I
suspect that'll be a useful distinction when validating bucket sorts.

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

Successfully merging a pull request may close this issue.

5 participants