Skip to content

Commit df4a8f7

Browse files
authored
Don't treat counter fields in outside of tsdb as counters. (#93800)
Fields that have the time_series_metric attribute set to counter in non tsdb indices should use number value source type instead of counter value source type. Essentially not handling these fields as counters at search time. Relates to #93539
1 parent 28294e6 commit df4a8f7

File tree

15 files changed

+121
-16
lines changed

15 files changed

+121
-16
lines changed

benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public class ScriptScoreBenchmark {
8282
private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class));
8383

8484
private final Map<String, MappedFieldType> fieldTypes = Map.ofEntries(
85-
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null))
85+
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null, null))
8686
);
8787
private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
8888
private final Map<String, Set<String>> sourcePaths = Map.of("n", Set.of("n"));

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ public ScaledFloatFieldMapper build(MapperBuilderContext context) {
192192
meta.getValue(),
193193
scalingFactor.getValue(),
194194
nullValue.getValue(),
195-
metric.getValue()
195+
metric.getValue(),
196+
indexMode
196197
);
197198
return new ScaledFloatFieldMapper(name, type, multiFieldsBuilder.build(this, context), copyTo.build(), this);
198199
}
@@ -205,6 +206,7 @@ public static final class ScaledFloatFieldType extends SimpleMappedFieldType {
205206
private final double scalingFactor;
206207
private final Double nullValue;
207208
private final TimeSeriesParams.MetricType metricType;
209+
private final IndexMode indexMode;
208210

209211
public ScaledFloatFieldType(
210212
String name,
@@ -214,20 +216,22 @@ public ScaledFloatFieldType(
214216
Map<String, String> meta,
215217
double scalingFactor,
216218
Double nullValue,
217-
TimeSeriesParams.MetricType metricType
219+
TimeSeriesParams.MetricType metricType,
220+
IndexMode indexMode
218221
) {
219222
super(name, indexed, stored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
220223
this.scalingFactor = scalingFactor;
221224
this.nullValue = nullValue;
222225
this.metricType = metricType;
226+
this.indexMode = indexMode;
223227
}
224228

225229
public ScaledFloatFieldType(String name, double scalingFactor) {
226230
this(name, scalingFactor, true);
227231
}
228232

229233
public ScaledFloatFieldType(String name, double scalingFactor, boolean indexed) {
230-
this(name, indexed, false, true, Collections.emptyMap(), scalingFactor, null, null);
234+
this(name, indexed, false, true, Collections.emptyMap(), scalingFactor, null, null, null);
231235
}
232236

233237
public double getScalingFactor() {
@@ -302,7 +306,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
302306
failIfNoDocValues();
303307
}
304308

305-
ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.COUNTER
309+
ValuesSourceType valuesSourceType = indexMode == IndexMode.TIME_SERIES && metricType == TimeSeriesParams.MetricType.COUNTER
306310
? TimeSeriesValuesSourceType.COUNTER
307311
: IndexNumericFieldData.NumericType.LONG.getValuesSourceType();
308312
if ((operation == FielddataOperation.SEARCH || operation == FielddataOperation.SCRIPT) && hasDocValues()) {

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapper.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
113113
meta,
114114
null,
115115
false,
116+
null,
116117
null
117118
);
118119
}

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldTypeTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public void testRangeQuery() throws IOException {
9191
Collections.emptyMap(),
9292
0.1 + randomDouble() * 100,
9393
null,
94+
null,
9495
null
9596
);
9697
Directory dir = newDirectory();
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"avg aggregation on counter field":
3+
- skip:
4+
version: " - 8.7.99"
5+
reason: "counter field support added in 8.7"
6+
7+
- do:
8+
indices.create:
9+
index: myindex1
10+
body:
11+
mappings:
12+
properties:
13+
counter_field:
14+
type : long
15+
time_series_metric: counter
16+
- do:
17+
indices.create:
18+
index: myindex2
19+
body:
20+
settings:
21+
index:
22+
mode: time_series
23+
routing_path: [ keyword_field ]
24+
time_series:
25+
start_time: 2023-01-01T00:00:00Z
26+
end_time: 2024-01-01T00:00:00Z
27+
mappings:
28+
properties:
29+
keyword_field:
30+
type: keyword
31+
time_series_dimension: true
32+
counter_field:
33+
type : long
34+
time_series_metric: counter
35+
- do:
36+
search:
37+
index: myindex1
38+
body:
39+
aggs:
40+
the_counter_avg:
41+
avg:
42+
field: counter_field
43+
- match: { aggregations.the_counter_avg.value: null }
44+
45+
- do:
46+
catch: /Field \[counter_field\] of type \[long\] is not supported for aggregation \[avg\]/
47+
search:
48+
index: myindex2
49+
body:
50+
aggs:
51+
the_counter_avg:
52+
avg:
53+
field: counter_field

server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,7 @@ public static class NumberFieldType extends SimpleMappedFieldType {
14651465
private final FieldValues<Number> scriptValues;
14661466
private final boolean isDimension;
14671467
private final MetricType metricType;
1468+
private final IndexMode indexMode;
14681469

14691470
public NumberFieldType(
14701471
String name,
@@ -1477,7 +1478,8 @@ public NumberFieldType(
14771478
Map<String, String> meta,
14781479
FieldValues<Number> script,
14791480
boolean isDimension,
1480-
MetricType metricType
1481+
MetricType metricType,
1482+
IndexMode indexMode
14811483
) {
14821484
super(name, isIndexed, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
14831485
this.type = Objects.requireNonNull(type);
@@ -1486,6 +1488,7 @@ public NumberFieldType(
14861488
this.scriptValues = script;
14871489
this.isDimension = isDimension;
14881490
this.metricType = metricType;
1491+
this.indexMode = indexMode;
14891492
}
14901493

14911494
NumberFieldType(String name, Builder builder) {
@@ -1500,7 +1503,8 @@ public NumberFieldType(
15001503
builder.meta.getValue(),
15011504
builder.scriptValues(),
15021505
builder.dimension.getValue(),
1503-
builder.metric.getValue()
1506+
builder.metric.getValue(),
1507+
builder.indexMode
15041508
);
15051509
}
15061510

@@ -1509,7 +1513,7 @@ public NumberFieldType(String name, NumberType type) {
15091513
}
15101514

15111515
public NumberFieldType(String name, NumberType type, boolean isIndexed) {
1512-
this(name, type, isIndexed, false, true, true, null, Collections.emptyMap(), null, false, null);
1516+
this(name, type, isIndexed, false, true, true, null, Collections.emptyMap(), null, false, null, null);
15131517
}
15141518

15151519
@Override
@@ -1589,7 +1593,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
15891593
failIfNoDocValues();
15901594
}
15911595

1592-
ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.COUNTER
1596+
ValuesSourceType valuesSourceType = indexMode == IndexMode.TIME_SERIES && metricType == TimeSeriesParams.MetricType.COUNTER
15931597
? TimeSeriesValuesSourceType.COUNTER
15941598
: type.numericType.getValuesSourceType();
15951599

server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,20 @@ private void doTestRequireDocValues(MappedFieldType ft) {
332332
public void testRequireDocValuesOnLongs() {
333333
doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", LONG));
334334
doTestRequireDocValues(
335-
new NumberFieldMapper.NumberFieldType("field", LONG, true, false, false, false, null, Collections.emptyMap(), null, false, null)
335+
new NumberFieldMapper.NumberFieldType(
336+
"field",
337+
LONG,
338+
true,
339+
false,
340+
false,
341+
false,
342+
null,
343+
Collections.emptyMap(),
344+
null,
345+
false,
346+
null,
347+
null
348+
)
336349
);
337350
}
338351

@@ -350,6 +363,7 @@ public void testRequireDocValuesOnDoubles() {
350363
Collections.emptyMap(),
351364
null,
352365
false,
366+
null,
353367
null
354368
)
355369
);

server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,20 @@ public void testLongTermQueryWithDecimalPart() {
127127
}
128128

129129
private static MappedFieldType unsearchable() {
130-
return new NumberFieldType("field", NumberType.LONG, false, false, false, true, null, Collections.emptyMap(), null, false, null);
130+
return new NumberFieldType(
131+
"field",
132+
NumberType.LONG,
133+
false,
134+
false,
135+
false,
136+
true,
137+
null,
138+
Collections.emptyMap(),
139+
null,
140+
false,
141+
null,
142+
null
143+
);
131144
}
132145

133146
public void testTermQuery() {

server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ private ValuesSourceConfig getVSConfig(
8989
Collections.emptyMap(),
9090
null,
9191
false,
92+
null,
9293
null
9394
);
9495
return ValuesSourceConfig.resolveFieldOnly(ft, context);

server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,7 @@ public void testDocValuesFieldExistsForNumber() throws IOException {
15121512
Map.of(),
15131513
null,
15141514
false,
1515+
null,
15151516
null
15161517
);
15171518
docValuesFieldExistsTestCase(new ExistsQueryBuilder("f"), ft, true, i -> {
@@ -1534,6 +1535,7 @@ public void testDocValuesFieldExistsForNumberWithoutData() throws IOException {
15341535
Map.of(),
15351536
null,
15361537
false,
1538+
null,
15371539
null
15381540
)
15391541
);

0 commit comments

Comments
 (0)