Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML][Data Frame] removing format support in date_histogram group_by #43659

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
public class DateHistogramGroupSource extends SingleGroupSource implements ToXContentObject {

private static final ParseField TIME_ZONE = new ParseField("time_zone");
private static final ParseField FORMAT = new ParseField("format");

// From DateHistogramAggregationBuilder in core, transplanted and modified to a set
// so we don't need to import a dependency on the class
Expand Down Expand Up @@ -195,8 +194,7 @@ public int hashCode() {
}

ZoneId zoneId = (ZoneId) args[3];
String format = (String) args[4];
return new DateHistogramGroupSource(field, interval, format, zoneId);
return new DateHistogramGroupSource(field, interval, zoneId);
});

static {
Expand All @@ -212,22 +210,18 @@ public int hashCode() {
return ZoneOffset.ofHours(p.intValue());
}
}, TIME_ZONE, ObjectParser.ValueType.LONG);

PARSER.declareString(optionalConstructorArg(), FORMAT);
}

public static DateHistogramGroupSource fromXContent(final XContentParser parser) {
return PARSER.apply(parser, null);
}

private final Interval interval;
private final String format;
private final ZoneId timeZone;

DateHistogramGroupSource(String field, Interval interval, String format, ZoneId timeZone) {
DateHistogramGroupSource(String field, Interval interval, ZoneId timeZone) {
super(field);
this.interval = interval;
this.format = format;
this.timeZone = timeZone;
}

Expand All @@ -240,10 +234,6 @@ public Interval getInterval() {
return interval;
}

public String getFormat() {
return format;
}

public ZoneId getTimeZone() {
return timeZone;
}
Expand All @@ -258,9 +248,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (timeZone != null) {
builder.field(TIME_ZONE.getPreferredName(), timeZone.toString());
}
if (format != null) {
builder.field(FORMAT.getPreferredName(), format);
}
builder.endObject();
return builder;
}
Expand All @@ -279,13 +266,12 @@ public boolean equals(Object other) {

return Objects.equals(this.field, that.field) &&
Objects.equals(this.interval, that.interval) &&
Objects.equals(this.timeZone, that.timeZone) &&
Objects.equals(this.format, that.format);
Objects.equals(this.timeZone, that.timeZone);
}

@Override
public int hashCode() {
return Objects.hash(field, interval, timeZone, format);
return Objects.hash(field, interval, timeZone);
}

