Jackie-Jiang commented on code in PR #11773: URL: https://github.com/apache/pinot/pull/11773#discussion_r1367496608
########## pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/DateTimeConversionHopTransformFunction.java: ########## @@ -0,0 +1,160 @@ +/** + * 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.pinot.core.operator.transform.function; + +import java.util.List; +import java.util.Map; +import org.apache.pinot.core.operator.ColumnContext; +import org.apache.pinot.core.operator.blocks.ValueBlock; +import org.apache.pinot.core.operator.transform.TransformResultMetadata; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.BaseDateTimeWindowHopTransformer; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.DateTimeWindowHopTransformerFactory; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.EpochToEpochWindowHopTransformer; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.EpochToSDFHopWindowTransformer; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.SDFToEpochWindowHopTransformer; +import org.apache.pinot.core.operator.transform.transformer.datetimehop.SDFToSDFWindowHopTransformer; +import org.roaringbitmap.RoaringBitmap; + +/** + * The <code>DateTimeConversionHopTransformFunction</code> class implements the date time conversion + * with hop transform function. + * <ul> + * <li> + * This transform function should be invoked with arguments: + * <ul> + * <li>Column name to convert. E.g. Date</li> + * <li>Input format of the column. E.g. 1:HOURS:EPOCH</li> Review Comment: Let's document the new pipe separated format (see `DateTimeFormatSpec` for details), and use the new format in the tests. Old colon separated format is deprecated ########## pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java: ########## @@ -129,6 +129,13 @@ public enum TransformFunctionType { OperandTypes.family(ImmutableList.of(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER)), "date_time_convert"), + DATE_TIME_CONVERT_WINDOW_HOP("dateTimeConvertWindowHop", + ReturnTypes.TO_ARRAY, Review Comment: (code format) Can you apply [Pinot Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#set-up-ide) and reformat all the changes? ########## pinot-core/src/main/java/org/apache/pinot/core/operator/transform/transformer/datetimehop/BaseDateTimeWindowHopTransformer.java: ########## @@ -0,0 +1,92 @@ +/** + * 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.pinot.core.operator.transform.transformer.datetimehop; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import javax.annotation.Nonnull; +import org.apache.pinot.core.operator.transform.transformer.DataTransformer; +import org.apache.pinot.spi.data.DateTimeFormatSpec; +import org.apache.pinot.spi.data.DateTimeFormatUnitSpec; +import org.apache.pinot.spi.data.DateTimeGranularitySpec; +import org.joda.time.DateTime; +import org.joda.time.format.DateTimeFormatter; + +public abstract class BaseDateTimeWindowHopTransformer<I, O> implements DataTransformer<I, O> { + private final int _inputTimeSize; + private final TimeUnit _inputTimeUnit; + private final DateTimeFormatter _inputDateTimeFormatter; + private final int _outputTimeSize; + private final DateTimeFormatUnitSpec.DateTimeTransformUnit _outputTimeUnit; + private final DateTimeFormatter _outputDateTimeFormatter; + private final long _outputGranularityMillis; + protected final long _hopWindowSizeMillis; + + public BaseDateTimeWindowHopTransformer(@Nonnull DateTimeFormatSpec inputFormat, Review Comment: (convention) We usually only annotate `@Nullable` and assume everything non-null if not annotated. Same for other classes ########## pinot-core/src/main/java/org/apache/pinot/core/operator/transform/transformer/datetimehop/EpochToEpochWindowHopTransformer.java: ########## @@ -0,0 +1,48 @@ +/** + * 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.pinot.core.operator.transform.transformer.datetimehop; + +import java.util.List; +import javax.annotation.Nonnull; +import org.apache.pinot.spi.data.DateTimeFormatSpec; +import org.apache.pinot.spi.data.DateTimeGranularitySpec; + +public class EpochToEpochWindowHopTransformer extends BaseDateTimeWindowHopTransformer<long[], long[][]> { + public EpochToEpochWindowHopTransformer(@Nonnull DateTimeFormatSpec inputFormat, + @Nonnull DateTimeFormatSpec outputFormat, + @Nonnull DateTimeGranularitySpec outputGranularity, + @Nonnull DateTimeGranularitySpec hopSize) { + super(inputFormat, outputFormat, outputGranularity, hopSize); + } + + @Override + public void transform(@Nonnull long[] input, @Nonnull long[][] output, int length) { + for (int i = 0; i < length; i++) { + long epochTime = input[i]; + long millisSinceEpoch = transformEpochToMillis(epochTime); + List<Long> hopWindows = hopWindows(millisSinceEpoch); + output[i] = hopWindows Review Comment: Suggest not using stream API for performance sensitive path -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
