clintropolis commented on a change in pull request #11853: URL: https://github.com/apache/druid/pull/11853#discussion_r744010821
########## File path: core/src/main/java/org/apache/druid/segment/column/ObjectByteStrategy.java ########## @@ -0,0 +1,58 @@ +/* + * 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.segment.column; + +import javax.annotation.Nullable; +import java.nio.ByteBuffer; +import java.util.Comparator; + +/** + * Naming is hard. This is the core interface extracted from another interface called ObjectStrategy that lives in + * 'druid-processing'. It provides basic methods for handling converting some type of object to a binary form, reading + * the binary form back into an object from a {@link ByteBuffer}, and mechanism to perform comparisons between objects. + * + * Complex types register one of these in {@link Types#registerStrategy}, which can be retrieved by the complex + * type name to convert values to and from binary format, and compare them. + * + * This could be recombined with 'ObjectStrategy' should these two modules be combined. + */ +public interface ObjectByteStrategy<T> extends Comparator<T> +{ + Class<? extends T> getClazz(); + + /** + * Convert values from their underlying byte representation. + * + * Implementations of this method <i>may</i> change the given buffer's mark, or limit, and position. + * + * Implementations of this method <i>may not</i> store the given buffer in a field of the "deserialized" object, + * need to use {@link ByteBuffer#slice()}, {@link ByteBuffer#asReadOnlyBuffer()} or {@link ByteBuffer#duplicate()} in + * this case. + * + * @param buffer buffer to read value from + * @param numBytes number of bytes used to store the value, starting at buffer.position() + * @return an object created from the given byte buffer representation + */ + @Nullable + T fromByteBuffer(ByteBuffer buffer, int numBytes); + + @Nullable + byte[] toBytes(@Nullable T val); Review comment: I'm currently thinking that this is a temporary state, what I really want is something like this: ``` +++ b/core/src/main/java/org/apache/druid/segment/column/TypeStrategy.java @@ -0,0 +1,62 @@ +package org.apache.druid.segment.column; + +import org.apache.druid.common.config.NullHandling; + +import javax.annotation.Nullable; +import java.nio.ByteBuffer; +import java.util.Comparator; + +public interface TypeStrategy<T> extends Comparator<T> +{ + TypeSignature<?> getType(); + + int estimateSizeBytes(@Nullable T value); + + T read(ByteBuffer buffer); + void write(ByteBuffer buffer, T value); + + @Nullable + default T readNullable(ByteBuffer buffer) { + if ((buffer.get() & NullHandling.IS_NULL_BYTE) == NullHandling.IS_NULL_BYTE) { + return null; + } + return read(buffer); + } + + @Nullable + default T readNullable(ByteBuffer buffer, int offset) + { + if (TypeStrategies.isNullableNull(buffer, offset)) { + return null; + } + return read(buffer, offset + TypeStrategies.VALUE_OFFSET); + } + + default T read(ByteBuffer buffer, int offset) + { + final int oldPosition = buffer.position(); + buffer.position(offset); + T value = read(buffer); + buffer.position(oldPosition); + return value; + } + + default int write(ByteBuffer buffer, int offset, T value) + { + final int oldPosition = buffer.position(); + buffer.position(offset); + write(buffer, value); + final int size = buffer.position() - offset; + buffer.position(oldPosition); + return size; + } + + default int writeNullable(ByteBuffer buffer, int offset, @Nullable T value) + { + if (value == null) { + return TypeStrategies.writeNull(buffer, offset); + } + buffer.put(offset, NullHandling.IS_NOT_NULL_BYTE); + return write(buffer, offset + TypeStrategies.VALUE_OFFSET, value); + } +} ``` which I've started to sketch out locally. I imagine that `TypeSignature` will have a `getStrategy` method that returns one of these, which means we have an easy way to get binary value serialization and a comparator for any given type. I will likely move `ObjectByteStrategy` back into `ObjectStrategy`, and instead make `ComplexMetricsSerde` define a `getTypeStrategy` which defaults to wrapping the `ObjectStrategy`, so that all of them get a free implementation out of the box, but can implement an optimized version in the event they are backed directly by the memory location. -- 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]
