From 6d47ee92684d4c17eeb2862eda3d60d5fc53d7bf Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 19:54:06 +0200 Subject: [PATCH] [ML-DataFrame] add support for fixed_interval, calendar_interval, remove interval (#42427) * add support for fixed_interval, calendar_interval, remove interval * adapt HLRC * checkstyle * add a hlrc to server test * adapt yml test * improve naming and doc * improve interface and add test code for hlrc to server * address review comments * repair merge conflict * fix date patterns * address review comments * remove assert for warning * improve exception message * use constants --- .../pivot/DateHistogramGroupSource.java | 231 ++++++++++++----- .../pivot/DateHistogramGroupSourceTests.java | 17 +- .../hlrc/DateHistogramGroupSourceTests.java | 79 ++++++ .../pivot/DateHistogramGroupSource.java | 235 ++++++++++++++---- .../pivot/DateHistogramGroupSourceTests.java | 10 +- .../integration/DataFrameIntegTestCase.java | 25 +- .../integration/DataFrameTransformIT.java | 8 +- .../integration/DataFramePivotRestIT.java | 19 +- .../test/data_frame/preview_transforms.yml | 15 +- 9 files changed, 476 insertions(+), 163 deletions(-) create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java index 71e7e258c5c..d880bfd8214 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSource.java @@ -20,9 +20,9 @@ package org.elasticsearch.client.dataframe.transforms.pivot; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,7 +31,11 @@ import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInter import java.io.IOException; import java.time.ZoneId; import java.time.ZoneOffset; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.Objects; +import java.util.Set; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; @@ -43,32 +47,164 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo 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 + private static final Set DATE_FIELD_UNITS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + "year", + "1y", + "quarter", + "1q", + "month", + "1M", + "week", + "1w", + "day", + "1d", + "hour", + "1h", + "minute", + "1m", + "second", + "1s"))); + + /** + * Interval can be specified in 2 ways: + * + * fixed_interval fixed intervals like 1h, 1m, 1d + * calendar_interval calendar aware intervals like 1M, 1Y, ... + * + * Note: data frames do not support the deprecated interval option + */ + public interface Interval extends ToXContentFragment { + String getName(); + DateHistogramInterval getInterval(); + } + + public static class FixedInterval implements Interval { + private static final String NAME = "fixed_interval"; + private final DateHistogramInterval interval; + + public FixedInterval(DateHistogramInterval interval) { + this.interval = interval; + } + + @Override + public String getName() { + return NAME; + } + + @Override + public DateHistogramInterval getInterval() { + return interval; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME); + interval.toXContent(builder, params); + return builder; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + final FixedInterval that = (FixedInterval) other; + return Objects.equals(this.interval, that.interval); + } + + @Override + public int hashCode() { + return Objects.hash(interval); + } + } + + public static class CalendarInterval implements Interval { + private static final String NAME = "calendar_interval"; + private final DateHistogramInterval interval; + + public CalendarInterval(DateHistogramInterval interval) { + this.interval = interval; + if (DATE_FIELD_UNITS.contains(interval.toString()) == false) { + throw new IllegalArgumentException("The supplied interval [" + interval + "] could not be parsed " + + "as a calendar interval."); + } + } + + @Override + public String getName() { + return NAME; + } + + @Override + public DateHistogramInterval getInterval() { + return interval; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME); + interval.toXContent(builder, params); + return builder; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + final CalendarInterval that = (CalendarInterval) other; + return Objects.equals(this.interval, that.interval); + } + + @Override + public int hashCode() { + return Objects.hash(interval); + } + } + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("date_histogram_group_source", true, (args) -> { String field = (String)args[0]; - long interval = 0; - DateHistogramInterval dateHistogramInterval = null; - if (args[1] instanceof Long) { - interval = (Long)args[1]; + String fixedInterval = (String) args[1]; + String calendarInterval = (String) args[2]; + + Interval interval = null; + + if (fixedInterval != null && calendarInterval != null) { + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found both"); + } else if (fixedInterval != null) { + interval = new FixedInterval(new DateHistogramInterval(fixedInterval)); + } else if (calendarInterval != null) { + interval = new CalendarInterval(new DateHistogramInterval(calendarInterval)); } else { - dateHistogramInterval = (DateHistogramInterval) args[1]; + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); } - ZoneId zoneId = (ZoneId) args[2]; - String format = (String) args[3]; - return new DateHistogramGroupSource(field, interval, dateHistogramInterval, format, zoneId); + + ZoneId zoneId = (ZoneId) args[3]; + String format = (String) args[4]; + return new DateHistogramGroupSource(field, interval, format, zoneId); }); static { PARSER.declareString(optionalConstructorArg(), FIELD); - PARSER.declareField(optionalConstructorArg(), p -> { - if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return p.longValue(); - } else { - return new DateHistogramInterval(p.text()); - } - }, HistogramGroupSource.INTERVAL, ObjectParser.ValueType.LONG); + + PARSER.declareString(optionalConstructorArg(), new ParseField(FixedInterval.NAME)); + PARSER.declareString(optionalConstructorArg(), new ParseField(CalendarInterval.NAME)); + PARSER.declareField(optionalConstructorArg(), p -> { if (p.currentToken() == XContentParser.Token.VALUE_STRING) { return ZoneId.of(p.text()); @@ -84,15 +220,13 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo return PARSER.apply(parser, null); } - private final long interval; - private final DateHistogramInterval dateHistogramInterval; + private final Interval interval; private final String format; private final ZoneId timeZone; - DateHistogramGroupSource(String field, long interval, DateHistogramInterval dateHistogramInterval, String format, ZoneId timeZone) { + DateHistogramGroupSource(String field, Interval interval, String format, ZoneId timeZone) { super(field); this.interval = interval; - this.dateHistogramInterval = dateHistogramInterval; this.format = format; this.timeZone = timeZone; } @@ -102,14 +236,10 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo return Type.DATE_HISTOGRAM; } - public long getInterval() { + public Interval getInterval() { return interval; } - public DateHistogramInterval getDateHistogramInterval() { - return dateHistogramInterval; - } - public String getFormat() { return format; } @@ -124,11 +254,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo if (field != null) { builder.field(FIELD.getPreferredName(), field); } - if (dateHistogramInterval == null) { - builder.field(HistogramGroupSource.INTERVAL.getPreferredName(), interval); - } else { - builder.field(HistogramGroupSource.INTERVAL.getPreferredName(), dateHistogramInterval.toString()); - } + interval.toXContent(builder, params); if (timeZone != null) { builder.field(TIME_ZONE.getPreferredName(), timeZone.toString()); } @@ -152,15 +278,14 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo final DateHistogramGroupSource that = (DateHistogramGroupSource) other; return Objects.equals(this.field, that.field) && - Objects.equals(interval, that.interval) && - Objects.equals(dateHistogramInterval, that.dateHistogramInterval) && - Objects.equals(timeZone, that.timeZone) && - Objects.equals(format, that.format); + Objects.equals(this.interval, that.interval) && + Objects.equals(this.timeZone, that.timeZone) && + Objects.equals(this.format, that.format); } @Override public int hashCode() { - return Objects.hash(field, interval, dateHistogramInterval, timeZone, format); + return Objects.hash(field, interval, timeZone, format); } public static Builder builder() { @@ -170,8 +295,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo public static class Builder { private String field; - private long interval = 0; - private DateHistogramInterval dateHistogramInterval; + private Interval interval; private String format; private ZoneId timeZone; @@ -187,41 +311,14 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo /** * Set the interval for the DateHistogram grouping - * @param interval the time interval in milliseconds + * @param interval a fixed or calendar interval * @return the {@link Builder} with the interval set. */ - public Builder setInterval(long interval) { - if (interval < 1) { - throw new IllegalArgumentException("[interval] must be greater than or equal to 1."); - } + public Builder setInterval(Interval interval) { this.interval = interval; return this; } - /** - * Set the interval for the DateHistogram grouping - * @param timeValue The time value to use as the interval - * @return the {@link Builder} with the interval set. - */ - public Builder setInterval(TimeValue timeValue) { - return setInterval(timeValue.getMillis()); - } - - /** - * Sets the interval of the DateHistogram grouping - * - * If this DateHistogramInterval is set, it supersedes the #{@link DateHistogramGroupSource#getInterval()} - * @param dateHistogramInterval the DateHistogramInterval to set - * @return The {@link Builder} with the dateHistogramInterval set. - */ - public Builder setDateHistgramInterval(DateHistogramInterval dateHistogramInterval) { - if (dateHistogramInterval == null) { - throw new IllegalArgumentException("[dateHistogramInterval] must not be null"); - } - this.dateHistogramInterval = dateHistogramInterval; - return this; - } - /** * Set the optional String formatting for the time interval. * @param format The format of the output for the time interval key @@ -243,7 +340,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo } public DateHistogramGroupSource build() { - return new DateHistogramGroupSource(field, interval, dateHistogramInterval, format, timeZone); + return new DateHistogramGroupSource(field, interval, format, timeZone); } } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java index c6a160d9b8b..32605f5c286 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java @@ -27,15 +27,20 @@ import java.io.IOException; public class DateHistogramGroupSourceTests extends AbstractXContentTestCase { + public static DateHistogramGroupSource.Interval randomDateHistogramInterval() { + if (randomBoolean()) { + return new DateHistogramGroupSource.FixedInterval(new DateHistogramInterval(randomPositiveTimeValue())); + } else { + return new DateHistogramGroupSource.CalendarInterval(new DateHistogramInterval(randomTimeValue(1, 1, "m", "h", "d", "w"))); + } + } + public static DateHistogramGroupSource randomDateHistogramGroupSource() { String field = randomAlphaOfLengthBetween(1, 20); - boolean setInterval = randomBoolean(); return new DateHistogramGroupSource(field, - setInterval ? randomLongBetween(1, 10_000) : 0, - setInterval ? null : randomFrom(DateHistogramInterval.days(10), - DateHistogramInterval.minutes(1), DateHistogramInterval.weeks(1)), - randomBoolean() ? randomAlphaOfLength(10) : null, - randomBoolean() ? randomZone() : null); + randomDateHistogramInterval(), + randomBoolean() ? randomAlphaOfLength(10) : null, + randomBoolean() ? randomZone() : null); } @Override diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java new file mode 100644 index 00000000000..dc31004607d --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java @@ -0,0 +1,79 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client.dataframe.transforms.pivot.hlrc; + +import org.elasticsearch.client.AbstractResponseTestCase; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; +import org.elasticsearch.xpack.core.dataframe.transforms.pivot.DateHistogramGroupSource; + +import static org.hamcrest.Matchers.equalTo; + +public class DateHistogramGroupSourceTests extends AbstractResponseTestCase< + DateHistogramGroupSource, + org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource> { + + public static DateHistogramGroupSource randomDateHistogramGroupSource() { + String field = randomAlphaOfLengthBetween(1, 20); + DateHistogramGroupSource dateHistogramGroupSource; // = new DateHistogramGroupSource(field); + if (randomBoolean()) { + dateHistogramGroupSource = new DateHistogramGroupSource(field, new DateHistogramGroupSource.FixedInterval( + new DateHistogramInterval(randomPositiveTimeValue()))); + } else { + dateHistogramGroupSource = new DateHistogramGroupSource(field, new DateHistogramGroupSource.CalendarInterval( + new DateHistogramInterval(randomTimeValue(1,1, "m", "h", "d", "w")))); + } + + if (randomBoolean()) { + dateHistogramGroupSource.setTimeZone(randomZone()); + } + if (randomBoolean()) { + dateHistogramGroupSource.setFormat(randomAlphaOfLength(10)); + } + return dateHistogramGroupSource; + } + + @Override + protected DateHistogramGroupSource createServerTestInstance() { + return randomDateHistogramGroupSource(); + } + + @Override + protected org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource doParseToClientInstance(XContentParser parser) { + return org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource.fromXContent(parser); + } + + @Override + 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())); + } + + private void assertSameInterval(DateHistogramGroupSource.Interval serverTestInstance, + org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource.Interval clientInstance) { + assertEquals(serverTestInstance.getName(), clientInstance.getName()); + assertEquals(serverTestInstance.getInterval(), clientInstance.getInterval()); + } + +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java index f4bf094235a..a3861ef65f6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSource.java @@ -8,10 +8,13 @@ package org.elasticsearch.xpack.core.dataframe.transforms.pivot; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import java.io.IOException; @@ -19,28 +22,186 @@ import java.time.ZoneId; import java.time.ZoneOffset; import java.util.Objects; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; + public class DateHistogramGroupSource extends SingleGroupSource { + private static final int CALENDAR_INTERVAL_ID = 1; + private static final int FIXED_INTERVAL_ID = 0; + + /** + * Interval can be specified in 2 ways: + * + * fixed_interval fixed intervals like 1h, 1m, 1d + * calendar_interval calendar aware intervals like 1M, 1Y, ... + * + * Note: data frames do not support the deprecated interval option + */ + public interface Interval extends Writeable, ToXContentFragment { + String getName(); + DateHistogramInterval getInterval(); + byte getIntervalTypeId(); + } + + public static class FixedInterval implements Interval { + private static final String NAME = "fixed_interval"; + private final DateHistogramInterval interval; + + public FixedInterval(DateHistogramInterval interval) { + this.interval = interval; + } + + public FixedInterval(StreamInput in) throws IOException { + this.interval = new DateHistogramInterval(in); + } + + @Override + public String getName() { + return NAME; + } + + @Override + public DateHistogramInterval getInterval() { + return interval; + } + + @Override + public byte getIntervalTypeId() { + return FIXED_INTERVAL_ID; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME); + interval.toXContent(builder, params); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + interval.writeTo(out); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + final FixedInterval that = (FixedInterval) other; + return Objects.equals(this.interval, that.interval); + } + + @Override + public int hashCode() { + return Objects.hash(interval); + } + } + + public static class CalendarInterval implements Interval { + private static final String NAME = "calendar_interval"; + private final DateHistogramInterval interval; + + public CalendarInterval(DateHistogramInterval interval) { + this.interval = interval; + if (DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval.toString()) == null) { + throw new IllegalArgumentException("The supplied interval [" + interval + "] could not be parsed " + + "as a calendar interval."); + } + } + + public CalendarInterval(StreamInput in) throws IOException { + this.interval = new DateHistogramInterval(in); + } + + @Override + public String getName() { + return NAME; + } + + @Override + public DateHistogramInterval getInterval() { + return interval; + } + + @Override + public byte getIntervalTypeId() { + return CALENDAR_INTERVAL_ID; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME); + interval.toXContent(builder, params); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + interval.writeTo(out); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + final CalendarInterval that = (CalendarInterval) other; + return Objects.equals(this.interval, that.interval); + } + + @Override + public int hashCode() { + return Objects.hash(interval); + } + } + + private Interval readInterval(StreamInput in) throws IOException { + byte id = in.readByte(); + switch (id) { + case FIXED_INTERVAL_ID: + return new FixedInterval(in); + case CALENDAR_INTERVAL_ID: + return new CalendarInterval(in); + default: + throw new IllegalArgumentException("unknown interval type [" + id + "]"); + } + } + + private void writeInterval(Interval interval, StreamOutput out) throws IOException { + out.write(interval.getIntervalTypeId()); + interval.writeTo(out); + } + 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 STRICT_PARSER = createParser(false); private static final ConstructingObjectParser LENIENT_PARSER = createParser(true); - private long interval = 0; - private DateHistogramInterval dateHistogramInterval; + + private final Interval interval; private String format; private ZoneId timeZone; - public DateHistogramGroupSource(String field) { + public DateHistogramGroupSource(String field, Interval interval) { super(field); + this.interval = interval; } public DateHistogramGroupSource(StreamInput in) throws IOException { super(in); - this.interval = in.readLong(); - this.dateHistogramInterval = in.readOptionalWriteable(DateHistogramInterval::new); + this.interval = readInterval(in); this.timeZone = in.readOptionalZoneId(); this.format = in.readOptionalString(); } @@ -48,24 +209,28 @@ public class DateHistogramGroupSource extends SingleGroupSource { private static ConstructingObjectParser createParser(boolean lenient) { ConstructingObjectParser parser = new ConstructingObjectParser<>(NAME, lenient, (args) -> { String field = (String) args[0]; - return new DateHistogramGroupSource(field); + String fixedInterval = (String) args[1]; + String calendarInterval = (String) args[2]; + + Interval interval = null; + + if (fixedInterval != null && calendarInterval != null) { + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found both"); + } else if (fixedInterval != null) { + interval = new FixedInterval(new DateHistogramInterval(fixedInterval)); + } else if (calendarInterval != null) { + interval = new CalendarInterval(new DateHistogramInterval(calendarInterval)); + } else { + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); + } + + return new DateHistogramGroupSource(field, interval); }); declareValuesSourceFields(parser); - parser.declareField((histogram, interval) -> { - if (interval instanceof Long) { - histogram.setInterval((long) interval); - } else { - histogram.setDateHistogramInterval((DateHistogramInterval) interval); - } - }, p -> { - if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return p.longValue(); - } else { - return new DateHistogramInterval(p.text()); - } - }, HistogramGroupSource.INTERVAL, ObjectParser.ValueType.LONG); + parser.declareString(optionalConstructorArg(), new ParseField(FixedInterval.NAME)); + parser.declareString(optionalConstructorArg(), new ParseField(CalendarInterval.NAME)); parser.declareField(DateHistogramGroupSource::setTimeZone, p -> { if (p.currentToken() == XContentParser.Token.VALUE_STRING) { @@ -88,28 +253,10 @@ public class DateHistogramGroupSource extends SingleGroupSource { return Type.DATE_HISTOGRAM; } - public long getInterval() { + public Interval getInterval() { return interval; } - public void setInterval(long interval) { - if (interval < 1) { - throw new IllegalArgumentException("[interval] must be greater than or equal to 1."); - } - this.interval = interval; - } - - public DateHistogramInterval getDateHistogramInterval() { - return dateHistogramInterval; - } - - public void setDateHistogramInterval(DateHistogramInterval dateHistogramInterval) { - if (dateHistogramInterval == null) { - throw new IllegalArgumentException("[dateHistogramInterval] must not be null"); - } - this.dateHistogramInterval = dateHistogramInterval; - } - public String getFormat() { return format; } @@ -129,8 +276,7 @@ public class DateHistogramGroupSource extends SingleGroupSource { @Override public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(field); - out.writeLong(interval); - out.writeOptionalWriteable(dateHistogramInterval); + writeInterval(interval, out); out.writeOptionalZoneId(timeZone); out.writeOptionalString(format); } @@ -141,11 +287,7 @@ public class DateHistogramGroupSource extends SingleGroupSource { if (field != null) { builder.field(FIELD.getPreferredName(), field); } - if (dateHistogramInterval == null) { - builder.field(HistogramGroupSource.INTERVAL.getPreferredName(), interval); - } else { - builder.field(HistogramGroupSource.INTERVAL.getPreferredName(), dateHistogramInterval.toString()); - } + interval.toXContent(builder, params); if (timeZone != null) { builder.field(TIME_ZONE.getPreferredName(), timeZone.toString()); } @@ -170,13 +312,12 @@ public class DateHistogramGroupSource extends SingleGroupSource { return Objects.equals(this.field, that.field) && Objects.equals(interval, that.interval) && - Objects.equals(dateHistogramInterval, that.dateHistogramInterval) && Objects.equals(timeZone, that.timeZone) && Objects.equals(format, that.format); } @Override public int hashCode() { - return Objects.hash(field, interval, dateHistogramInterval, timeZone, format); + return Objects.hash(field, interval, timeZone, format); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java index e9d989d5e5f..7ce03743313 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/DateHistogramGroupSourceTests.java @@ -17,13 +17,15 @@ public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase groups = new HashMap<>(); - groups.put("by-day", createDateHistogramGroupSource("timestamp", DateHistogramInterval.DAY, null, null)); + groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null, null)); groups.put("by-user", TermsGroupSource.builder().setField("user_id").build()); groups.put("by-business", TermsGroupSource.builder().setField("business_id").build()); @@ -49,10 +49,8 @@ public class DataFrameTransformIT extends DataFrameIntegTestCase { "reviews-by-user-business-day", REVIEWS_INDEX_NAME); - final RequestOptions options = - expectWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); - assertTrue(putDataFrameTransform(config, options).isAcknowledged()); - assertTrue(startDataFrameTransform(config.getId(), options).isStarted()); + assertTrue(putDataFrameTransform(config, RequestOptions.DEFAULT).isAcknowledged()); + assertTrue(startDataFrameTransform(config.getId(), RequestOptions.DEFAULT).isStarted()); waitUntilCheckpoint(config.getId(), 1L); diff --git a/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java b/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java index dab7e819881..a0bec6ec13c 100644 --- a/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java +++ b/x-pack/plugin/data-frame/qa/single-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFramePivotRestIT.java @@ -218,7 +218,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { + " \"group_by\": {" + " \"by_hr\": {" + " \"date_histogram\": {" - + " \"interval\": \"1h\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-DD_HH\"" + + " \"fixed_interval\": \"1h\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd_HH\"" + " } } }," + " \"aggregations\": {" + " \"avg_rating\": {" @@ -228,14 +228,11 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { + "}"; createDataframeTransformRequest.setJsonEntity(config); - createDataframeTransformRequest.setOptions(expectWarnings("[interval] on [date_histogram] is deprecated, " + - "use [fixed_interval] or [calendar_interval] in the future.")); Map createDataframeTransformResponse = entityAsMap(client().performRequest(createDataframeTransformRequest)); assertThat(createDataframeTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE)); - startAndWaitForTransform(transformId, dataFrameIndex, BASIC_AUTH_VALUE_DATA_FRAME_ADMIN_WITH_SOME_DATA_ACCESS, - "[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); + startAndWaitForTransform(transformId, dataFrameIndex, BASIC_AUTH_VALUE_DATA_FRAME_ADMIN_WITH_SOME_DATA_ACCESS); assertTrue(indexExists(dataFrameIndex)); Map indexStats = getAsMap(dataFrameIndex + "/_stats"); @@ -255,7 +252,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { config += " \"pivot\": {" + " \"group_by\": {" + " \"reviewer\": {\"terms\": { \"field\": \"user_id\" }}," - + " \"by_day\": {\"date_histogram\": {\"interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-DD\"}}}," + + " \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"}}}," + " \"aggregations\": {" + " \"avg_rating\": {" + " \"avg\": {" @@ -263,8 +260,6 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { + " } } } }" + "}"; createPreviewRequest.setJsonEntity(config); - createPreviewRequest.setOptions(expectWarnings("[interval] on [date_histogram] is deprecated, " + - "use [fixed_interval] or [calendar_interval] in the future.")); Map previewDataframeResponse = entityAsMap(client().performRequest(createPreviewRequest)); List> preview = (List>)previewDataframeResponse.get("preview"); @@ -292,7 +287,7 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { config +=" \"pivot\": { \n" + " \"group_by\": {\n" + " \"by_day\": {\"date_histogram\": {\n" + - " \"interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-DD\"\n" + + " \"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"\n" + " }}\n" + " },\n" + " \n" + @@ -307,13 +302,11 @@ public class DataFramePivotRestIT extends DataFrameRestTestCase { + "}"; createDataframeTransformRequest.setJsonEntity(config); - createDataframeTransformRequest.setOptions(expectWarnings("[interval] on [date_histogram] is deprecated, " + - "use [fixed_interval] or [calendar_interval] in the future.")); + Map createDataframeTransformResponse = entityAsMap(client().performRequest(createDataframeTransformRequest)); assertThat(createDataframeTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE)); - startAndWaitForTransform(transformId, dataFrameIndex, BASIC_AUTH_VALUE_DATA_FRAME_ADMIN_WITH_SOME_DATA_ACCESS, - "[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); + startAndWaitForTransform(transformId, dataFrameIndex, BASIC_AUTH_VALUE_DATA_FRAME_ADMIN_WITH_SOME_DATA_ACCESS); assertTrue(indexExists(dataFrameIndex)); // we expect 21 documents as there shall be 21 days worth of docs diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/data_frame/preview_transforms.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/data_frame/preview_transforms.yml index 1d4a190b24e..5e58048b3bf 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/data_frame/preview_transforms.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/data_frame/preview_transforms.yml @@ -67,12 +67,7 @@ setup: --- "Test preview transform": - - skip: - reason: date histo interval is deprecated - features: "warnings" - do: - warnings: - - "[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future." data_frame.preview_data_frame_transform: body: > { @@ -80,7 +75,7 @@ setup: "pivot": { "group_by": { "airline": {"terms": {"field": "airline"}}, - "by-hour": {"date_histogram": {"interval": "1h", "field": "time", "format": "yyyy-MM-DD HH"}}}, + "by-hour": {"date_histogram": {"fixed_interval": "1h", "field": "time", "format": "yyyy-MM-dd HH"}}}, "aggs": { "avg_response": {"avg": {"field": "responsetime"}}, "time.max": {"max": {"field": "time"}}, @@ -89,17 +84,17 @@ setup: } } - match: { preview.0.airline: foo } - - match: { preview.0.by-hour: "2017-02-49 00" } + - match: { preview.0.by-hour: "2017-02-18 00" } - match: { preview.0.avg_response: 1.0 } - match: { preview.0.time.max: "2017-02-18T00:30:00.000Z" } - match: { preview.0.time.min: "2017-02-18T00:00:00.000Z" } - match: { preview.1.airline: bar } - - match: { preview.1.by-hour: "2017-02-49 01" } + - match: { preview.1.by-hour: "2017-02-18 01" } - match: { preview.1.avg_response: 42.0 } - match: { preview.1.time.max: "2017-02-18T01:00:00.000Z" } - match: { preview.1.time.min: "2017-02-18T01:00:00.000Z" } - match: { preview.2.airline: foo } - - match: { preview.2.by-hour: "2017-02-49 01" } + - match: { preview.2.by-hour: "2017-02-18 01" } - match: { preview.2.avg_response: 42.0 } - match: { preview.2.time.max: "2017-02-18T01:01:00.000Z" } - match: { preview.2.time.min: "2017-02-18T01:01:00.000Z" } @@ -128,7 +123,7 @@ setup: "pivot": { "group_by": { "airline": {"terms": {"field": "airline"}}, - "by-hour": {"date_histogram": {"interval": "1h", "field": "time", "format": "yyyy-MM-DD HH"}}}, + "by-hour": {"date_histogram": {"fixed_interval": "1h", "field": "time", "format": "yyyy-MM-dd HH"}}}, "aggs": {"avg_response": {"avg": {"field": "responsetime"}}} } }