Browse Source

[ML][Data Frame] removing format support (#43659)

Benjamin Trent 6 years ago
parent
commit
d1ff9818fa

+ 5 - 30
client/rest-high-level/src/main/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java

@@ -45,7 +45,6 @@ import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optiona
 public class DateHistogramGroupSource extends SingleGroupSource implements ToXContentObject {
 
     private static final ParseField TIME_ZONE = new ParseField("time_zone");
-    private static final ParseField FORMAT = new ParseField("format");
 
     // From DateHistogramAggregationBuilder in core, transplanted and modified to a set
     // so we don't need to import a dependency on the class
@@ -195,8 +194,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
                    }
 
                    ZoneId zoneId = (ZoneId) args[3];
-                   String format = (String) args[4];
-                   return new DateHistogramGroupSource(field, interval, format, zoneId);
+                   return new DateHistogramGroupSource(field, interval, zoneId);
                 });
 
     static {
@@ -212,8 +210,6 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
                 return ZoneOffset.ofHours(p.intValue());
             }
         }, TIME_ZONE, ObjectParser.ValueType.LONG);
-
-        PARSER.declareString(optionalConstructorArg(), FORMAT);
     }
 
     public static DateHistogramGroupSource fromXContent(final XContentParser parser) {
@@ -221,13 +217,11 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
     }
 
     private final Interval interval;
-    private final String format;
     private final ZoneId timeZone;
 
-    DateHistogramGroupSource(String field, Interval interval, String format, ZoneId timeZone) {
+    DateHistogramGroupSource(String field, Interval interval, ZoneId timeZone) {
         super(field);
         this.interval = interval;
-        this.format = format;
         this.timeZone = timeZone;
     }
 
@@ -240,10 +234,6 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
         return interval;
     }
 
-    public String getFormat() {
-        return format;
-    }
-
     public ZoneId getTimeZone() {
         return timeZone;
     }
@@ -258,9 +248,6 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
         if (timeZone != null) {
             builder.field(TIME_ZONE.getPreferredName(), timeZone.toString());
         }
-        if (format != null) {
-            builder.field(FORMAT.getPreferredName(), format);
-        }
         builder.endObject();
         return builder;
     }
@@ -279,13 +266,12 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
 
         return Objects.equals(this.field, that.field) &&
                 Objects.equals(this.interval, that.interval) &&
