Rename dimension mapping parameter to time_series_dimension (elas…

…tic#78012) This PR renames dimension mapping parameter to time_series_dimension to make it consistent with time_series_metric parameter (elastic#76766) Relates to elastic#74450 and elastic#74014
csoulios · Sep 23, 2021 · e681a7c · e681a7c
1 parent 30772ce
commit e681a7c
Show file tree

Hide file tree

Showing 11 changed files with 104 additions and 66 deletions.
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml
@@ -18,12 +18,14 @@ enable:
                   type: date
                 metricset:
                   type: keyword
+                  time_series_dimension: true
                 k8s:
                   properties:
                     pod:
                       properties:
                         uid:
                           type: keyword
+                          time_series_dimension: true
                         name:
                           type: keyword
                         ip:

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java
@@ -90,14 +90,19 @@ public Builder(String name, ScriptCompiler scriptCompiler, boolean ignoreMalform
                 = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault);
             this.script.precludesParameters(nullValue, ignoreMalformed);
             addScriptValidation(script, indexed, hasDocValues);
-            this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false)
-                .addValidator(v -> {
-                    if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
-                        throw new IllegalArgumentException(
-                            "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true"
-                        );
-                    }
-                });
+            this.dimension = TimeSeriesParams.dimensionParam(m -> toType(m).dimension).addValidator(v -> {
+                if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
+                    throw new IllegalArgumentException(
+                        "Field ["
+                            + TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM
+                            + "] requires that ["
+                            + indexed.name
+                            + "] and ["
+                            + hasDocValues.name
+                            + "] are true"
+                    );
+                }
+            });
         }
 
         Builder nullValue(String nullValue) {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
@@ -126,10 +126,16 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script
             this.script.precludesParameters(nullValue);
             addScriptValidation(script, indexed, hasDocValues);
 
-            this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false).addValidator(v -> {
+            this.dimension = TimeSeriesParams.dimensionParam(m -> toType(m).dimension).addValidator(v -> {
                 if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
                     throw new IllegalArgumentException(
-                        "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true"
+                        "Field ["
+                            + TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM
+                            + "] requires that ["
+                            + indexed.name
+                            + "] and ["
+                            + hasDocValues.name
+                            + "] are true"
                     );
                 }
             }).precludesParameters(normalizer, ignoreAbove);

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -125,17 +125,24 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
             this.nullValue = new Parameter<>("null_value", false, () -> null,
                 (n, c, o) -> o == null ? null : type.parse(o, false), m -> toType(m).nullValue).acceptsNull();
 
-            this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false)
-                .addValidator(v -> {
-                    if (v && EnumSet.of(NumberType.INTEGER, NumberType.LONG, NumberType.BYTE, NumberType.SHORT).contains(type) == false) {
-                        throw new IllegalArgumentException("Parameter [dimension] cannot be set to numeric type [" + type.name + "]");
-                    }
-                    if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
-                        throw new IllegalArgumentException(
-                            "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true"
-                        );
-                    }
-                });
+            this.dimension = TimeSeriesParams.dimensionParam(m -> toType(m).dimension).addValidator(v -> {
+                if (v && EnumSet.of(NumberType.INTEGER, NumberType.LONG, NumberType.BYTE, NumberType.SHORT).contains(type) == false) {
+                    throw new IllegalArgumentException(
+                        "Parameter [" + TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM + "] cannot be set to numeric type [" + type.name + "]"
+                    );
+                }
+                if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
+                    throw new IllegalArgumentException(
+                        "Field ["
+                            + TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM
+                            + "] requires that ["
+                            + indexed.name
+                            + "] and ["
+                            + hasDocValues.name
+                            + "] are true"
+                    );
+                }
+            });
 
             this.metric = TimeSeriesParams.metricParam(m -> toType(m).metricType, MetricType.gauge, MetricType.counter)
                 .addValidator(v -> {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -18,8 +18,10 @@
 public final class TimeSeriesParams {
 
     public static final String TIME_SERIES_METRIC_PARAM = "time_series_metric";
+    public static final String TIME_SERIES_DIMENSION_PARAM = "time_series_dimension";
 
-    private TimeSeriesParams() {}
+    private TimeSeriesParams() {
+    }
 
     public enum MetricType {
         gauge,
@@ -42,4 +44,8 @@ public static FieldMapper.Parameter<MetricType> metricParam(Function<FieldMapper
         ).acceptsNull();
     }
 
+    public static FieldMapper.Parameter<Boolean> dimensionParam(Function<FieldMapper, Boolean> initializer) {
+        return FieldMapper.Parameter.boolParam(TIME_SERIES_DIMENSION_PARAM, false, initializer, false);
+    }
+
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java
@@ -343,7 +343,7 @@ public void testTooManyDimensionFields() {
             for (int i = 0; i <= max; i++) {
                 b.startObject("field" + i)
                     .field("type", randomFrom("ip", "keyword", "long", "integer", "byte", "short"))
-                    .field("dimension", true)
+                    .field("time_series_dimension", true)
                     .endObject();
             }
         })));

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java
@@ -210,33 +210,39 @@ public void testDimensionIndexedAndDocvalues() {
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", false);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", true).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", true).field("doc_values", false);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", true);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", true);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
     }
 
     public void testDimensionMultiValuedField() throws IOException {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true);
