Skip to content

Commit

Permalink
Deprecate dots in aggregation names
Browse files Browse the repository at this point in the history
Dots in aggregation names can lead to tricky parsing situations, like
being unable to sort by agg names with dots.

This adds a deprecation logger and a note in the docs, letting us
remove them in 8.0.

Related to elastic#17600 and elastic#19040
  • Loading branch information
polyfractal committed Jun 20, 2018
1 parent 90d62e6 commit 646e1e9
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 2 deletions.
8 changes: 7 additions & 1 deletion docs/reference/aggregations.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ The following snippet captures the basic structure of aggregations:
The `aggregations` object (the key `aggs` can also be used) in the JSON holds the aggregations to be computed. Each aggregation
is associated with a logical name that the user defines (e.g. if the aggregation computes the average price, then it would
make sense to name it `avg_price`). These logical names will also be used to uniquely identify the aggregations in the
response. Each aggregation has a specific type (`<aggregation_type>` in the above snippet) and is typically the first
response.

Aggregation names can be any alphanumeric character except a left bracket (`[`), right bracket (`]`) or greater-than sign (`>`).

deprecated[6.4.0, Periods in aggregations names have been deprecated, and will be removed in `8.0.0`.]

Each aggregation has a specific type (`<aggregation_type>` in the above snippet) and is typically the first
key within the named aggregation body. Each type of aggregation defines its own body, depending on the nature of the
aggregation (e.g. an `avg` aggregation on a specific field will define the field on which the average will be calculated).
At the same level of the aggregation type definition, one can optionally define a set of additional aggregations,
Expand Down
7 changes: 6 additions & 1 deletion docs/reference/migration/migrate_7_0/aggregations.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@ Requests that try to return more than the limit will fail with an exception.
==== `missing` option of the `composite` aggregation has been removed

The `missing` option of the `composite` aggregation, deprecated in 6.x,
has been removed. `missing_bucket` should be used instead.
has been removed. `missing_bucket` should be used instead.

==== Dots in aggregation names have been deprecated

Starting in 6.4.0, dots in aggregation names have been deprecated. Aggregation names that use
dots will start to throw exceptions in 8.0.0.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
setup:
- do:
indices.create:
index: test_1
body:
settings:
number_of_replicas: 0
mappings:
doc:
properties:
int_field:
type : integer
double_field:
type : double
string_field:
type: keyword

- do:
bulk:
refresh: true
body:
- index:
_index: test_1
_type: doc
_id: 1
- int_field: 1
double_field: 1.0
string_field: foo
- index:
_index: test_1
_type: doc
_id: 2
- int_field: 51
double_field: 51.0
string_field: foo
- index:
_index: test_1
_type: doc
_id: 3
- int_field: 101
double_field: 101.0
string_field: foo
- index:
_index: test_1
_type: doc
_id: 4
- int_field: 151
double_field: 151.0
string_field: foo

---
"Dots in agg names":

- skip:
version: " - 6.3.99"
reason: this feature was deprecated in 6.4.0
features: "warnings"
- do:
warnings:
- "Dots in aggregation names are deprecated and will be removed in 8.0"
search:
body:
aggs:
dots.in.agg.name:
avg:
field: int_field

- match: { hits.total: 4 }
- length: { hits.hits: 4 }
- match:
aggregations:
dots.in.agg.name:
value: 76.0


Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand Down Expand Up @@ -50,6 +52,7 @@
import java.util.regex.Pattern;

public class AggregatorFactories {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger("deprecation"));
public static final Pattern VALID_AGG_NAME = Pattern.compile("[^\\[\\]>]+");

/**
Expand All @@ -75,6 +78,9 @@ private static AggregatorFactories.Builder parseAggregators(XContentParser parse
throw new ParsingException(parser.getTokenLocation(), "Invalid aggregation name [" + aggregationName
+ "]. Aggregation names must be alpha-numeric and can only contain '_' and '-'");
}
if (aggregationName.contains(".")) {
deprecationLogger.deprecated("Dots in aggregation names are deprecated and will be removed in 8.0");
}

token = parser.nextToken();
if (token != XContentParser.Token.START_OBJECT) {
Expand Down

0 comments on commit 646e1e9

Please sign in to comment.