clintropolis commented on a change in pull request #11004:
URL: https://github.com/apache/druid/pull/11004#discussion_r596496610
##########
File path:
processing/src/test/java/org/apache/druid/segment/data/VSizeLongSerdeTest.java
##########
@@ -20,132 +20,352 @@
package org.apache.druid.segment.data;
+import com.google.common.primitives.Ints;
+import org.apache.druid.java.util.common.StringUtils;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.stream.Collectors;
+@RunWith(Enclosed.class)
public class VSizeLongSerdeTest
Review comment:
Looking closer, I don't think it is necessary to add the checks based on
how `IntermediateColumnarLongsSerializer` works.
For delta encoding, the delta is computed with `LongMath.checkedSubtract`
which will ensure delta is a positive number and the checks ensure it won't be
used if delta is `Long.MAX_VALUE`, or it overflowed because the range between
values was too big, setting delta to -1. These checks make it so the value in
the constructor of the writer fed to `VSizeLongSerde.getBitsForMax` will be
appropriate. The `minVal` is used as the base, and the writer subtracts the
base from every value while serializing so every number given to the serializer
will be between 0 and delta.
For table encoding, since the values are replaced with their corresponding
index into the table array, they should also always be 0 or positive.
Is it worth the overhead to make `LongSerializer.write` implementations
check this for every value?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
