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

benwtrent · Jun 28, 2019 · ab174a8 · ab174a8
1 parent 74dd6e4
commit ab174a8
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 85 deletions.
diff --git a/...in/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java b/...in/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java
@@ -45,7 +45,6 @@
 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 int hashCode() {
                    }
 
                    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,22 +210,18 @@ public int hashCode() {
                 return ZoneOffset.ofHours(p.intValue());
             }
         }, TIME_ZONE, ObjectParser.ValueType.LONG);
-
-        PARSER.declareString(optionalConstructorArg(), FORMAT);
     }
 
     public static DateHistogramGroupSource fromXContent(final XContentParser parser) {
         return PARSER.apply(parser, null);
     }
 
     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 Interval getInterval() {
         return interval;
     }
 
-    public String getFormat() {
-        return format;
-    }
-
     public ZoneId getTimeZone() {
         return timeZone;
     }
@@ -258,9 +248,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         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 boolean equals(Object other) {
 
         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 static class Builder {
 
         private String field;
         private Interval interval;
-        private String format;
         private ZoneId timeZone;
 
         /**
@@ -319,16 +304,6 @@ public Builder setInterval(Interval interval) {
             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 Builder setTimeZone(ZoneId timeZone) {
         }
 
         public DateHistogramGroupSource build() {
-            return new DateHistogramGroupSource(field, interval, format, timeZone);
+            return new DateHistogramGroupSource(field, interval, timeZone);
         }
     }
 }
diff --git a/...va/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java b/...va/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java
@@ -39,7 +39,6 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() {
         String field = randomAlphaOfLengthBetween(1, 20);
         return new DateHistogramGroupSource(field,
                 randomDateHistogramInterval(),
-                randomBoolean() ? randomAlphaOfLength(10) : null,
                 randomBoolean() ? randomZone() : null);
     }
 

diff --git a/...g/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java b/...g/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java
@@ -44,9 +44,6 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() {
         if (randomBoolean()) {
             dateHistogramGroupSource.setTimeZone(randomZone());
         }
-        if (randomBoolean()) {
-            dateHistogramGroupSource.setFormat(randomAlphaOfLength(10));
-        }
         return dateHistogramGroupSource;
     }
 
@@ -64,7 +61,6 @@ protected org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroup
     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()));

diff --git a/...ava/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java b/...ava/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 @@ private void writeInterval(Interval interval, StreamOutput out) throws IOExcepti
 
     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 DateHistogramGroupSource(StreamInput in) throws IOException {
         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 @@ private static ConstructingObjectParser<DateHistogramGroupSource, Void> createPa
             }
         }, TIME_ZONE, ObjectParser.ValueType.LONG);
 
-        parser.declareString(DateHistogramGroupSource::setFormat, FORMAT);
         return parser;
     }
 
@@ -259,14 +260,6 @@ public Interval getInterval() {
         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 void writeTo(StreamOutput out) throws IOException {
         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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         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 boolean equals(Object other) {
 
         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

diff --git a/...rg/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java b/...rg/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 static DateHistogramGroupSource randomDateHistogramGroupSource() {
         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);

diff --git a/...s/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java b/...s/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java
@@ -143,25 +143,21 @@ protected void waitUntilCheckpoint(String id, long checkpoint, TimeValue waitTim
 
     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();
     }
 

diff --git a/...sts/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java b/...sts/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java
@@ -45,7 +45,7 @@ public void testDataFrameTransformCrud() throws Exception {
         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 void testContinuousDataFrameTransformCrud() throws Exception {
         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());
 

diff --git a/...sts/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java b/...sts/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java
@@ -373,7 +373,7 @@ public void testDateHistogramPivot() throws Exception {
             + "   \"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 void testPreviewTransform() throws Exception {
         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 void testPreviewTransformWithPipeline() throws Exception {
             + " \"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 void testPivotWithMaxOnDateField() throws Exception {
         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" +