Skip to content

Commit

Permalink
Revert Rounding API visibility changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
  • Loading branch information
@reta
reta committed Nov 29, 2023
1 parent 6bc0cd6 commit 998db2a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 53 deletions.
68 changes: 42 additions & 26 deletions server/src/main/java/org/opensearch/common/Rounding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.format.TextStyle;
import java.time.temporal.ChronoField;
import java.time.temporal.ChronoUnit;
import java.time.temporal.IsoFields;
Expand All @@ -64,6 +65,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.OptionalLong;
import java.util.concurrent.TimeUnit;

/**
Expand Down Expand Up @@ -98,7 +100,7 @@ long roundFloor(long utcMillis) {
}

@Override
public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return extraLocalOffsetLookup;
}
},
Expand All @@ -109,7 +111,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundYear(utcMillis);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return extraLocalOffsetLookup;
}
},
Expand All @@ -120,7 +122,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundQuarterOfYear(utcMillis);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return extraLocalOffsetLookup;
}
},
Expand All @@ -131,7 +133,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundMonthOfYear(utcMillis);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return extraLocalOffsetLookup;
}
},
Expand All @@ -140,7 +142,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, this.ratio);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return ratio;
}
},
Expand All @@ -149,7 +151,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, ratio);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return ratio;
}
},
Expand All @@ -164,7 +166,7 @@ long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, ratio);
}

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return ratio;
}
},
Expand Down Expand Up @@ -216,7 +218,7 @@ public long extraLocalOffsetLookup() {
* look up so that we can see transitions that we might have rounded
* down beyond.
*/
public abstract long extraLocalOffsetLookup();
abstract long extraLocalOffsetLookup();

public byte getId() {
return id;
Expand Down Expand Up @@ -487,7 +489,7 @@ public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
*
* @opensearch.internal
*/
public static class TimeUnitRounding extends Rounding {
static class TimeUnitRounding extends Rounding {
static final byte ID = 1;

private final DateTimeUnit unit;
Expand Down Expand Up @@ -515,14 +517,6 @@ public byte id() {
return ID;
}

public DateTimeUnit getUnit() {
return this.unit;
}

public ZoneId getTimeZone() {
return this.timeZone;
}

private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) {
switch (unit) {
case SECOND_OF_MINUTE:
Expand Down Expand Up @@ -953,7 +947,7 @@ public final long nextRoundingValue(long utcMillis) {
*
* @opensearch.internal
*/
public static class TimeIntervalRounding extends Rounding {
static class TimeIntervalRounding extends Rounding {
static final byte ID = 2;

private final long interval;
Expand All @@ -980,14 +974,6 @@ public byte id() {
return ID;
}

public long getInterval() {
return this.interval;
}

public ZoneId getTimeZone() {
return this.timeZone;
}

@Override
public Prepared prepare(long minUtcMillis, long maxUtcMillis) {
long minLookup = minUtcMillis - interval;
Expand Down Expand Up @@ -1384,4 +1370,34 @@ public static Rounding read(StreamInput in) throws IOException {
throw new OpenSearchException("unknown rounding id [" + id + "]");
}
}

public static OptionalLong getInterval(Rounding rounding) {
long interval = 0;

if (rounding instanceof TimeUnitRounding) {
interval = (((TimeUnitRounding) rounding).unit).extraLocalOffsetLookup();
if (!isUTCTimeZone(((TimeUnitRounding) rounding).timeZone)) {
// Fast filter aggregation cannot be used if it needs time zone rounding
return OptionalLong.empty();
}
} else if (rounding instanceof TimeIntervalRounding) {
interval = ((TimeIntervalRounding) rounding).interval;
if (!isUTCTimeZone(((TimeIntervalRounding) rounding).timeZone)) {
// Fast filter aggregation cannot be used if it needs time zone rounding
return OptionalLong.empty();
}
} else {
return OptionalLong.empty();
}

return OptionalLong.of(interval);
}

/**
* Helper function for checking if the time zone requested for date histogram
* aggregation is utc or not
*/
private static boolean isUTCTimeZone(final ZoneId zoneId) {
return "Z".equals(zoneId.getDisplayName(TextStyle.FULL, Locale.ENGLISH));
}
}
32 changes: 5 additions & 27 deletions ...rc/main/java/org/opensearch/search/aggregations/bucket/histogram/FilterRewriteHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,10 @@
import org.opensearch.search.internal.SearchContext;

import java.io.IOException;
import java.time.ZoneId;
import java.time.format.TextStyle;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.OptionalLong;
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.function.Supplier;
Expand All @@ -58,7 +56,7 @@ public FilterContext(DateFieldMapper.DateFieldType fieldType, Weight[] filters)
}

private static final int MAX_NUM_FILTER_BUCKETS = 1024;
private static final Map<Class, Function<Query, Query>> queryWrappers;
private static final Map<Class<?>, Function<Query, Query>> queryWrappers;

// Initialize the wrappers map for unwrapping the query
static {
Expand Down Expand Up @@ -122,14 +120,6 @@ static long[] getAggregationBounds(final SearchContext context, final String fie
return null;
}

/**
* Helper function for checking if the time zone requested for date histogram
* aggregation is utc or not
*/
private static boolean isUTCTimeZone(final ZoneId zoneId) {
return "Z".equals(zoneId.getDisplayName(TextStyle.FULL, Locale.ENGLISH));
}

/**
* Creates the range query filters for aggregations using the interval, min/max
* bounds and the rounding values
Expand All @@ -143,24 +133,12 @@ private static Weight[] createFilterForAggregations(
final long low,
final long high
) throws IOException {
long interval;
if (rounding instanceof Rounding.TimeUnitRounding) {
interval = (((Rounding.TimeUnitRounding) rounding).getUnit()).extraLocalOffsetLookup();
if (!isUTCTimeZone(((Rounding.TimeUnitRounding) rounding).getTimeZone())) {
// Fast filter aggregation cannot be used if it needs time zone rounding
return null;
}
} else if (rounding instanceof Rounding.TimeIntervalRounding) {
interval = ((Rounding.TimeIntervalRounding) rounding).getInterval();
if (!isUTCTimeZone(((Rounding.TimeIntervalRounding) rounding).getTimeZone())) {
// Fast filter aggregation cannot be used if it needs time zone rounding
return null;
}
} else {
// Unexpected scenario, exit and fall back to original
final OptionalLong intervalOpt = Rounding.getInterval(rounding);
if (intervalOpt.isEmpty()) {
return null;
}

final long interval = intervalOpt.getAsLong();
// Calculate the number of buckets using range and interval
long roundedLow = preparedRounding.round(fieldType.convertNanosToMillis(low));
long prevRounded = roundedLow;
Expand Down

0 comments on commit 998db2a

Please sign in to comment.