nssalian commented on code in PR #16748:
URL: https://github.com/apache/iceberg/pull/16748#discussion_r3385202156
##########
api/src/main/java/org/apache/iceberg/variants/VariantUtil.java:
##########
@@ -33,78 +33,6 @@ class VariantUtil {
private VariantUtil() {}
- /** A hacky absolute put for ByteBuffer */
- static int writeBufferAbsolute(ByteBuffer buffer, int offset, ByteBuffer
toCopy) {
- int originalPosition = buffer.position();
- buffer.position(offset);
- ByteBuffer copy = toCopy.duplicate();
- buffer.put(copy); // duplicate so toCopy is not modified
- buffer.position(originalPosition);
- Preconditions.checkArgument(copy.remaining() <= 0, "Not fully written");
- return toCopy.remaining();
- }
-
- static void writeByte(ByteBuffer buffer, int value, int offset) {
- buffer.put(buffer.position() + offset, (byte) (value & 0xFF));
- }
-
- static void writeLittleEndianUnsigned(ByteBuffer buffer, int value, int
offset, int size) {
- int base = buffer.position() + offset;
- switch (size) {
- case 4:
- buffer.putInt(base, value);
- return;
- case 3:
- buffer.putShort(base, (short) (value & 0xFFFF));
- buffer.put(base + 2, (byte) ((value >> 16) & 0xFF));
- return;
- case 2:
- buffer.putShort(base, (short) (value & 0xFFFF));
- return;
- case 1:
- buffer.put(base, (byte) (value & 0xFF));
- return;
- }
-
- throw new IllegalArgumentException("Invalid size: " + size);
- }
-
- static byte readLittleEndianInt8(ByteBuffer buffer, int offset) {
- return buffer.get(buffer.position() + offset);
- }
-
- static short readLittleEndianInt16(ByteBuffer buffer, int offset) {
- return buffer.getShort(buffer.position() + offset);
- }
-
- static int readByte(ByteBuffer buffer, int offset) {
- return buffer.get(buffer.position() + offset) & 0xFF;
- }
-
- static int readLittleEndianUnsigned(ByteBuffer buffer, int offset, int size)
{
- int base = buffer.position() + offset;
- switch (size) {
- case 4:
- return buffer.getInt(base);
- case 3:
- return (((int) buffer.getShort(base)) & 0xFFFF) | ((buffer.get(base +
2) & 0xFF) << 16);
- case 2:
- return ((int) buffer.getShort(base)) & 0xFFFF;
- case 1:
- return buffer.get(base) & 0xFF;
- }
-
- throw new IllegalArgumentException("Invalid size: " + size);
- }
-
- static int readLittleEndianInt32(ByteBuffer buffer, int offset) {
- return buffer.getInt(buffer.position() + offset);
- }
-
- static long readLittleEndianInt64(ByteBuffer buffer, int offset) {
- return buffer.getLong(buffer.position() + offset);
- }
-
static float readFloat(ByteBuffer buffer, int offset) {
Review Comment:
should readFloat and readDouble be moved as well?
##########
api/src/test/java/org/apache/iceberg/util/TestByteBuffers.java:
##########
@@ -0,0 +1,46 @@
+/*
+ *
+ * * 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.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.ByteBuffer;
+import org.junit.jupiter.api.Test;
+
+public class TestByteBuffers {
+ @Test
+ public void testReadByteUnsigned() {
+ ByteBuffer buffer = ByteBuffer.wrap(new byte[] {(byte) 0xFF});
+ assertThat(ByteBuffers.readByte(buffer, 0)).isEqualTo(255);
+ }
+
+ @Test
+ public void testRead2ByteUnsigned() {
+ ByteBuffer buffer = ByteBuffer.wrap(new byte[] {(byte) 0xFF, (byte) 0xFF});
+ assertThat(ByteBuffers.readLittleEndianUnsigned(buffer, 0,
2)).isEqualTo(65535);
+ }
+
+ @Test
+ public void testRead3ByteUnsigned() {
+ ByteBuffer buffer = ByteBuffer.wrap(new byte[] {(byte) 0xFF, (byte) 0xFF,
(byte) 0xFF});
+ assertThat(ByteBuffers.readLittleEndianUnsigned(buffer, 0,
3)).isEqualTo(16777215);
+ }
Review Comment:
worth adding tests for the other methods too. I think the variant tests
would exercise these indirectly but good to have unit tests.
--
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]
