From 11d0717cd889fae608745cfb4a9a2c1c24ed71ea Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Wed, 22 May 2019 22:08:42 +0200 Subject: [PATCH 01/14] add support for fixed_interval, calendar_interval, remove interval --- .../pivot/DateHistogramGroupSource.java | 191 +++++++++++++----- .../pivot/DateHistogramGroupSourceTests.java | 15 +- .../integration/DataFrameIntegTestCase.java | 6 +- .../integration/DataFrameTransformIT.java | 2 +- .../integration/DataFramePivotRestIT.java | 19 +- 5 files changed, 164 insertions(+), 69 deletions(-) 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 f4bf094235ae4..56210b548d9c6 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 @@ 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,140 @@ import java.time.ZoneOffset; import java.util.Objects; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; + public class DateHistogramGroupSource extends SingleGroupSource { + public interface Interval extends Writeable, ToXContentFragment { + + } + + 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME, interval.toString()); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.write((byte) 0); + 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME, interval.toString()); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.write((byte) 1); + 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 0: + return new FixedInterval(in); + case 1: + return new CalendarInterval(in); + default: + throw new IllegalArgumentException("unknown type"); + } + } + 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 +163,28 @@ public DateHistogramGroupSource(StreamInput in) throws IOException { 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]; - declareValuesSourceFields(parser); + Interval interval = null; - parser.declareField((histogram, interval) -> { - if (interval instanceof Long) { - histogram.setInterval((long) interval); + if (fixedInterval != null && calendarInterval != null) { + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); + } else if (fixedInterval != null) { + interval = new FixedInterval(new DateHistogramInterval(fixedInterval)); + } else if (calendarInterval != null) { + interval = new CalendarInterval(new DateHistogramInterval(calendarInterval)); } else { - histogram.setDateHistogramInterval((DateHistogramInterval) interval); + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found both"); } - }, p -> { - if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return p.longValue(); - } else { - return new DateHistogramInterval(p.text()); - } - }, HistogramGroupSource.INTERVAL, ObjectParser.ValueType.LONG); + + return new DateHistogramGroupSource(field, interval); + }); + + declareValuesSourceFields(parser); + + 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 +207,10 @@ public Type getType() { 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 +230,9 @@ public void setTimeZone(ZoneId timeZone) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(field); - out.writeLong(interval); - out.writeOptionalWriteable(dateHistogramInterval); + interval.writeTo(out); + //out.writeLong(interval); + //out.writeOptionalWriteable(dateHistogramInterval); out.writeOptionalZoneId(timeZone); out.writeOptionalString(format); } @@ -141,11 +243,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws 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 +268,12 @@ public boolean equals(Object 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); } @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 e9d989d5e5f38..2ae149f5d0e60 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 @@ -8,22 +8,29 @@ import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.AbstractSerializingTestCase; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase { + private static List TIME_UNITS = new ArrayList<>(DateHistogramAggregationBuilder.DATE_FIELD_UNITS.keySet()); + public static DateHistogramGroupSource randomDateHistogramGroupSource() { String field = randomAlphaOfLengthBetween(1, 20); - DateHistogramGroupSource dateHistogramGroupSource = new DateHistogramGroupSource(field); + DateHistogramGroupSource dateHistogramGroupSource; // = new DateHistogramGroupSource(field); if (randomBoolean()) { - dateHistogramGroupSource.setInterval(randomLongBetween(1, 10_000)); + dateHistogramGroupSource = new DateHistogramGroupSource(field, new DateHistogramGroupSource.FixedInterval( + new DateHistogramInterval(randomPositiveTimeValue()))); } else { - dateHistogramGroupSource.setDateHistogramInterval(randomFrom(DateHistogramInterval.days(10), - DateHistogramInterval.minutes(1), DateHistogramInterval.weeks(1))); + dateHistogramGroupSource = new DateHistogramGroupSource(field, new DateHistogramGroupSource.CalendarInterval( + new DateHistogramInterval(randomTimeValue(1,1, "m", "h", "d", "w")))); } + if (randomBoolean()) { dateHistogramGroupSource.setTimeZone(randomZone()); } diff --git a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java index 122cd570ab108..90b9f9d972f6a 100644 --- a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java +++ b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java @@ -135,22 +135,20 @@ protected void waitUntilCheckpoint(String id, long checkpoint, TimeValue waitTim TimeUnit.MILLISECONDS); } - protected DateHistogramGroupSource createDateHistogramGroupSource(String field, long interval, ZoneId zone, String format) { + protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field, DateHistogramInterval interval, ZoneId zone, String format) { DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) - .setFormat(format) .setInterval(interval) .setTimeZone(zone); return builder.build(); } - protected DateHistogramGroupSource createDateHistogramGroupSource(String field, + protected DateHistogramGroupSource createDateHistogramGroupSourceWithCalendarInterval(String field, DateHistogramInterval interval, ZoneId zone, String format) { DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) - .setFormat(format) .setDateHistgramInterval(interval) .setTimeZone(zone); return builder.build(); diff --git a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java index bce4a4a3b503b..6835e3f39b533 100644 --- a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java +++ b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java @@ -34,7 +34,7 @@ public void testDataFrameTransformCrud() throws Exception { createReviewsIndex(); Map 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()); 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 770eaec7bd141..98e765e9862e5 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 @@ -216,7 +216,7 @@ public void testDateHistogramPivot() throws Exception { + " \"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\": {" @@ -226,14 +226,11 @@ public void testDateHistogramPivot() throws Exception { + "}"; 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"); @@ -253,7 +250,7 @@ public void testPreviewTransform() throws Exception { 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\": {" @@ -261,8 +258,6 @@ public void testPreviewTransform() throws Exception { + " } } } }" + "}"; 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"); @@ -290,7 +285,7 @@ public void testPivotWithMaxOnDateField() throws Exception { 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" + @@ -305,13 +300,11 @@ public void testPivotWithMaxOnDateField() throws Exception { + "}"; 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 From 5eb352828f8666564775b0c822b08996e22adecb Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 12:05:36 +0200 Subject: [PATCH 02/14] adapt HLRC --- .../pivot/DateHistogramGroupSource.java | 199 ++++++++++++------ .../pivot/DateHistogramGroupSourceTests.java | 17 +- .../pivot/DateHistogramGroupSource.java | 2 - .../pivot/DateHistogramGroupSourceTests.java | 2 +- 4 files changed, 147 insertions(+), 73 deletions(-) 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 71e7e258c5c8b..35890508781f1 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,10 @@ import java.io.IOException; import java.time.ZoneId; import java.time.ZoneOffset; +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 +46,139 @@ 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; + static { + Set dateFieldUnits = new HashSet<>(); + dateFieldUnits.add("year"); + dateFieldUnits.add("1y"); + dateFieldUnits.add("quarter"); + dateFieldUnits.add("1q"); + dateFieldUnits.add("month"); + dateFieldUnits.add("1M"); + dateFieldUnits.add("week"); + dateFieldUnits.add("1w"); + dateFieldUnits.add("day"); + dateFieldUnits.add("1d"); + dateFieldUnits.add("hour"); + dateFieldUnits.add("1h"); + dateFieldUnits.add("minute"); + dateFieldUnits.add("1m"); + dateFieldUnits.add("second"); + dateFieldUnits.add("1s"); + DATE_FIELD_UNITS = Collections.unmodifiableSet(dateFieldUnits); + } + + public interface Interval extends ToXContentFragment { + + } + + 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME, interval.toString()); + 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(NAME, interval.toString()); + 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 none"); + } 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 both"); } - 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 +194,13 @@ public static DateHistogramGroupSource fromXContent(final XContentParser parser) 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 +210,10 @@ public Type getType() { return Type.DATE_HISTOGRAM; } - public long getInterval() { + public Interval getInterval() { return interval; } - public DateHistogramInterval getDateHistogramInterval() { - return dateHistogramInterval; - } - public String getFormat() { return format; } @@ -124,11 +228,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws 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()); } @@ -153,14 +253,13 @@ public boolean equals(Object 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); } @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 +269,7 @@ public static Builder builder() { 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 +285,14 @@ public Builder setField(String field) { /** * 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 +314,7 @@ public Builder setTimeZone(ZoneId timeZone) { } 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 c6a160d9b8b8d..32605f5c286ad 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 @@ 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/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 56210b548d9c6..d586545106e1e 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 @@ -231,8 +231,6 @@ public void setTimeZone(ZoneId timeZone) { public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(field); interval.writeTo(out); - //out.writeLong(interval); - //out.writeOptionalWriteable(dateHistogramInterval); out.writeOptionalZoneId(timeZone); out.writeOptionalString(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 2ae149f5d0e60..d60a0ed925d32 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 @@ -22,7 +22,7 @@ public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase Date: Thu, 23 May 2019 12:12:51 +0200 Subject: [PATCH 03/14] checkstyle --- .../dataframe/integration/DataFrameIntegTestCase.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java index 90b9f9d972f6a..765037784f790 100644 --- a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java +++ b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java @@ -135,7 +135,7 @@ protected void waitUntilCheckpoint(String id, long checkpoint, TimeValue waitTim TimeUnit.MILLISECONDS); } - protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field, DateHistogramInterval interval, ZoneId zone, String format) { + protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field, DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) .setInterval(interval) @@ -144,9 +144,9 @@ protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterv } protected DateHistogramGroupSource createDateHistogramGroupSourceWithCalendarInterval(String field, - DateHistogramInterval interval, - ZoneId zone, - String format) { + DateHistogramInterval interval, + ZoneId zone, + String format) { DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) .setDateHistgramInterval(interval) From dddfdf398c546eeed1a31f80fa86992e340cf5a5 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 12:56:24 +0200 Subject: [PATCH 04/14] add a hlrc to server test --- .../hlrc/DateHistogramGroupSourceTests.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) 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/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 0000000000000..38fd483bb40bf --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/pivot/hlrc/DateHistogramGroupSourceTests.java @@ -0,0 +1,73 @@ +/* + * 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())); + //assertThat(serverTestInstance.getInterval(), equalTo(clientInstance.getInterval())); + assertThat(serverTestInstance.getTimeZone(), equalTo(clientInstance.getTimeZone())); + //assertThat(serverTestInstance.getType(), equalTo(clientInstance.getType())); + } + +} From 5fa114b88db76e14d9b466d6f5108fcabb407d06 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 12:57:05 +0200 Subject: [PATCH 05/14] adapt yml test --- .../rest-api-spec/test/data_frame/preview_transforms.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) 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 1d4a190b24e14..e7b7021b5bf62 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"}}, @@ -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"}}} } } From a304e26e81561a44f4af5616a4c48833ea2654aa Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 14:23:03 +0200 Subject: [PATCH 06/14] improve naming and doc --- .../pivot/DateHistogramGroupSource.java | 20 +++++++++++++++- .../pivot/DateHistogramGroupSource.java | 24 ++++++++++++++++--- .../pivot/DateHistogramGroupSourceTests.java | 7 +----- 3 files changed, 41 insertions(+), 10 deletions(-) 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 35890508781f1..0268d851d06ed 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 @@ -70,8 +70,16 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo DATE_FIELD_UNITS = Collections.unmodifiableSet(dateFieldUnits); } + /** + * 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(); } public static class FixedInterval implements Interval { @@ -82,6 +90,11 @@ public FixedInterval(DateHistogramInterval interval) { this.interval = interval; } + @Override + public String getName() { + return NAME; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME, interval.toString()); @@ -121,6 +134,11 @@ public CalendarInterval(DateHistogramInterval interval) { } } + @Override + public String getName() { + return NAME; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME, interval.toString()); 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 d586545106e1e..29b11adfcf6ad 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 @@ -27,8 +27,16 @@ public class DateHistogramGroupSource extends SingleGroupSource { + /** + * 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(); } public static class FixedInterval implements Interval { @@ -43,6 +51,11 @@ public FixedInterval(StreamInput in) throws IOException { this.interval = new DateHistogramInterval(in); } + @Override + public String getName() { + return NAME; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME, interval.toString()); @@ -83,7 +96,7 @@ public static class CalendarInterval implements 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 " + + throw new IllegalArgumentException("The supplied interval [" + interval + "] could not be parsed " + "as a calendar interval."); } } @@ -92,6 +105,11 @@ public CalendarInterval(StreamInput in) throws IOException { this.interval = new DateHistogramInterval(in); } + @Override + public String getName() { + return NAME; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME, interval.toString()); @@ -133,7 +151,7 @@ private Interval readInterval(StreamInput in) throws IOException { case 1: return new CalendarInterval(in); default: - throw new IllegalArgumentException("unknown type"); + throw new IllegalArgumentException("unknown interval"); } } 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 d60a0ed925d32..7ce0374331323 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 @@ -8,18 +8,13 @@ import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.AbstractSerializingTestCase; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase { - private static List TIME_UNITS = new ArrayList<>(DateHistogramAggregationBuilder.DATE_FIELD_UNITS.keySet()); - public static DateHistogramGroupSource randomDateHistogramGroupSource() { String field = randomAlphaOfLengthBetween(1, 20); DateHistogramGroupSource dateHistogramGroupSource; @@ -28,7 +23,7 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() { new DateHistogramInterval(randomPositiveTimeValue()))); } else { dateHistogramGroupSource = new DateHistogramGroupSource(field, new DateHistogramGroupSource.CalendarInterval( - new DateHistogramInterval(randomTimeValue(1,1, "m", "h", "d", "w")))); + new DateHistogramInterval(randomTimeValue(1, 1, "m", "h", "d", "w")))); } if (randomBoolean()) { From c4ccab51ba8f7ca88b154f5d9d902a7fc6ef9034 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 15:15:05 +0200 Subject: [PATCH 07/14] improve interface and add test code for hlrc to server --- .../transforms/pivot/DateHistogramGroupSource.java | 11 +++++++++++ .../pivot/hlrc/DateHistogramGroupSourceTests.java | 10 ++++++++-- .../transforms/pivot/DateHistogramGroupSource.java | 11 +++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) 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 0268d851d06ed..1f273b60a81e8 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 @@ -80,6 +80,7 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo */ public interface Interval extends ToXContentFragment { String getName(); + DateHistogramInterval getInterval(); } public static class FixedInterval implements Interval { @@ -95,6 +96,11 @@ 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.toString()); @@ -139,6 +145,11 @@ 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.toString()); 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 index 38fd483bb40bf..dc31004607dcd 100644 --- 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 @@ -65,9 +65,15 @@ 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())); - //assertThat(serverTestInstance.getInterval(), equalTo(clientInstance.getInterval())); + assertSameInterval(serverTestInstance.getInterval(), clientInstance.getInterval()); assertThat(serverTestInstance.getTimeZone(), equalTo(clientInstance.getTimeZone())); - //assertThat(serverTestInstance.getType(), equalTo(clientInstance.getType())); + 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 29b11adfcf6ad..3c4101714d07d 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 @@ -37,6 +37,7 @@ public class DateHistogramGroupSource extends SingleGroupSource { */ public interface Interval extends Writeable, ToXContentFragment { String getName(); + DateHistogramInterval getInterval(); } public static class FixedInterval implements Interval { @@ -56,6 +57,11 @@ 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.toString()); @@ -110,6 +116,11 @@ 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.toString()); From e707c2a125181c78d9426c51ff36327ed8da4812 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 16:38:37 +0200 Subject: [PATCH 08/14] address review comments --- .../pivot/DateHistogramGroupSource.java | 53 +++++++++---------- .../pivot/DateHistogramGroupSource.java | 12 ++--- 2 files changed, 31 insertions(+), 34 deletions(-) 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 1f273b60a81e8..0bd4af6645e75 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 @@ -31,6 +31,7 @@ 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; @@ -48,27 +49,23 @@ public class DateHistogramGroupSource extends SingleGroupSource implements ToXCo // 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; - static { - Set dateFieldUnits = new HashSet<>(); - dateFieldUnits.add("year"); - dateFieldUnits.add("1y"); - dateFieldUnits.add("quarter"); - dateFieldUnits.add("1q"); - dateFieldUnits.add("month"); - dateFieldUnits.add("1M"); - dateFieldUnits.add("week"); - dateFieldUnits.add("1w"); - dateFieldUnits.add("day"); - dateFieldUnits.add("1d"); - dateFieldUnits.add("hour"); - dateFieldUnits.add("1h"); - dateFieldUnits.add("minute"); - dateFieldUnits.add("1m"); - dateFieldUnits.add("second"); - dateFieldUnits.add("1s"); - DATE_FIELD_UNITS = Collections.unmodifiableSet(dateFieldUnits); - } + 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: @@ -103,7 +100,8 @@ public DateHistogramInterval getInterval() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(NAME, interval.toString()); + builder.field(NAME); + interval.toXContent(builder, params); return builder; } @@ -118,7 +116,6 @@ public boolean equals(Object other) { } final FixedInterval that = (FixedInterval) other; - return Objects.equals(this.interval, that.interval); } @@ -135,7 +132,7 @@ public static class CalendarInterval implements 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 " + + throw new IllegalArgumentException("The supplied interval [" + interval + "] could not be parsed " + "as a calendar interval."); } } @@ -152,7 +149,8 @@ public DateHistogramInterval getInterval() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(NAME, interval.toString()); + builder.field(NAME); + interval.toXContent(builder, params); return builder; } @@ -167,7 +165,6 @@ public boolean equals(Object other) { } final CalendarInterval that = (CalendarInterval) other; - return Objects.equals(this.interval, that.interval); } @@ -188,13 +185,13 @@ public int hashCode() { Interval interval = null; if (fixedInterval != null && calendarInterval != null) { - throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); + 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 both"); + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); } ZoneId zoneId = (ZoneId) args[3]; 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 3c4101714d07d..fade2dd00b4b0 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 @@ -64,7 +64,8 @@ public DateHistogramInterval getInterval() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(NAME, interval.toString()); + builder.field(NAME); + interval.toXContent(builder, params); return builder; } @@ -85,7 +86,6 @@ public boolean equals(Object other) { } final FixedInterval that = (FixedInterval) other; - return Objects.equals(this.interval, that.interval); } @@ -123,7 +123,8 @@ public DateHistogramInterval getInterval() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(NAME, interval.toString()); + builder.field(NAME); + interval.toXContent(builder, params); return builder; } @@ -144,7 +145,6 @@ public boolean equals(Object other) { } final CalendarInterval that = (CalendarInterval) other; - return Objects.equals(this.interval, that.interval); } @@ -198,13 +198,13 @@ private static ConstructingObjectParser createPa Interval interval = null; if (fixedInterval != null && calendarInterval != null) { - throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); + 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 both"); + throw new IllegalArgumentException("You must specify either fixed_interval or calendar_interval, found none"); } return new DateHistogramGroupSource(field, interval); From 2647c8e4e64992b496817b5264dbb9c3cca737b7 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Thu, 23 May 2019 23:35:45 +0200 Subject: [PATCH 09/14] repair merge conflict --- .../integration/DataFrameIntegTestCase.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java index 765037784f790..1320b5ad231e8 100644 --- a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java +++ b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameIntegTestCase.java @@ -136,10 +136,14 @@ protected void waitUntilCheckpoint(String id, long checkpoint, TimeValue waitTim } protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field, + DateHistogramInterval interval, + ZoneId zone, + String format) { DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) - .setInterval(interval) - .setTimeZone(zone); + .setInterval(new DateHistogramGroupSource.FixedInterval(interval)) + .setTimeZone(zone) + .setFormat(format); return builder.build(); } @@ -149,8 +153,9 @@ protected DateHistogramGroupSource createDateHistogramGroupSourceWithCalendarInt String format) { DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder() .setField(field) - .setDateHistgramInterval(interval) - .setTimeZone(zone); + .setInterval(new DateHistogramGroupSource.CalendarInterval(interval)) + .setTimeZone(zone) + .setFormat(format); return builder.build(); } From c75f7f9b6599593e188506ba8917b74be788760e Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 08:10:21 +0200 Subject: [PATCH 10/14] fix date patterns --- .../dataframe/integration/DataFramePivotRestIT.java | 6 +++--- .../test/data_frame/preview_transforms.yml | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) 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 98e765e9862e5..22586a7b37d27 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 @@ -216,7 +216,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\",\"format\":\"yyyy-MM-dd_HH\"" + " } } }," + " \"aggregations\": {" + " \"avg_rating\": {" @@ -250,7 +250,7 @@ public void testPreviewTransform() throws Exception { config += " \"pivot\": {" + " \"group_by\": {" + " \"reviewer\": {\"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\",\"format\":\"yyyy-MM-dd\"}}}," + " \"aggregations\": {" + " \"avg_rating\": {" + " \"avg\": {" @@ -285,7 +285,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\",\"format\":\"yyyy-MM-dd\"\n" + " }}\n" + " },\n" + " \n" + 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 e7b7021b5bf62..5e58048b3bf0f 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 @@ -75,7 +75,7 @@ setup: "pivot": { "group_by": { "airline": {"terms": {"field": "airline"}}, - "by-hour": {"date_histogram": {"fixed_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"}}, @@ -84,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" } @@ -123,7 +123,7 @@ setup: "pivot": { "group_by": { "airline": {"terms": {"field": "airline"}}, - "by-hour": {"date_histogram": {"fixed_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"}}} } } From 27104d23d29e22e701161ca0859323724a5b43ba Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 16:31:25 +0200 Subject: [PATCH 11/14] address review comments --- .../pivot/DateHistogramGroupSource.java | 6 +++--- .../pivot/DateHistogramGroupSource.java | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) 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 0bd4af6645e75..d880bfd82140b 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 @@ -278,9 +278,9 @@ public boolean equals(Object other) { final DateHistogramGroupSource that = (DateHistogramGroupSource) 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(this.interval, that.interval) && + Objects.equals(this.timeZone, that.timeZone) && + Objects.equals(this.format, that.format); } @Override 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 fade2dd00b4b0..26b93b860cd57 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 @@ -38,6 +38,7 @@ public class DateHistogramGroupSource extends SingleGroupSource { public interface Interval extends Writeable, ToXContentFragment { String getName(); DateHistogramInterval getInterval(); + byte getIntervalTypeId(); } public static class FixedInterval implements Interval { @@ -62,6 +63,11 @@ public DateHistogramInterval getInterval() { return interval; } + @Override + public byte getIntervalTypeId() { + return 0; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME); @@ -71,7 +77,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { - out.write((byte) 0); interval.writeTo(out); } @@ -121,6 +126,11 @@ public DateHistogramInterval getInterval() { return interval; } + @Override + public byte getIntervalTypeId() { + return 1; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME); @@ -130,7 +140,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { - out.write((byte) 1); interval.writeTo(out); } @@ -166,6 +175,11 @@ private Interval readInterval(StreamInput in) throws IOException { } } + 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"); @@ -259,7 +273,7 @@ public void setTimeZone(ZoneId timeZone) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(field); - interval.writeTo(out); + writeInterval(interval, out); out.writeOptionalZoneId(timeZone); out.writeOptionalString(format); } From 61660c162473628d494a9e1ec4798600ea8d999d Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 17:13:08 +0200 Subject: [PATCH 12/14] remove assert for warning --- .../xpack/dataframe/integration/DataFrameTransformIT.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java index 6835e3f39b533..b98367979bff9 100644 --- a/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java +++ b/x-pack/plugin/data-frame/qa/multi-node-tests/src/test/java/org/elasticsearch/xpack/dataframe/integration/DataFrameTransformIT.java @@ -48,10 +48,8 @@ public void testDataFrameTransformCrud() throws Exception { "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); From 0016ef30794e10ccffa9b9a01efbfe9717b6bc30 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 17:22:33 +0200 Subject: [PATCH 13/14] improve exception message --- .../dataframe/transforms/pivot/DateHistogramGroupSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 26b93b860cd57..bef9cbad44d2a 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 @@ -171,7 +171,7 @@ private Interval readInterval(StreamInput in) throws IOException { case 1: return new CalendarInterval(in); default: - throw new IllegalArgumentException("unknown interval"); + throw new IllegalArgumentException("unknown interval type [" + id + "]"); } } From 42a3981387809bb2e624af5b71023d07caad5b9d Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 24 May 2019 17:47:08 +0200 Subject: [PATCH 14/14] use constants --- .../transforms/pivot/DateHistogramGroupSource.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) 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 bef9cbad44d2a..a3861ef65f648 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 @@ -27,6 +27,9 @@ 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: * @@ -65,7 +68,7 @@ public DateHistogramInterval getInterval() { @Override public byte getIntervalTypeId() { - return 0; + return FIXED_INTERVAL_ID; } @Override @@ -128,7 +131,7 @@ public DateHistogramInterval getInterval() { @Override public byte getIntervalTypeId() { - return 1; + return CALENDAR_INTERVAL_ID; } @Override @@ -166,9 +169,9 @@ public int hashCode() { private Interval readInterval(StreamInput in) throws IOException { byte id = in.readByte(); switch (id) { - case 0: + case FIXED_INTERVAL_ID: return new FixedInterval(in); - case 1: + case CALENDAR_INTERVAL_ID: return new CalendarInterval(in); default: throw new IllegalArgumentException("unknown interval type [" + id + "]");