-                Objects.equals(this.timeZone, that.timeZone) &&
-                Objects.equals(this.format, that.format);
+                Objects.equals(this.timeZone, that.timeZone);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(field, interval, timeZone, format);
+        return Objects.hash(field, interval, timeZone);
     }
 
     public static Builder builder() {
@@ -296,7 +282,6 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
 
         private String field;
         private Interval interval;
-        private String format;
         private ZoneId timeZone;
 
         /**
@@ -319,16 +304,6 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
             return this;
         }
 
-        /**
-         * Set the optional String formatting for the time interval.
-         * @param format The format of the output for the time interval key
-         * @return The {@link Builder} with the format set.
-         */
-        public Builder setFormat(String format) {
-            this.format = format;
-            return this;
-        }
-
         /**
          * Sets the time zone to use for this aggregation
          * @param timeZone The zoneId for the timeZone
@@ -340,7 +315,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo
         }
 
         public DateHistogramGroupSource build() {
-            return new DateHistogramGroupSource(field, interval, format, timeZone);
+            return new DateHistogramGroupSource(field, interval, timeZone);
         }
     }
 }

+ 0 - 1
client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java

@@ -39,7 +39,6 @@ public class DateHistogramGroupSourceTests extends AbstractXContentTestCase<Date
         String field = randomAlphaOfLengthBetween(1, 20);
         return new DateHistogramGroupSource(field,
                 randomDateHistogramInterval(),
-                randomBoolean() ? randomAlphaOfLength(10) : null,
                 randomBoolean() ? randomZone() : null);
     }
 

+ 0 - 4
client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java

@@ -44,9 +44,6 @@ public class DateHistogramGroupSourceTests extends AbstractResponseTestCase<
         if (randomBoolean()) {
             dateHistogramGroupSource.setTimeZone(randomZone());
         }
-        if (randomBoolean()) {
-            dateHistogramGroupSource.setFormat(randomAlphaOfLength(10));
-        }
         return dateHistogramGroupSource;
     }
 
@@ -64,7 +61,6 @@ public class DateHistogramGroupSourceTests extends AbstractResponseTestCase<
     protected void assertInstances(DateHistogramGroupSource serverTestInstance,
             org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource clientInstance) {
         assertThat(serverTestInstance.getField(), equalTo(clientInstance.getField()));
-        assertThat(serverTestInstance.getFormat(), equalTo(clientInstance.getFormat()));
         assertSameInterval(serverTestInstance.getInterval(), clientInstance.getInterval());
         assertThat(serverTestInstance.getTimeZone(), equalTo(clientInstance.getTimeZone()));
         assertThat(serverTestInstance.getType().name(), equalTo(clientInstance.getType().name()));

+ 11 - 19
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.core.dataframe.transforms.pivot;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -187,13 +188,11 @@ public class DateHistogramGroupSource extends SingleGroupSource {
 
     private static final String NAME = "data_frame_date_histogram_group";
     private static final ParseField TIME_ZONE = new ParseField("time_zone");
-    private static final ParseField FORMAT = new ParseField("format");
 
     private static final ConstructingObjectParser<DateHistogramGroupSource, Void> STRICT_PARSER = createParser(false);
     private static final ConstructingObjectParser<DateHistogramGroupSource, Void> LENIENT_PARSER = createParser(true);
 
     private final Interval interval;
-    private String format;
     private ZoneId timeZone;
 
     public DateHistogramGroupSource(String field, Interval interval) {
@@ -205,7 +204,10 @@ public class DateHistogramGroupSource extends SingleGroupSource {
         super(in);
         this.interval = readInterval(in);
         this.timeZone = in.readOptionalZoneId();
-        this.format = in.readOptionalString();
+        // Format was optional in 7.2.x, removed in 7.3+
+        if (in.getVersion().before(Version.V_7_3_0)) {
+            in.readOptionalString();
+        }
     }
 
     private static ConstructingObjectParser<DateHistogramGroupSource, Void> createParser(boolean lenient) {
@@ -242,7 +244,6 @@ public class DateHistogramGroupSource extends SingleGroupSource {
             }
         }, TIME_ZONE, ObjectParser.ValueType.LONG);
 
-        parser.declareString(DateHistogramGroupSource::setFormat, FORMAT);
         return parser;
     }
 
@@ -259,14 +260,6 @@ public class DateHistogramGroupSource extends SingleGroupSource {
         return interval;
     }
 
-    public String getFormat() {
-        return format;
-    }
-
-    public void setFormat(String format) {
-        this.format = format;
-    }
-
     public ZoneId getTimeZone() {
         return timeZone;
     }
@@ -280,7 +273,10 @@ public class DateHistogramGroupSource extends SingleGroupSource {
         out.writeOptionalString(field);
         writeInterval(interval, out);
         out.writeOptionalZoneId(timeZone);
-        out.writeOptionalString(format);
+        // Format was optional in 7.2.x, removed in 7.3+
+        if (out.getVersion().before(Version.V_7_3_0)) {
+            out.writeOptionalString(null);
+        }
     }
 
     @Override
@@ -293,9 +289,6 @@ public class DateHistogramGroupSource extends SingleGroupSource {
         if (timeZone != null) {
             builder.field(TIME_ZONE.getPreferredName(), timeZone.toString());
         }
-        if (format != null) {
-            builder.field(FORMAT.getPreferredName(), format);
-        }
         builder.endObject();
         return builder;
     }
@@ -314,13 +307,12 @@ public class DateHistogramGroupSource extends SingleGroupSource {
 
         return Objects.equals(this.field, that.field) &&
             Objects.equals(interval, that.interval) &&
-            Objects.equals(timeZone, that.timeZone) &&
-            Objects.equals(format, that.format);
+            Objects.equals(timeZone, that.timeZone);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(field, interval, timeZone, format);
+        return Objects.hash(field, interval, timeZone);
     }
 
     @Override

+ 16 - 3
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java

@@ -6,6 +6,9 @@
 
 package org.elasticsearch.xpack.core.dataframe.transforms.pivot;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.Writeable.Reader;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
@@ -29,12 +32,22 @@ public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase<D
         if (randomBoolean()) {
             dateHistogramGroupSource.setTimeZone(randomZone());
         }
-        if (randomBoolean()) {
-            dateHistogramGroupSource.setFormat(randomAlphaOfLength(10));
-        }
         return dateHistogramGroupSource;
     }
 
+    public void testBackwardsSerialization() throws IOException {
+        DateHistogramGroupSource groupSource = randomDateHistogramGroupSource();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            output.setVersion(Version.V_7_2_0);
+            groupSource.writeTo(output);
+            try (StreamInput in = output.bytes().streamInput()) {
+                in.setVersion(Version.V_7_2_0);
+                DateHistogramGroupSource streamedGroupSource = new DateHistogramGroupSource(in);
+                assertEquals(groupSource, streamedGroupSource);
+            }
+        }
+    }
+
     @Override
     protected DateHistogramGroupSource doParseInstance(XContentParser parser) throws IOException {
         return DateHistogramGroupSource.fromXContent(parser, false);

+ 4 - 8
x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java

@@ -143,25 +143,21 @@ abstract class DataFrameIntegTestCase extends ESRestTestCase {
 
     protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field,
                                                                                        DateHistogramInterval interval,
-                                                                                       ZoneId zone,
-                                                                                       String format) {
+                                                                                       ZoneId zone) {
         DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder()
             .setField(field)
             .setInterval(new DateHistogramGroupSource.FixedInterval(interval))
-            .setTimeZone(zone)
-            .setFormat(format);
+            .setTimeZone(zone);
         return builder.build();
     }
 
     protected DateHistogramGroupSource createDateHistogramGroupSourceWithCalendarInterval(String field,
                                                                                           DateHistogramInterval interval,
-                                                                                          ZoneId zone,
-                                                                                          String format) {
+                                                                                          ZoneId zone) {
         DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder()
             .setField(field)
             .setInterval(new DateHistogramGroupSource.CalendarInterval(interval))
-            .setTimeZone(zone)
-            .setFormat(format);
+            .setTimeZone(zone);
         return builder.build();
     }
 

+ 2 - 2
x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java

@@ -45,7 +45,7 @@ public class DataFrameTransformIT extends DataFrameIntegTestCase {
         createReviewsIndex(indexName, 100);
 
         Map<String, SingleGroupSource> groups = new HashMap<>();
-        groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null, null));
+        groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null));
         groups.put("by-user", TermsGroupSource.builder().setField("user_id").build());
         groups.put("by-business", TermsGroupSource.builder().setField("business_id").build());
 
@@ -82,7 +82,7 @@ public class DataFrameTransformIT extends DataFrameIntegTestCase {
         createReviewsIndex(indexName, 100);
 
         Map<String, SingleGroupSource> groups = new HashMap<>();
-        groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null, null));
+        groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null));
         groups.put("by-user", TermsGroupSource.builder().setField("user_id").build());
         groups.put("by-business", TermsGroupSource.builder().setField("business_id").build());
 

+ 4 - 4
x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java

@@ -373,7 +373,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase {
             + "   \"group_by\": {"
             + "     \"by_hr\": {"
             + "       \"date_histogram\": {"
-            + "         \"fixed_interval\": \"1h\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd_HH\""
+            + "         \"fixed_interval\": \"1h\",\"field\":\"timestamp\""
             + " } } },"
             + "   \"aggregations\": {"
             + "     \"avg_rating\": {"
@@ -407,7 +407,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase {
         config += " \"pivot\": {"
             + "   \"group_by\": {"
             + "     \"user.id\": {\"terms\": { \"field\": \"user_id\" }},"
-            + "     \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"}}},"
+            + "     \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\"}}},"
             + "   \"aggregations\": {"
             + "     \"user.avg_rating\": {"
             + "       \"avg\": {"
@@ -457,7 +457,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase {
             + " \"pivot\": {"
             + "   \"group_by\": {"
             + "     \"user.id\": {\"terms\": { \"field\": \"user_id\" }},"
-            + "     \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"}}},"
+            + "     \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\"}}},"
             + "   \"aggregations\": {"
             + "     \"user.avg_rating\": {"
             + "       \"avg\": {"
@@ -497,7 +497,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase {
         config +="    \"pivot\": { \n" +
             "        \"group_by\": {\n" +
             "            \"by_day\": {\"date_histogram\": {\n" +
-            "                \"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"\n" +
+            "                \"fixed_interval\": \"1d\",\"field\":\"timestamp\"\n" +
             "            }}\n" +
             "        },\n" +
             "    \n" +

+ 3 - 16
x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/persistence/DataframeIndex.java

@@ -19,8 +19,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.xpack.core.dataframe.DataFrameField;
 import org.elasticsearch.xpack.core.dataframe.DataFrameMessages;
 import org.elasticsearch.xpack.core.dataframe.transforms.DataFrameTransformConfig;
-import org.elasticsearch.xpack.core.dataframe.transforms.pivot.DateHistogramGroupSource;
-import org.elasticsearch.xpack.core.dataframe.transforms.pivot.SingleGroupSource;
 
 import java.io.IOException;
 import java.time.Clock;
@@ -35,9 +33,7 @@ public final class DataframeIndex {
 
     private static final String PROPERTIES = "properties";
     private static final String TYPE = "type";
-    private static final String FORMAT = "format";
     private static final String META = "_meta";
-    private static final String DEFAULT_TIME_FORMAT = "strict_date_optional_time||epoch_millis";
 
     private DataframeIndex() {
     }
@@ -56,7 +52,7 @@ public final class DataframeIndex {
 
         request.mapping(
             SINGLE_MAPPING_NAME,
-            createMappingXContent(mappings, transformConfig.getPivotConfig().getGroupConfig().getGroups(), transformConfig.getId(), clock));
+            createMappingXContent(mappings, transformConfig.getId(), clock));
 
         client.execute(CreateIndexAction.INSTANCE, request, ActionListener.wrap(createIndexResponse -> {
             listener.onResponse(true);
@@ -69,13 +65,12 @@ public final class DataframeIndex {
     }
 
     private static XContentBuilder createMappingXContent(Map<String, String> mappings,
-                                                         Map<String, SingleGroupSource> groupSources,
                                                          String id,
                                                          Clock clock) {
         try {
             XContentBuilder builder = jsonBuilder().startObject();
             builder.startObject(SINGLE_MAPPING_NAME);
-            addProperties(builder, mappings, groupSources);
+            addProperties(builder, mappings);
             addMetaData(builder, id, clock);
             builder.endObject(); // _doc type
             return builder.endObject();
@@ -85,8 +80,7 @@ public final class DataframeIndex {
     }
 
     private static XContentBuilder addProperties(XContentBuilder builder,
-                                                 Map<String, String> mappings,
-                                                 Map<String, SingleGroupSource> groupSources) throws IOException {
+                                                 Map<String, String> mappings) throws IOException {
         builder.startObject(PROPERTIES);
         for (Entry<String, String> field : mappings.entrySet()) {
             String fieldName = field.getKey();
@@ -95,13 +89,6 @@ public final class DataframeIndex {
             builder.startObject(fieldName);
             builder.field(TYPE, fieldType);
 
-            SingleGroupSource groupSource = groupSources.get(fieldName);
-            if (groupSource instanceof DateHistogramGroupSource) {
-                String format = ((DateHistogramGroupSource) groupSource).getFormat();
-                if (format != null) {
-                    builder.field(FORMAT, DEFAULT_TIME_FORMAT + "||" + format);
-                }
-            }
             builder.endObject();
         }
         builder.endObject(); // PROPERTIES