Skip to content

Commit e1b4209

Browse files
felixbarnyfelixbarny
authored
Allow dimension fields to have multiple values in standard and logsdb index mode (#112345)
Fixes #112232 Fixes #112239
1 parent 827e90d commit e1b4209

File tree

12 files changed

+194
-65
lines changed

12 files changed

+194
-65
lines changed

‎docs/changelog/112345.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
pr: 112345
2+
summary: Allow dimension fields to have multiple values in standard and logsdb index
3+
mode
4+
area: Mapping
5+
type: enhancement
6+
issues:
7+
- 112232
8+
- 112239

‎server/src/main/java/org/elasticsearch/index/IndexMode.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public IdFieldMapper buildIdFieldMapper(BooleanSupplier fieldDataEnabled) {
107107

108108
@Override
109109
public DocumentDimensions buildDocumentDimensions(IndexSettings settings) {
110-
return new DocumentDimensions.OnlySingleValueAllowed();
110+
return DocumentDimensions.Noop.INSTANCE;
111111
}
112112

113113
@Override
@@ -281,7 +281,7 @@ public MetadataFieldMapper timeSeriesRoutingHashFieldMapper() {
281281

282282
@Override
283283
public DocumentDimensions buildDocumentDimensions(IndexSettings settings) {
284-
return new DocumentDimensions.OnlySingleValueAllowed();
284+
return DocumentDimensions.Noop.INSTANCE;
285285
}
286286

287287
@Override

‎server/src/main/java/org/elasticsearch/index/mapper/DocumentDimensions.java‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
import org.elasticsearch.index.IndexSettings;
1313

1414
import java.net.InetAddress;
15-
import java.util.HashSet;
16-
import java.util.Set;
1715

1816
/**
1917
* Collects dimensions from documents.
@@ -49,59 +47,45 @@ default DocumentDimensions addString(String fieldName, String value) {
4947
DocumentDimensions validate(IndexSettings settings);
5048

5149
/**
52-
* Makes sure that each dimension only appears on time.
50+
* Noop implementation that doesn't perform validations on dimension fields
5351
*/
54-
class OnlySingleValueAllowed implements DocumentDimensions {
55-
private final Set<String> names = new HashSet<>();
52+
enum Noop implements DocumentDimensions {
53+
54+
INSTANCE;
5655

5756
@Override
58-
public DocumentDimensions addString(String fieldName, BytesRef value) {
59-
add(fieldName);
57+
public DocumentDimensions addString(String fieldName, BytesRef utf8Value) {
6058
return this;
6159
}
6260

63-
// Override to skip the UTF-8 conversion that happens in the default implementation
6461
@Override
6562
public DocumentDimensions addString(String fieldName, String value) {
66-
add(fieldName);
6763
return this;
6864
}
6965

7066
@Override
7167
public DocumentDimensions addIp(String fieldName, InetAddress value) {
72-
add(fieldName);
7368
return this;
7469
}
7570

7671
@Override
7772
public DocumentDimensions addLong(String fieldName, long value) {
78-
add(fieldName);
7973
return this;
8074
}
8175

8276
@Override
8377
public DocumentDimensions addUnsignedLong(String fieldName, long value) {
84-
add(fieldName);
8578
return this;
8679
}
8780

8881
@Override
8982
public DocumentDimensions addBoolean(String fieldName, boolean value) {
90-
add(fieldName);
9183
return this;
9284
}
9385

9486
@Override
95-
public DocumentDimensions validate(final IndexSettings settings) {
96-
// DO NOTHING
87+
public DocumentDimensions validate(IndexSettings settings) {
9788
return this;
9889
}
99-
100-
private void add(String fieldName) {
101-
boolean isNew = names.add(fieldName);
102-
if (false == isNew) {
103-
throw new IllegalArgumentException("Dimension field [" + fieldName + "] cannot be a multi-valued field.");
104-
}
105-
}
106-
};
90+
}
10791
}

‎server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.common.Strings;
1818
import org.elasticsearch.common.settings.Settings;
1919
import org.elasticsearch.core.Tuple;
20+
import org.elasticsearch.index.IndexMode;
2021
import org.elasticsearch.index.IndexSettings;
2122
import org.elasticsearch.index.IndexVersion;
2223
import org.elasticsearch.script.BooleanFieldScript;
@@ -25,11 +26,14 @@
2526
import org.elasticsearch.xcontent.XContentFactory;
2627

2728
import java.io.IOException;
29+
import java.time.Instant;
2830
import java.util.List;
2931
import java.util.function.Function;
3032

3133
import static org.hamcrest.Matchers.containsString;
3234
import static org.hamcrest.Matchers.equalTo;
35+
import static org.hamcrest.Matchers.greaterThan;
36+
import static org.hamcrest.Matchers.hasSize;
3337

3438
public class BooleanFieldMapperTests extends MapperTestCase {
3539

@@ -257,17 +261,29 @@ public void testDimensionIndexedAndDocvalues() {
257261
}
258262
}
259263

260-
public void testDimensionMultiValuedField() throws IOException {
261-
XContentBuilder mapping = fieldMapping(b -> {
264+
public void testDimensionMultiValuedFieldTSDB() throws IOException {
265+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
262266
minimalMapping(b);
263267
b.field("time_series_dimension", true);
264-
});
265-
DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping);
268+
}), IndexMode.TIME_SERIES);
266269

267270
Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", true, false))));
268271
assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field"));
269272
}
270273

274+
public void testDimensionMultiValuedFieldNonTSDB() throws IOException {
275+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
276+
minimalMapping(b);
277+
b.field("time_series_dimension", true);
278+
}), randomFrom(IndexMode.STANDARD, IndexMode.LOGSDB));
279+
280+
ParsedDocument doc = mapper.parse(source(b -> {
281+
b.array("field", true, false);
282+
b.field("@timestamp", Instant.now());
283+
}));
284+
assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1)));
285+
}
286+
271287
public void testDimensionInRoutingPath() throws IOException {
272288
MapperService mapper = createMapperService(fieldMapping(b -> b.field("type", "keyword").field("time_series_dimension", true)));
273289
IndexSettings settings = createIndexSettings(

‎server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.elasticsearch.common.network.InetAddresses;
1919
import org.elasticsearch.common.network.NetworkAddress;
2020
import org.elasticsearch.core.Tuple;
21+
import org.elasticsearch.index.IndexMode;
2122
import org.elasticsearch.index.IndexVersion;
2223
import org.elasticsearch.index.IndexVersions;
2324
import org.elasticsearch.script.IpFieldScript;
@@ -26,6 +27,7 @@
2627

2728
import java.io.IOException;
2829
import java.net.InetAddress;
30+
import java.time.Instant;
2931
import java.util.ArrayList;
3032
import java.util.List;
3133
import java.util.function.Function;
@@ -35,6 +37,8 @@
3537
import static org.hamcrest.Matchers.containsString;
3638
import static org.hamcrest.Matchers.empty;
3739
import static org.hamcrest.Matchers.equalTo;
40+
import static org.hamcrest.Matchers.greaterThan;
41+
import static org.hamcrest.Matchers.hasSize;
3842

3943
public class IpFieldMapperTests extends MapperTestCase {
4044

@@ -255,11 +259,11 @@ public void testDimensionIndexedAndDocvalues() {
255259
}
256260
}
257261

258-
public void testDimensionMultiValuedField() throws IOException {
262+
public void testDimensionMultiValuedFieldTSDB() throws IOException {
259263
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
260264
minimalMapping(b);
261265
b.field("time_series_dimension", true);
262-
}));
266+
}), IndexMode.TIME_SERIES);
263267

