rash67 commented on code in PR #12879: URL: https://github.com/apache/druid/pull/12879#discussion_r957626091
########## processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexColumn.java: ########## @@ -0,0 +1,134 @@ +/* + * 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.druid.query.aggregation; + +import com.google.common.base.Preconditions; +import org.apache.druid.collections.ResourceHolder; +import org.apache.druid.java.util.common.RE; +import org.apache.druid.java.util.common.io.Closer; +import org.apache.druid.segment.column.ComplexColumn; +import org.apache.druid.segment.serde.cell.CellReader; +import org.apache.druid.segment.serde.cell.NativeClearedByteBufferProvider; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; + +public class SerializablePairLongStringComplexColumn implements ComplexColumn +{ + private final Closer closer; + private final int serializedSize; + private final CellReader cellReader; + private final SerializablePairLongStringDeltaEncodedStagedSerde serde; + + public SerializablePairLongStringComplexColumn( + CellReader cellReader, + SerializablePairLongStringDeltaEncodedStagedSerde serde, + Closer closer, + int serializedSize + ) + { + this.cellReader = cellReader; + this.serde = serde; + this.closer = closer; + this.serializedSize = serializedSize; + } + + @Override + public Class<?> getClazz() + { + return SerializablePairLongString.class; + } + + @Override + public String getTypeName() + { + return SerializablePairLongStringComplexMetricSerde.TYPE_NAME; + } + + @SuppressWarnings("ConstantConditions") + @Override + public Object getRowValue(int rowNum) + { + // nulls are handled properly by the aggregator Review Comment: sure. the comment was meant mostly to explain my " @SuppressWarnings("ConstantConditions")" which suppresses an idea warning about returning null. the method is subject to a broad "@NotNull" annotation across packages, but all aggregators I've seen (granted very few) seem to handle null as they should. And this method is for precisely two aggregators. were it not for the idea warning, i wouldn't have added any comment as the actual contract seems to be that ComplexColumn.getRowValue() can in fact return null in general, not just in this implementation. (comment updated, but here's the context where I think the @NotNull should really not apply, it's just...to almost everything; not sure if an @Nullable annotation to it would be useful) see org.apache.druid.segment.package-info.json: ``` @EverythingIsNonnullByDefault package org.apache.druid.segment; import org.apache.druid.annotations.EverythingIsNonnullByDefault; ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
