Allow bucket paths to specify _count within a bucket #85720
Conversation
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
Hi @benwtrent, I've created a changelog YAML for you. |
…nt/elasticsearch into feature/agg-bucket-path-refactor
| assertInvalidPath("foo[]", "empty brackets in the token expression"); | ||
| assertInvalidPath("foo[bar]baz", "brackets not enclosing at the end of the token expression"); | ||
| assertInvalidPath(".foo", "dot separator at the beginning of the token expression"); | ||
| assertInvalidPath("foo.", "dot separator at the end of the token expression"); |
There was a problem hiding this comment.
Let's leave these reasons as comments then? It may not be immediately obvious why some of these are wrong.
| int keyIndex = element.lastIndexOf('['); | ||
| int metricIndex = element.lastIndexOf('.'); | ||
| if (keyIndex >= 0) { | ||
| if (keyIndex == 0 || keyIndex > element.length() - 3) { | ||
| throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); |
There was a problem hiding this comment.
Unrelated to this PR, but AggregationExecutionException is the wrong thing to throw here. That's a 500 class error, but this is a user error that should not be retried, which is what 400 class is for. I opened #86273 to try to not lose track of it.
| * <li> | ||
| * {@code agg1["foo"]._count} - where agg1 is multi-bucket, and the path expects a bucket "foo". | ||
| * This would extract the doc_count for that specific bucket. | ||
| * </li> |
There was a problem hiding this comment.
These examples should probably also land in the user facing docs
I 100% agree. It is out of the scope of this particular PR. But, it strikes me that agg paths should reflect the aggregation DAG and be able to walk it. Seems like it would be good to refactor this into an N node tree that can be walked given Builders or Aggregators |
Users should be able to specify specific metrics/keys within a specific bucket key.
An example is
agg["bucket_foo"]._count.This change now allows that.
closes: #76320