epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629895942
########## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ########## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import org.apache.iceberg.util.SerializableFunction; -enum Timestamps implements Transform<Long, Integer> { - YEAR(ChronoUnit.YEARS, "year"), - MONTH(ChronoUnit.MONTHS, "month"), - DAY(ChronoUnit.DAYS, "day"), - HOUR(ChronoUnit.HOURS, "hour"); +class Timestamps implements Transform<Long, Integer> { + + static final Timestamps YEAR_FROM_MICROS = + new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.YEARS, "year"); + static final Timestamps MONTH_FROM_MICROS = + new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.MONTHS, "month"); + static final Timestamps DAY_FROM_MICROS = + new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.DAYS, "day"); + static final Timestamps HOUR_FROM_MICROS = + new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.HOURS, "hour"); + static final Timestamps YEAR_FROM_NANOS = + new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.YEARS, "year"); + static final Timestamps MONTH_FROM_NANOS = + new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.MONTHS, "month"); + static final Timestamps DAY_FROM_NANOS = + new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.DAYS, "day"); + static final Timestamps HOUR_FROM_NANOS = + new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.HOURS, "hour"); + + static Timestamps get(Types.TimestampType type, String resultTypeUnit) { + switch (resultTypeUnit.toLowerCase(Locale.ENGLISH)) { + case "year": + return get(type, ChronoUnit.YEARS); + case "month": + return get(type, ChronoUnit.MONTHS); + case "day": + return get(type, ChronoUnit.DAYS); + case "hour": + return get(type, ChronoUnit.HOURS); + default: + throw new IllegalArgumentException( + "Unsupported source/result type units: " + type + "->" + resultTypeUnit); + } + } + + static Timestamps get(Types.TimestampNanoType type, String resultTypeUnit) { + switch (resultTypeUnit.toLowerCase(Locale.ENGLISH)) { + case "year": + return get(type, ChronoUnit.YEARS); + case "month": + return get(type, ChronoUnit.MONTHS); + case "day": + return get(type, ChronoUnit.DAYS); + case "hour": + return get(type, ChronoUnit.HOURS); + default: + throw new IllegalArgumentException( + "Unsupported source/result type units: " + type + "->" + resultTypeUnit); + } + } + + static Timestamps get(Types.TimestampType type, ChronoUnit resultTypeUnit) { + switch (resultTypeUnit) { + case YEARS: + return YEAR_FROM_MICROS; + case MONTHS: + return MONTH_FROM_MICROS; + case DAYS: + return DAY_FROM_MICROS; + case HOURS: + return HOUR_FROM_MICROS; + default: + throw new IllegalArgumentException( + "Unsupported source/result type units: " + type + "->" + resultTypeUnit); + } + } + + static Timestamps get(Types.TimestampNanoType type, ChronoUnit resultTypeUnit) { + switch (resultTypeUnit) { + case YEARS: + return YEAR_FROM_NANOS; + case MONTHS: + return MONTH_FROM_NANOS; + case DAYS: + return DAY_FROM_NANOS; + case HOURS: + return HOUR_FROM_NANOS; + default: + throw new IllegalArgumentException( + "Unsupported source/result type units: " + type + "->" + resultTypeUnit); + } + } + + enum ResultTypeUnit { + YEARS(ChronoUnit.YEARS), + MONTHS(ChronoUnit.MONTHS), + DAYS(ChronoUnit.DAYS), + HOURS(ChronoUnit.HOURS), + MICROS(ChronoUnit.MICROS), + NANOS(ChronoUnit.NANOS), + ; + + private final ChronoUnit unit; + + ResultTypeUnit(final ChronoUnit unit) { + this.unit = unit; + } + + Duration getDuration() { + return unit.getDuration(); + } + } @Immutable static class Apply implements SerializableFunction<Long, Integer> { - private final ChronoUnit granularity; + private final ChronoUnit sourceTypeUnit; + private final ResultTypeUnit resultTypeUnit; - Apply(ChronoUnit granularity) { - this.granularity = granularity; + Apply(ChronoUnit sourceTypeUnit, ResultTypeUnit resultTypeUnit) { + this.sourceTypeUnit = sourceTypeUnit; + this.resultTypeUnit = resultTypeUnit; } @Override - public Integer apply(Long timestampMicros) { - if (timestampMicros == null) { + public Integer apply(Long timestampUnits) { + if (timestampUnits == null) { return null; } - switch (granularity) { - case YEARS: - return DateTimeUtil.microsToYears(timestampMicros); - case MONTHS: - return DateTimeUtil.microsToMonths(timestampMicros); - case DAYS: - return DateTimeUtil.microsToDays(timestampMicros); - case HOURS: - return DateTimeUtil.microsToHours(timestampMicros); + switch (sourceTypeUnit) { + case MICROS: + switch (resultTypeUnit) { + case YEARS: + return DateTimeUtil.microsToYears(timestampUnits); + case MONTHS: + return DateTimeUtil.microsToMonths(timestampUnits); + case DAYS: + return DateTimeUtil.microsToDays(timestampUnits); + case HOURS: + return DateTimeUtil.microsToHours(timestampUnits); + default: + throw new UnsupportedOperationException( + "Unsupported result type unit: " + resultTypeUnit); + } + case NANOS: + // TODO(epg): Overflows for MILLIS, MICROS, and NANOS! Fixing this is quite invasive, as + // Timestamps is assumed to be Transform<Long, Integer> in many, many places. + return (int) DateTimeUtil.convertNanos(timestampUnits, resultTypeUnit.unit); Review Comment: I've done that now. It seems to me a change to 64-bit integers may belong on the roadmap. ########## api/src/test/java/org/apache/iceberg/transforms/TestDays.java: ########## @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.iceberg.transforms; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +public class TestDays { + @Test + public void satisfiesOrderOf() { + assertThatThrownBy(() -> DAYS.satisfiesOrderOf(Timestamps.DAY_FROM_NANOS)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageMatching("Unsupported timestamp unit: DAYS"); + } + + private static final Days<Object> DAYS = Days.get(); Review Comment: done ########## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ########## @@ -185,7 +194,28 @@ private static int convertMicros(long micros, ChronoUnit granularity) { } } + public static long convertNanos(long nanos, ChronoUnit granularity) { + if (nanos >= 0) { + long epochSecond = Math.floorDiv(nanos, NANOS_PER_SECOND); + long nanoAdjustment = Math.floorMod(nanos, NANOS_PER_SECOND); + return granularity.between(EPOCH, toOffsetDateTime(epochSecond, nanoAdjustment)); + } else { + // add 1 nano to the value to account for the case where there is exactly 1 unit between + // the timestamp and epoch because the result will always be decremented. + long epochSecond = Math.floorDiv(nanos, NANOS_PER_SECOND); + long nanoAdjustment = Math.floorMod(nanos + 1, NANOS_PER_SECOND); + return granularity.between(EPOCH, toOffsetDateTime(epochSecond, nanoAdjustment)) - 1; + } + } + private static OffsetDateTime toOffsetDateTime(long epochSecond, long nanoAdjustment) { return Instant.ofEpochSecond(epochSecond, nanoAdjustment).atOffset(ZoneOffset.UTC); } + + private static final DateTimeFormatter FORMATTER = Review Comment: done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org