This is an automated email from the ASF dual-hosted git repository. dmollitor pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push: new e74029d HIVE-23592: Routine makeIntPair is Not Correct (David Mollitor, reviewed by Miklos Gergely) e74029d is described below commit e74029d4fd5c4bfc50d33a8f1155ffacc151ba8f Author: belugabehr <12578579+belugab...@users.noreply.github.com> AuthorDate: Tue Jun 16 09:23:19 2020 -0400 HIVE-23592: Routine makeIntPair is Not Correct (David Mollitor, reviewed by Miklos Gergely) --- .../org/apache/hadoop/hive/common/NumberUtils.java | 60 ++++++++++++++++++++++ .../apache/hadoop/hive/common/TestNumberUtils.java | 57 ++++++++++++++++++++ .../hadoop/hive/llap/cache/BuddyAllocator.java | 15 ++---- .../hadoop/hive/ql/io/orc/encoded/IoTrace.java | 16 ++---- 4 files changed, 125 insertions(+), 23 deletions(-) diff --git a/common/src/java/org/apache/hadoop/hive/common/NumberUtils.java b/common/src/java/org/apache/hadoop/hive/common/NumberUtils.java new file mode 100644 index 0000000..c9fa424 --- /dev/null +++ b/common/src/java/org/apache/hadoop/hive/common/NumberUtils.java @@ -0,0 +1,60 @@ +/* + * 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.hadoop.hive.common; + +/** + * Collection of {@link Number} manipulation utilities common across Hive. + */ +public final class NumberUtils { + + private NumberUtils() { + } + + /** + * Store two ints in a single long value. + * + * @param i1 First int to store + * @param i2 Second int to store + * @return The combined value stored in a long + */ + public static long makeIntPair(int i1, int i2) { + return (((long) i1) << 32) | (i2 & 0xffffffffL); + } + + /** + * Get the first int stored in a long value. + * + * @param pair The pair generated from makeIntPair + * @return The first value stored in the long + */ + public static int getFirstInt(long pair) { + return (int) (pair >> 32); + } + + /** + * Get the second int stored in a long value. + * + * @param pair The pair generated from makeIntPair + * @return The first value stored in the long + */ + public static int getSecondInt(long pair) { + return (int) pair; + } + +} diff --git a/common/src/test/org/apache/hadoop/hive/common/TestNumberUtils.java b/common/src/test/org/apache/hadoop/hive/common/TestNumberUtils.java new file mode 100644 index 0000000..c370dbd --- /dev/null +++ b/common/src/test/org/apache/hadoop/hive/common/TestNumberUtils.java @@ -0,0 +1,57 @@ +/* + * 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.hadoop.hive.common; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Test suite for the {@link NumberUtils} class. + */ +public class TestNumberUtils { + + @Test + public void testMinLong() { + final long pair = NumberUtils.makeIntPair(Integer.MIN_VALUE, Integer.MIN_VALUE); + Assert.assertEquals(Integer.MIN_VALUE, NumberUtils.getFirstInt(pair)); + Assert.assertEquals(Integer.MIN_VALUE, NumberUtils.getSecondInt(pair)); + } + + @Test + public void testMaxLong() { + final long pair = NumberUtils.makeIntPair(Integer.MAX_VALUE, Integer.MAX_VALUE); + Assert.assertEquals(Integer.MAX_VALUE, NumberUtils.getFirstInt(pair)); + Assert.assertEquals(Integer.MAX_VALUE, NumberUtils.getSecondInt(pair)); + } + + @Test + public void testZeroLong() { + final long pair = NumberUtils.makeIntPair(0, 0); + Assert.assertEquals(0, NumberUtils.getFirstInt(pair)); + Assert.assertEquals(0, NumberUtils.getSecondInt(pair)); + } + + @Test + public void testNegativePositiveLong() { + final long pair = NumberUtils.makeIntPair(1, -1); + Assert.assertEquals(1, NumberUtils.getFirstInt(pair)); + Assert.assertEquals(-1, NumberUtils.getSecondInt(pair)); + } + +} diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java index c7a6402..854db5d 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java @@ -17,6 +17,10 @@ */ package org.apache.hadoop.hive.llap.cache; +import static org.apache.hadoop.hive.common.NumberUtils.getFirstInt; +import static org.apache.hadoop.hive.common.NumberUtils.getSecondInt; +import static org.apache.hadoop.hive.common.NumberUtils.makeIntPair; + import java.nio.channels.ClosedByInterruptException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -1673,17 +1677,6 @@ public final class BuddyAllocator return Math.max(freeListIx - minAllocLog2, 0); } - // Utility methods used to store pairs of ints as long. - private static long makeIntPair(int first, int second) { - return ((long)first) << 32 | second; - } - private static int getFirstInt(long result) { - return (int) (result >>> 32); - } - private static int getSecondInt(long result) { - return (int) (result & ((1L << 32) - 1)); - } - // Debug/test related methods. private void assertBufferLooksValid( int freeListIx, LlapAllocatorBuffer buf, int arenaIx, int headerIx) { diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/IoTrace.java b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/IoTrace.java index bfc82b5..63af064 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/IoTrace.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/IoTrace.java @@ -18,6 +18,10 @@ package org.apache.hadoop.hive.ql.io.orc.encoded; +import static org.apache.hadoop.hive.common.NumberUtils.getFirstInt; +import static org.apache.hadoop.hive.common.NumberUtils.getSecondInt; +import static org.apache.hadoop.hive.common.NumberUtils.makeIntPair; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.common.Pool; import org.apache.hadoop.hive.common.io.DiskRange; @@ -217,18 +221,6 @@ public final class IoTrace { } } - //Utility methods used to store pairs of ints as long. - private static long makeIntPair(int first, int second) { - return ((long)first) << 32 | second; - } - - private static int getFirstInt(long result) { - return (int) (result >>> 32); - } - private static int getSecondInt(long result) { - return (int) (result & ((1L << 32) - 1)); - } - public void logTreeReaderNextVector(int idx) { if (log == null) return; int offset = this.offset;