264268
Exception e = expectThrows(
265269
DocumentParsingException.class,
@@ -268,6 +272,19 @@ public void testDimensionMultiValuedField() throws IOException {
268272
assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field"));
269273
}
270274

275+
public void testDimensionMultiValuedFieldNonTSDB() throws IOException {
276+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
277+
minimalMapping(b);
278+
b.field("time_series_dimension", true);
279+
}), randomFrom(IndexMode.STANDARD, IndexMode.LOGSDB));
280+
281+
ParsedDocument doc = mapper.parse(source(b -> {
282+
b.array("field", "192.168.1.1", "192.168.1.1");
283+
b.field("@timestamp", Instant.now());
284+
}));
285+
assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1)));
286+
}
287+
271288
@Override
272289
protected String generateRandomInputValue(MappedFieldType ft) {
273290
return NetworkAddress.format(randomIp(randomBoolean()));

‎server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.Strings;
2626
import org.elasticsearch.common.lucene.Lucene;
2727
import org.elasticsearch.common.settings.Settings;
28+
import org.elasticsearch.index.IndexMode;
2829
import org.elasticsearch.index.IndexSettings;
2930
import org.elasticsearch.index.IndexVersion;
3031
import org.elasticsearch.index.analysis.AnalyzerScope;
@@ -44,6 +45,7 @@
4445
import org.elasticsearch.xcontent.XContentBuilder;
4546

4647
import java.io.IOException;
48+
import java.time.Instant;
4749
import java.util.Arrays;
4850
import java.util.Collection;
4951
import java.util.List;
@@ -57,6 +59,8 @@
5759
import static org.hamcrest.Matchers.containsString;
5860
import static org.hamcrest.Matchers.empty;
5961
import static org.hamcrest.Matchers.equalTo;
62+
import static org.hamcrest.Matchers.greaterThan;
63+
import static org.hamcrest.Matchers.hasSize;
6064
import static org.hamcrest.Matchers.instanceOf;
6165

6266
public class KeywordFieldMapperTests extends MapperTestCase {
@@ -373,17 +377,29 @@ public void testDimensionIndexedAndDocvalues() {
373377
}
374378
}
375379

376-
public void testDimensionMultiValuedField() throws IOException {
377-
XContentBuilder mapping = fieldMapping(b -> {
380+
public void testDimensionMultiValuedFieldTSDB() throws IOException {
381+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
378382
minimalMapping(b);
379383
b.field("time_series_dimension", true);
380-
});
381-
DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping);
384+
}), IndexMode.TIME_SERIES);
382385