public static Builder builder() {
Expand All @@ -296,7 +282,6 @@ public static class Builder {

private String field;
private Interval interval;
private String format;
private ZoneId timeZone;

/**
Expand All @@ -319,16 +304,6 @@ public Builder setInterval(Interval interval) {
return this;
}

/**
* Set the optional String formatting for the time interval.
* @param format The format of the output for the time interval key
* @return The {@link Builder} with the format set.
*/
public Builder setFormat(String format) {
this.format = format;
return this;
}

/**
* Sets the time zone to use for this aggregation
* @param timeZone The zoneId for the timeZone
Expand All @@ -340,7 +315,7 @@ public Builder setTimeZone(ZoneId timeZone) {
}

public DateHistogramGroupSource build() {
return new DateHistogramGroupSource(field, interval, format, timeZone);
return new DateHistogramGroupSource(field, interval, timeZone);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() {
String field = randomAlphaOfLengthBetween(1, 20);
return new DateHistogramGroupSource(field,
randomDateHistogramInterval(),
randomBoolean() ? randomAlphaOfLength(10) : null,
randomBoolean() ? randomZone() : null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() {
if (randomBoolean()) {
dateHistogramGroupSource.setTimeZone(randomZone());
}
if (randomBoolean()) {
dateHistogramGroupSource.setFormat(randomAlphaOfLength(10));
}
return dateHistogramGroupSource;
}

Expand All @@ -64,7 +61,6 @@ protected org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroup
protected void assertInstances(DateHistogramGroupSource serverTestInstance,
org.elasticsearch.client.dataframe.transforms.pivot.DateHistogramGroupSource clientInstance) {
assertThat(serverTestInstance.getField(), equalTo(clientInstance.getField()));
assertThat(serverTestInstance.getFormat(), equalTo(clientInstance.getFormat()));
assertSameInterval(serverTestInstance.getInterval(), clientInstance.getInterval());
assertThat(serverTestInstance.getTimeZone(), equalTo(clientInstance.getTimeZone()));
assertThat(serverTestInstance.getType().name(), equalTo(clientInstance.getType().name()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.core.dataframe.transforms.pivot;

import org.elasticsearch.Version;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -187,13 +188,11 @@ private void writeInterval(Interval interval, StreamOutput out) throws IOExcepti

private static final String NAME = "data_frame_date_histogram_group";
private static final ParseField TIME_ZONE = new ParseField("time_zone");
private static final ParseField FORMAT = new ParseField("format");

private static final ConstructingObjectParser<DateHistogramGroupSource, Void> STRICT_PARSER = createParser(false);
private static final ConstructingObjectParser<DateHistogramGroupSource, Void> LENIENT_PARSER = createParser(true);

private final Interval interval;
private String format;
private ZoneId timeZone;

public DateHistogramGroupSource(String field, Interval interval) {
Expand All @@ -205,7 +204,10 @@ public DateHistogramGroupSource(StreamInput in) throws IOException {
super(in);
this.interval = readInterval(in);
this.timeZone = in.readOptionalZoneId();
this.format = in.readOptionalString();
// Format was optional in 7.2.x, removed in 7.3+
if (in.getVersion().before(Version.V_7_3_0)) {
in.readOptionalString();
}
}

private static ConstructingObjectParser<DateHistogramGroupSource, Void> createParser(boolean lenient) {
Expand Down Expand Up @@ -242,7 +244,6 @@ private static ConstructingObjectParser<DateHistogramGroupSource, Void> createPa
}
}, TIME_ZONE, ObjectParser.ValueType.LONG);

parser.declareString(DateHistogramGroupSource::setFormat, FORMAT);
return parser;
}

Expand All @@ -259,14 +260,6 @@ public Interval getInterval() {
return interval;
}

public String getFormat() {
return format;
}

public void setFormat(String format) {
this.format = format;
}

public ZoneId getTimeZone() {
return timeZone;
}
Expand All @@ -280,7 +273,10 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(field);
writeInterval(interval, out);
out.writeOptionalZoneId(timeZone);
out.writeOptionalString(format);
// Format was optional in 7.2.x, removed in 7.3+
if (out.getVersion().before(Version.V_7_3_0)) {
out.writeOptionalString(null);
}
}

@Override
Expand All @@ -293,9 +289,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (timeZone != null) {
builder.field(TIME_ZONE.getPreferredName(), timeZone.toString());
}
if (format != null) {
builder.field(FORMAT.getPreferredName(), format);
}
builder.endObject();
return builder;
}
Expand All @@ -314,13 +307,12 @@ public boolean equals(Object other) {

return Objects.equals(this.field, that.field) &&
Objects.equals(interval, that.interval) &&
Objects.equals(timeZone, that.timeZone) &&
Objects.equals(format, that.format);
Objects.equals(timeZone, that.timeZone);
}

@Override
public int hashCode() {
return Objects.hash(field, interval, timeZone, format);
return Objects.hash(field, interval, timeZone);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

package org.elasticsearch.xpack.core.dataframe.transforms.pivot;

import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.Writeable.Reader;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
Expand All @@ -29,12 +32,22 @@ public static DateHistogramGroupSource randomDateHistogramGroupSource() {
if (randomBoolean()) {
dateHistogramGroupSource.setTimeZone(randomZone());
}
if (randomBoolean()) {
dateHistogramGroupSource.setFormat(randomAlphaOfLength(10));
}
return dateHistogramGroupSource;
}

public void testBackwardsSerialization() throws IOException {
DateHistogramGroupSource groupSource = randomDateHistogramGroupSource();
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.setVersion(Version.V_7_2_0);
groupSource.writeTo(output);
try (StreamInput in = output.bytes().streamInput()) {
in.setVersion(Version.V_7_2_0);
DateHistogramGroupSource streamedGroupSource = new DateHistogramGroupSource(in);
assertEquals(groupSource, streamedGroupSource);
}
}
}

@Override
protected DateHistogramGroupSource doParseInstance(XContentParser parser) throws IOException {
return DateHistogramGroupSource.fromXContent(parser, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,25 +143,21 @@ protected void waitUntilCheckpoint(String id, long checkpoint, TimeValue waitTim

protected DateHistogramGroupSource createDateHistogramGroupSourceWithFixedInterval(String field,
DateHistogramInterval interval,
ZoneId zone,
String format) {
ZoneId zone) {
DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder()
.setField(field)
.setInterval(new DateHistogramGroupSource.FixedInterval(interval))
.setTimeZone(zone)
.setFormat(format);
.setTimeZone(zone);
return builder.build();
}

protected DateHistogramGroupSource createDateHistogramGroupSourceWithCalendarInterval(String field,
DateHistogramInterval interval,
ZoneId zone,
String format) {
ZoneId zone) {
DateHistogramGroupSource.Builder builder = DateHistogramGroupSource.builder()
.setField(field)
.setInterval(new DateHistogramGroupSource.CalendarInterval(interval))
.setTimeZone(zone)
.setFormat(format);
.setTimeZone(zone);
return builder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void testDataFrameTransformCrud() throws Exception {
createReviewsIndex(indexName, 100);

Map<String, SingleGroupSource> groups = new HashMap<>();
groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null, null));
groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null));
groups.put("by-user", TermsGroupSource.builder().setField("user_id").build());
groups.put("by-business", TermsGroupSource.builder().setField("business_id").build());

Expand Down Expand Up @@ -82,7 +82,7 @@ public void testContinuousDataFrameTransformCrud() throws Exception {
createReviewsIndex(indexName, 100);

Map<String, SingleGroupSource> groups = new HashMap<>();
groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null, null));
groups.put("by-day", createDateHistogramGroupSourceWithCalendarInterval("timestamp", DateHistogramInterval.DAY, null));
groups.put("by-user", TermsGroupSource.builder().setField("user_id").build());
groups.put("by-business", TermsGroupSource.builder().setField("business_id").build());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ public void testDateHistogramPivot() throws Exception {
+ " \"group_by\": {"
+ " \"by_hr\": {"
+ " \"date_histogram\": {"
+ " \"fixed_interval\": \"1h\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd_HH\""
+ " \"fixed_interval\": \"1h\",\"field\":\"timestamp\""
+ " } } },"
+ " \"aggregations\": {"
+ " \"avg_rating\": {"
Expand Down Expand Up @@ -407,7 +407,7 @@ public void testPreviewTransform() throws Exception {
config += " \"pivot\": {"
+ " \"group_by\": {"
+ " \"user.id\": {\"terms\": { \"field\": \"user_id\" }},"
+ " \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"}}},"
+ " \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\"}}},"
+ " \"aggregations\": {"
+ " \"user.avg_rating\": {"
+ " \"avg\": {"
Expand Down Expand Up @@ -457,7 +457,7 @@ public void testPreviewTransformWithPipeline() throws Exception {
+ " \"pivot\": {"
+ " \"group_by\": {"
+ " \"user.id\": {\"terms\": { \"field\": \"user_id\" }},"
+ " \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"}}},"
+ " \"by_day\": {\"date_histogram\": {\"fixed_interval\": \"1d\",\"field\":\"timestamp\"}}},"
+ " \"aggregations\": {"
+ " \"user.avg_rating\": {"
+ " \"avg\": {"
Expand Down Expand Up @@ -497,7 +497,7 @@ public void testPivotWithMaxOnDateField() throws Exception {
config +=" \"pivot\": { \n" +
" \"group_by\": {\n" +
" \"by_day\": {\"date_histogram\": {\n" +
" \"fixed_interval\": \"1d\",\"field\":\"timestamp\",\"format\":\"yyyy-MM-dd\"\n" +
" \"fixed_interval\": \"1d\",\"field\":\"timestamp\"\n" +
" }}\n" +
" },\n" +
" \n" +
Expand Down
Loading