+            b.field("time_series_dimension", true);
         }));
 
         Exception e = expectThrows(MapperParsingException.class,

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java
@@ -324,52 +324,52 @@ public void testDimension() throws IOException {
     public void testDimensionAndIgnoreAbove() {
         Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true).field("ignore_above", 2048);
+            b.field("time_series_dimension", true).field("ignore_above", 2048);
         })));
         assertThat(e.getCause().getMessage(),
-            containsString("Field [ignore_above] cannot be set in conjunction with field [dimension]"));
+            containsString("Field [ignore_above] cannot be set in conjunction with field [time_series_dimension]"));
     }
 
     public void testDimensionAndNormalizer() {
         Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true).field("normalizer", "my_normalizer");
+            b.field("time_series_dimension", true).field("normalizer", "my_normalizer");
         })));
         assertThat(e.getCause().getMessage(),
-            containsString("Field [normalizer] cannot be set in conjunction with field [dimension]"));
+            containsString("Field [normalizer] cannot be set in conjunction with field [time_series_dimension]"));
     }
 
     public void testDimensionIndexedAndDocvalues() {
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", false);
             })));
             assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true"));
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", true).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", true).field("doc_values", false);
             })));
             assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true"));
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", true);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", true);
             })));
             assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true"));
         }
     }
 
     public void testDimensionMultiValuedField() throws IOException {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true);
+            b.field("time_series_dimension", true);
         }));
 
         Exception e = expectThrows(MapperParsingException.class,
@@ -381,7 +381,7 @@ public void testDimensionMultiValuedField() throws IOException {
     public void testDimensionExtraLongKeyword() throws IOException {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true);
+            b.field("time_series_dimension", true);
         }));
 
         Exception e = expectThrows(MapperParsingException.class,

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -255,9 +255,9 @@ public void testDimension() throws IOException {
         // dimension = true is not allowed
         Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true);
+            b.field("time_series_dimension", true);
         })));
-        assertThat(e.getCause().getMessage(), containsString("Parameter [dimension] cannot be set"));
+        assertThat(e.getCause().getMessage(), containsString("Parameter [time_series_dimension] cannot be set"));
     }
 
     public void testMetricType() throws IOException {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java
@@ -39,33 +39,39 @@ public void testDimensionIndexedAndDocvalues() {
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", false);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", true).field("doc_values", false);
+                b.field("time_series_dimension", true).field("index", true).field("doc_values", false);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
         {
             Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true).field("index", false).field("doc_values", true);
+                b.field("time_series_dimension", true).field("index", false).field("doc_values", true);
             })));
-            assertThat(e.getCause().getMessage(),
-                containsString("Field [dimension] requires that [index] and [doc_values] are true"));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Field [time_series_dimension] requires that [index] and [doc_values] are true")
+            );
         }
     }
 
     public void testDimensionMultiValuedField() throws IOException {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", true);
+            b.field("time_series_dimension", true);
         }));
 
         Exception e = expectThrows(MapperParsingException.class,

diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
@@ -174,7 +174,7 @@ protected static void assertNoDocValuesField(LuceneDocument doc, String field) {
     protected <T> void assertDimension(boolean isDimension, Function<T, Boolean> checker) throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("dimension", isDimension);
+            b.field("time_series_dimension", isDimension);
         }));
 
         @SuppressWarnings("unchecked") // Syntactic sugar in tests
@@ -546,25 +546,25 @@ protected String randomFetchTestFormat() {
      */
     protected void registerDimensionChecks(ParameterChecker checker) throws IOException {
         // dimension cannot be updated
-        checker.registerConflictCheck("dimension", b -> b.field("dimension", true));
-        checker.registerConflictCheck("dimension", b -> b.field("dimension", false));
-        checker.registerConflictCheck("dimension",
+        checker.registerConflictCheck("time_series_dimension", b -> b.field("time_series_dimension", true));
+        checker.registerConflictCheck("time_series_dimension", b -> b.field("time_series_dimension", false));
+        checker.registerConflictCheck("time_series_dimension",
             fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", false);
+                b.field("time_series_dimension", false);
             }),
             fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true);
+                b.field("time_series_dimension", true);
             }));
-        checker.registerConflictCheck("dimension",
+        checker.registerConflictCheck("time_series_dimension",
             fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", true);
+                b.field("time_series_dimension", true);
             }),
             fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("dimension", false);
+                b.field("time_series_dimension", false);
             }));
     }