383386
Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", "1234", "45678"))));
384387
assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field"));
385388
}
386389

390+
public void testDimensionMultiValuedFieldNonTSDB() throws IOException {
391+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
392+
minimalMapping(b);
393+
b.field("time_series_dimension", true);
394+
}), randomFrom(IndexMode.STANDARD, IndexMode.LOGSDB));
395+
396+
ParsedDocument doc = mapper.parse(source(b -> {
397+
b.array("field", "1234", "45678");
398+
b.field("@timestamp", Instant.now());
399+
}));
400+
assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1)));
401+
}
402+
387403
public void testDimensionExtraLongKeyword() throws IOException {
388404
DocumentMapper mapper = createTimeSeriesModeDocumentMapper(fieldMapping(b -> {
389405
minimalMapping(b);

‎server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.common.bytes.BytesArray;
1616
import org.elasticsearch.common.bytes.BytesReference;
1717
import org.elasticsearch.common.settings.Settings;
18+
import org.elasticsearch.index.IndexMode;
1819
import org.elasticsearch.index.IndexSettings;
1920
import org.elasticsearch.index.IndexVersion;
2021
import org.elasticsearch.index.mapper.DocumentMapper;
@@ -34,6 +35,7 @@
3435
import org.junit.AssumptionViolatedException;
3536

3637
import java.io.IOException;
38+
import java.time.Instant;
3739
import java.util.ArrayList;
3840
import java.util.Arrays;
3941
import java.util.Collections;
@@ -46,6 +48,8 @@
4648
import static org.apache.lucene.tests.analysis.BaseTokenStreamTestCase.assertTokenStreamContents;
4749
import static org.hamcrest.Matchers.containsString;
4850
import static org.hamcrest.Matchers.equalTo;
51+
import static org.hamcrest.Matchers.greaterThan;
52+
import static org.hamcrest.Matchers.hasSize;
4953

5054
public class FlattenedFieldMapperTests extends MapperTestCase {
5155

@@ -189,12 +193,11 @@ public void testDimensionIndexedAndDocvalues() {
189193
}
190194
}
191195

192-
public void testDimensionMultiValuedField() throws IOException {
193-
XContentBuilder mapping = fieldMapping(b -> {
196+
public void testDimensionMultiValuedFieldTSDB() throws IOException {
197+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
194198
minimalMapping(b);
195199
b.field("time_series_dimensions", List.of("key1", "key2", "field3.key3"));
196-
});
197-
DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping);
200+
}), IndexMode.TIME_SERIES);
198201

199202
Exception e = expectThrows(
200203
DocumentParsingException.class,
@@ -203,6 +206,19 @@ public void testDimensionMultiValuedField() throws IOException {
203206
assertThat(e.getCause().getMessage(), containsString("Dimension field [field.key1] cannot be a multi-valued field"));
204207
}
205208

209+
public void testDimensionMultiValuedFieldNonTSDB() throws IOException {
210+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
211+
minimalMapping(b);
212+
b.field("time_series_dimensions", List.of("key1", "key2", "field3.key3"));
213+
}), randomFrom(IndexMode.STANDARD, IndexMode.LOGSDB));
214+
215+
ParsedDocument doc = mapper.parse(source(b -> {
216+
b.array("field.key1", "value1", "value2");
217+
b.field("@timestamp", Instant.now());
218+
}));
219+
assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1)));
220+
}
221+
206222
public void testDisableIndex() throws Exception {
207223

208224
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {

‎test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,14 @@ protected static String randomIndexOptions() {
139139
return randomFrom("docs", "freqs", "positions", "offsets");
140140
}
141141

142+
protected final DocumentMapper createDocumentMapper(XContentBuilder mappings, IndexMode indexMode) throws IOException {
143+
return switch (indexMode) {
144+
case STANDARD -> createDocumentMapper(mappings);
145+
case TIME_SERIES -> createTimeSeriesModeDocumentMapper(mappings);
146+
case LOGSDB -> createLogsModeDocumentMapper(mappings);
147+
};
148+
}
149+
142150
protected final DocumentMapper createDocumentMapper(XContentBuilder mappings) throws IOException {
143151
return createMapperService(mappings).documentMapper();
144152
}

0 commit comments

Comments
 (0)