Skip to content

Commit

Permalink
Revert Rounding API visibility changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Ankit Jain <[email protected]>
  • Loading branch information
@jainankitk
jainankitk committed Nov 30, 2023
1 parent 6fdc854 commit 57b5cdf
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 53 deletions.
74 changes: 48 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 @@ -55,6 +55,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 @@ -65,6 +66,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 @@ -99,7 +101,7 @@ long roundFloor(long utcMillis) {
}

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

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

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

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

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

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

public long extraLocalOffsetLookup() {
long extraLocalOffsetLookup() {
return ratio;
}
},
Expand Down Expand Up @@ -217,7 +219,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 @@ -488,7 +490,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 @@ -523,14 +525,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 @@ -961,7 +955,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 Down Expand Up @@ -992,14 +986,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 @@ -1399,4 +1385,40 @@ public static Rounding read(StreamInput in) throws IOException {
throw new OpenSearchException("unknown rounding id [" + id + "]");
}
}

/**
* Extracts the interval value from the {@link Rounding} instance
* @param rounding {@link Rounding} instance
* @return the interval value from the {@link Rounding} instance or {@code OptionalLong.empty()}
* if the interval is not available
*/
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();

Check warning on line 1408 in server/src/main/java/org/opensearch/common/Rounding.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/common/Rounding.java#L1408 

Added line #L1408 was not covered by tests
}
} else {
return OptionalLong.empty();

Check warning on line 1411 in server/src/main/java/org/opensearch/common/Rounding.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/common/Rounding.java#L1411 

Added line #L1411 was not covered by tests
}

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 57b5cdf

Please sign in to comment.