From 572e33b0ee0b0c5cf4f8001e7745f5496b1f0e8a Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Sun, 1 Nov 2026 13:20:34 -0800 Subject: [PATCH 1/3] Clean up CodeQL alerts in library and test code Library code, no behavior change: - Util.convertToLongArray: walk each 8-byte group forward with an explicit exclusive end, so the index bounds are visible. The output is unchanged, and UtilTest.checkConvertToLongArray still passes. - ReservoirItemsUnion and ReservoirLongsUnion toByteArray(): derive "empty" from gadgetBytes itself, so the null check guards the dereference directly. Tests: - UtilTest.checkCeilingMultiple2expK used (1 << 36) - 1L, an int shift that evaluates to 15, so the test only ever checked n = 15. It now uses 1L << 36 as intended. - PositionalSegmentTest: compute byte offsets in long arithmetic. - Remove locals that were assigned but never read. Where the call is what is tested, the call stays as a statement. CpcWrapperTest now asserts the empty-sketch estimate and bounds it previously discarded, and ArrayOfDoublesCompactSketchTest asserts the type its cast checked. Left as they are: the println(String s) debug helpers whose bodies are commented out (the unused-parameter alerts), locals used by commented-out debug println lines, and the deliberate bad cast in HeapIntersectionTest. Co-Authored-By: Claude Opus 5.5 --- .../org/apache/datasketches/common/Util.java | 13 ++---- .../sampling/ReservoirItemsUnion.java | 4 +- .../sampling/ReservoirLongsUnion.java | 4 +- .../apache/datasketches/common/UtilTest.java | 2 +- .../positional/PositionalSegmentTest.java | 42 +++++++++---------- .../datasketches/cpc/CpcWrapperTest.java | 14 +++---- .../datasketches/hll/HllSketchTest.java | 8 ++-- .../datasketches/hll/IsomorphicTest.java | 1 - .../datasketches/hll/PreambleUtilTest.java | 16 ++++--- .../DirectUpdateDoublesSketchTest.java | 2 - .../PartitionBoundariesTest.java | 2 +- .../datasketches/req/ReqSketchTest.java | 2 - .../theta/DirectQuickSelectSketchTest.java | 14 +++---- .../ArrayOfDoublesCompactSketchTest.java | 3 +- 14 files changed, 56 insertions(+), 71 deletions(-) diff --git a/src/main/java/org/apache/datasketches/common/Util.java b/src/main/java/org/apache/datasketches/common/Util.java index 187380fbf..23d94a17d 100644 --- a/src/main/java/org/apache/datasketches/common/Util.java +++ b/src/main/java/org/apache/datasketches/common/Util.java @@ -181,16 +181,11 @@ static long[] convertToLongArray(final byte[] byteArr, final boolean littleEndia int off = 0; int longArrIdx = 0; while (off < len) { - final int rem = Math.min(len - 1 - off, 7); + final int end = Math.min(off + 8, len); //exclusive long tgt = 0; - if (littleEndian) { - for (int j = off + rem, k = 0; j >= off; --j, k++) { - tgt |= (byteArr[j] & 0XFFL) << (k * 8); - } - } else { //BE - for (int j = off + rem, k = rem; j >= off; --j, k--) { - tgt |= (byteArr[j] & 0XFFL) << (k * 8); - } + for (int j = off; j < end; j++) { + final int shift = littleEndian ? (end - 1 - j) * 8 : (j - off) * 8; + tgt |= (byteArr[j] & 0XFFL) << shift; } off += 8; longArr[longArrIdx++] = tgt; diff --git a/src/main/java/org/apache/datasketches/sampling/ReservoirItemsUnion.java b/src/main/java/org/apache/datasketches/sampling/ReservoirItemsUnion.java index 424388581..3905cb368 100644 --- a/src/main/java/org/apache/datasketches/sampling/ReservoirItemsUnion.java +++ b/src/main/java/org/apache/datasketches/sampling/ReservoirItemsUnion.java @@ -288,8 +288,8 @@ public String toString() { // gadgetBytes will be null only if gadget_ == null AND empty == true public byte[] toByteArray(final ArrayOfItemsSerDe serDe, final Class clazz) { final int preLongs, outBytes; - final boolean empty = gadget_ == null; - final byte[] gadgetBytes = (gadget_ != null ? gadget_.toByteArray(serDe, clazz) : null); + final byte[] gadgetBytes = (gadget_ == null) ? null : gadget_.toByteArray(serDe, clazz); + final boolean empty = gadgetBytes == null; if (empty) { preLongs = Family.RESERVOIR_UNION.getMinPreLongs(); diff --git a/src/main/java/org/apache/datasketches/sampling/ReservoirLongsUnion.java b/src/main/java/org/apache/datasketches/sampling/ReservoirLongsUnion.java index 37129a299..9c4d03233 100644 --- a/src/main/java/org/apache/datasketches/sampling/ReservoirLongsUnion.java +++ b/src/main/java/org/apache/datasketches/sampling/ReservoirLongsUnion.java @@ -241,8 +241,8 @@ public String toString() { // gadgetBytes will be null only if gadget_ == null AND empty == true public byte[] toByteArray() { final int preLongs, outBytes; - final boolean empty = gadget_ == null; - final byte[] gadgetBytes = (gadget_ != null ? gadget_.toByteArray() : null); + final byte[] gadgetBytes = (gadget_ == null) ? null : gadget_.toByteArray(); + final boolean empty = gadgetBytes == null; if (empty) { preLongs = Family.RESERVOIR_UNION.getMinPreLongs(); diff --git a/src/test/java/org/apache/datasketches/common/UtilTest.java b/src/test/java/org/apache/datasketches/common/UtilTest.java index f8a5096fb..f21438194 100644 --- a/src/test/java/org/apache/datasketches/common/UtilTest.java +++ b/src/test/java/org/apache/datasketches/common/UtilTest.java @@ -446,7 +446,7 @@ static void checkConvertToLongArray() { @Test static void checkCeilingMultiple2expK() { - final long n = (1 << 36) - 1L; + final long n = (1L << 36) - 1L; final int k = 6; final long v = ceilingMultiple2expK(n, k); final long v2 = (long)Math.ceil(n / (double)(1 << k)); diff --git a/src/test/java/org/apache/datasketches/common/positional/PositionalSegmentTest.java b/src/test/java/org/apache/datasketches/common/positional/PositionalSegmentTest.java index f3bf385dc..4e66878a0 100644 --- a/src/test/java/org/apache/datasketches/common/positional/PositionalSegmentTest.java +++ b/src/test/java/org/apache/datasketches/common/positional/PositionalSegmentTest.java @@ -40,10 +40,10 @@ public void checkBytes() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Byte.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setByte((byte)i); } - for (int i = n/2; i < n; i++) { posSeg.setByte(i * Byte.BYTES, (byte)i); } + for (int i = n/2; i < n; i++) { posSeg.setByte((long) i * Byte.BYTES, (byte)i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final byte v1 = posSeg.getByte(i * Byte.BYTES); + final byte v1 = posSeg.getByte((long) i * Byte.BYTES); final byte v2 = posSeg.getByte(); assertEquals(v1, v2); assertEquals(v2, i); @@ -57,7 +57,7 @@ public void checkBytes() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_BYTE, i * Byte.BYTES), (byte)i); + assertEquals(seg2.get(JAVA_BYTE, (long) i * Byte.BYTES), (byte)i); } } @@ -67,10 +67,10 @@ public void checkChars() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Character.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setChar((char)i); } - for (int i = n/2; i < n; i++) { posSeg.setChar(i * Character.BYTES, (char)i); } + for (int i = n/2; i < n; i++) { posSeg.setChar((long) i * Character.BYTES, (char)i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final int v1 = posSeg.getChar(i *Character.BYTES); + final int v1 = posSeg.getChar((long) i * Character.BYTES); final int v2 = posSeg.getChar(); assertEquals(v1, v2); assertEquals(v2, i); @@ -84,7 +84,7 @@ public void checkChars() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_CHAR_UNALIGNED, i * Character.BYTES), (char)i); + assertEquals(seg2.get(JAVA_CHAR_UNALIGNED, (long) i * Character.BYTES), (char)i); } } @@ -94,10 +94,10 @@ public void checkShorts() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Short.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setShort((short)i); } - for (int i = n/2; i < n; i++) { posSeg.setShort(i * Short.BYTES, (short)i); } + for (int i = n/2; i < n; i++) { posSeg.setShort((long) i * Short.BYTES, (short)i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final int v1 = posSeg.getShort(i *Short.BYTES); + final int v1 = posSeg.getShort((long) i * Short.BYTES); final int v2 = posSeg.getShort(); assertEquals(v1, v2); assertEquals(v2, i); @@ -111,7 +111,7 @@ public void checkShorts() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_SHORT_UNALIGNED, i * Short.BYTES), (short)i); + assertEquals(seg2.get(JAVA_SHORT_UNALIGNED, (long) i * Short.BYTES), (short)i); } } @@ -121,10 +121,10 @@ public void checkInts() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Integer.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setInt((int)i); } - for (int i = n/2; i < n; i++) { posSeg.setInt(i * Integer.BYTES, (int)i); } + for (int i = n/2; i < n; i++) { posSeg.setInt((long) i * Integer.BYTES, (int)i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final int v1 = posSeg.getInt(i * Integer.BYTES); + final int v1 = posSeg.getInt((long) i * Integer.BYTES); final int v2 = posSeg.getInt(); assertEquals(v1, v2); assertEquals(v2, i); @@ -138,7 +138,7 @@ public void checkInts() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_INT_UNALIGNED, i * Integer.BYTES), (int)i); + assertEquals(seg2.get(JAVA_INT_UNALIGNED, (long) i * Integer.BYTES), (int)i); } } @@ -149,10 +149,10 @@ public void checkLongs() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Long.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setLong(i); } - for (int i = n/2; i < n; i++) { posSeg.setLong(i * Long.BYTES, i); } + for (int i = n/2; i < n; i++) { posSeg.setLong((long) i * Long.BYTES, i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final long v1 = posSeg.getLong(i * Long.BYTES); + final long v1 = posSeg.getLong((long) i * Long.BYTES); final long v2 = posSeg.getLong(); assertEquals(v1, v2); assertEquals(v2, i); @@ -166,7 +166,7 @@ public void checkLongs() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_LONG_UNALIGNED, i * Long.BYTES), i); + assertEquals(seg2.get(JAVA_LONG_UNALIGNED, (long) i * Long.BYTES), i); } } @@ -176,10 +176,10 @@ public void checkFloats() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Float.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setFloat(i); } - for (int i = n/2; i < n; i++) { posSeg.setFloat(i * Float.BYTES, i); } + for (int i = n/2; i < n; i++) { posSeg.setFloat((long) i * Float.BYTES, i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final float v1 = posSeg.getFloat(i * Float.BYTES); + final float v1 = posSeg.getFloat((long) i * Float.BYTES); final float v2 = posSeg.getFloat(); assertEquals(v1, v2); assertEquals(v2, i); @@ -193,7 +193,7 @@ public void checkFloats() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_FLOAT_UNALIGNED, i * Float.BYTES), i); + assertEquals(seg2.get(JAVA_FLOAT_UNALIGNED, (long) i * Float.BYTES), i); } } @@ -203,10 +203,10 @@ public void checkDoubles() { final MemorySegment seg = MemorySegment.ofArray(new byte[n * Double.BYTES]); final PositionalSegment posSeg = PositionalSegment.wrap(seg); for (int i = 0; i < (n/2); i++) { posSeg.setDouble(i); } - for (int i = n/2; i < n; i++) { posSeg.setDouble(i * Double.BYTES, i); } + for (int i = n/2; i < n; i++) { posSeg.setDouble((long) i * Double.BYTES, i); } posSeg.resetPosition(); for (int i = 0; i < n; i++) { - final double v1 = posSeg.getDouble(i * Double.BYTES); + final double v1 = posSeg.getDouble((long) i * Double.BYTES); final double v2 = posSeg.getDouble(); assertEquals(v1, v2); assertEquals(v2, i); @@ -220,7 +220,7 @@ public void checkDoubles() { final MemorySegment seg2 = posSeg.getMemorySegment(); for (int i = 0; i < n; i++) { - assertEquals(seg2.get(JAVA_DOUBLE_UNALIGNED, i * Double.BYTES), i); + assertEquals(seg2.get(JAVA_DOUBLE_UNALIGNED, (long) i * Double.BYTES), i); } } diff --git a/src/test/java/org/apache/datasketches/cpc/CpcWrapperTest.java b/src/test/java/org/apache/datasketches/cpc/CpcWrapperTest.java index 81f35f63c..4e184fa57 100644 --- a/src/test/java/org/apache/datasketches/cpc/CpcWrapperTest.java +++ b/src/test/java/org/apache/datasketches/cpc/CpcWrapperTest.java @@ -41,9 +41,9 @@ public void check() { final CpcSketch sk1 = new CpcSketch(lgK); final CpcSketch sk2 = new CpcSketch(lgK); final CpcSketch skD = new CpcSketch(lgK); - final double dEst = skD.getEstimate(); - final double dlb = skD.getLowerBound(2); - final double dub = skD.getUpperBound(2); + assertEquals(skD.getEstimate(), 0.0); + assertEquals(skD.getLowerBound(2), 0.0); + assertEquals(skD.getUpperBound(2), 0.0); final int n = 100000; for (int i = 0; i < n; i++) { @@ -56,9 +56,9 @@ public void check() { final CpcUnion union = new CpcUnion(lgK); final CpcSketch result = union.getResult(); - final double uEst = result.getEstimate(); - final double ulb = result.getLowerBound(2); - final double uub = result.getUpperBound(2); + assertEquals(result.getEstimate(), 0.0); + assertEquals(result.getLowerBound(2), 0.0); + assertEquals(result.getUpperBound(2), 0.0); union.update(sk1); union.update(sk2); final CpcSketch merged = union.getResult(); @@ -90,7 +90,7 @@ public void checkIsCompressed() { final byte[] byteArr = sk.toByteArray(); byteArr[5] &= (byte) -3; try { - final CpcWrapper wrapper = new CpcWrapper(MemorySegment.ofArray(byteArr)); + new CpcWrapper(MemorySegment.ofArray(byteArr)); fail(); } catch (final AssertionError e) {} } diff --git a/src/test/java/org/apache/datasketches/hll/HllSketchTest.java b/src/test/java/org/apache/datasketches/hll/HllSketchTest.java index 672d69601..5b9e239e1 100644 --- a/src/test/java/org/apache/datasketches/hll/HllSketchTest.java +++ b/src/test/java/org/apache/datasketches/hll/HllSketchTest.java @@ -268,13 +268,13 @@ private static void checkSerSizes(final int lgConfigK, final TgtHllType tgtHllTy @Test public void checkConfigKLimits() { try { - final HllSketch sk = new HllSketch(HllUtil.MIN_LOG_K - 1); + new HllSketch(HllUtil.MIN_LOG_K - 1); fail(); } catch (final SketchesArgumentException e) { //expected } try { - final HllSketch sk = new HllSketch(HllUtil.MAX_LOG_K + 1); + new HllSketch(HllUtil.MAX_LOG_K + 1); fail(); } catch (final SketchesArgumentException e) { //expected @@ -303,7 +303,7 @@ public void checkMemorySegmentNotLargeEnough() { final int bytes = getMaxUpdatableSerializationBytes(8, TgtHllType.HLL_8); final MemorySegment wseg = MemorySegment.ofArray(new byte[bytes -1]); try { - final HllSketch sk = new HllSketch(8, TgtHllType.HLL_8, wseg); + new HllSketch(8, TgtHllType.HLL_8, wseg); fail(); } catch (final SketchesArgumentException e) { //OK @@ -415,7 +415,7 @@ public void checkWritableWrapOfCompact() { final HllSketch sk = new HllSketch(); final byte[] byteArr = sk.toCompactByteArray(); final MemorySegment wseg = MemorySegment.ofArray(byteArr); - final HllSketch sk2 = HllSketch.writableWrap(wseg); + HllSketch.writableWrap(wseg); } @SuppressWarnings("unused") diff --git a/src/test/java/org/apache/datasketches/hll/IsomorphicTest.java b/src/test/java/org/apache/datasketches/hll/IsomorphicTest.java index d895246a8..6665350f1 100644 --- a/src/test/java/org/apache/datasketches/hll/IsomorphicTest.java +++ b/src/test/java/org/apache/datasketches/hll/IsomorphicTest.java @@ -158,7 +158,6 @@ public void isomorphicHllMerges() { private static void innerLoop(final int uLgK, final int skLgK, final TgtHllType tgtHllType) { HllUnion u; HllSketch sk; - final HllSketch skOut; //CASE 1 Heap HllUnion, Heap HllSketch u = buildHeapUnionHllMode(uLgK, 0); diff --git a/src/test/java/org/apache/datasketches/hll/PreambleUtilTest.java b/src/test/java/org/apache/datasketches/hll/PreambleUtilTest.java index 94d675dc4..dfd5899d8 100644 --- a/src/test/java/org/apache/datasketches/hll/PreambleUtilTest.java +++ b/src/test/java/org/apache/datasketches/hll/PreambleUtilTest.java @@ -110,12 +110,11 @@ public void checkCorruptMemorySegmentInput() { final HllSketch sk = new HllSketch(12); byte[] segObj = sk.toCompactByteArray(); MemorySegment wseg = MemorySegment.ofArray(segObj); - HllSketch bad; //checkFamily try { wseg.set(JAVA_BYTE, FAMILY_BYTE, (byte) 0); //corrupt, should be 7 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertFamilyId(wseg); //corrected @@ -123,7 +122,7 @@ public void checkCorruptMemorySegmentInput() { //check SerVer try { wseg.set(JAVA_BYTE, SER_VER_BYTE, (byte) 0); //corrupt, should be 1 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertSerVer(wseg); //corrected @@ -131,7 +130,7 @@ public void checkCorruptMemorySegmentInput() { //check bad PreInts try { insertPreInts(wseg, 0); //corrupt, should be 2 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertPreInts(wseg, 2); //corrected @@ -139,7 +138,7 @@ public void checkCorruptMemorySegmentInput() { //check wrong PreInts and LIST try { insertPreInts(wseg, 3); //corrupt, should be 2 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertPreInts(wseg, 2); //corrected @@ -152,7 +151,7 @@ public void checkCorruptMemorySegmentInput() { //check wrong PreInts and SET try { insertPreInts(wseg, 2); //corrupt, should be 3 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertPreInts(wseg, 3); //corrected @@ -165,7 +164,7 @@ public void checkCorruptMemorySegmentInput() { //check wrong PreInts and HLL try { insertPreInts(wseg, 2); //corrupt, should be 10 - bad = HllSketch.heapify(wseg); + HllSketch.heapify(wseg); fail(); } catch (final SketchesArgumentException e) { /* OK */ } insertPreInts(wseg, 10); //corrected @@ -176,8 +175,7 @@ public void checkCorruptMemorySegmentInput() { public void checkExtractFlags() { final int bytes = HllSketch.getMaxUpdatableSerializationBytes(4, TgtHllType.HLL_4); final MemorySegment wseg = MemorySegment.ofArray(new byte[bytes]); - final Object segObj = wseg.toArray(JAVA_BYTE); - final HllSketch sk = new HllSketch(4, TgtHllType.HLL_4, wseg); + new HllSketch(4, TgtHllType.HLL_4, wseg); final int flags = extractFlags(wseg); assertEquals(flags, EMPTY_FLAG_MASK); } diff --git a/src/test/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketchTest.java b/src/test/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketchTest.java index 088b4462c..6470c5351 100644 --- a/src/test/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketchTest.java +++ b/src/test/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketchTest.java @@ -167,9 +167,7 @@ public void checkMisc() { @SuppressWarnings("unused") @Test public void variousExceptions() { - final MemorySegment seg = MemorySegment.ofArray(new byte[8]); try { - final int flags = PreambleUtil.COMPACT_FLAG_MASK; DirectUpdateDoublesSketch.checkCompact(2, 0); fail(); } catch (final SketchesArgumentException e) {} //OK diff --git a/src/test/java/org/apache/datasketches/quantilescommon/PartitionBoundariesTest.java b/src/test/java/org/apache/datasketches/quantilescommon/PartitionBoundariesTest.java index 7ff4596d5..278502041 100644 --- a/src/test/java/org/apache/datasketches/quantilescommon/PartitionBoundariesTest.java +++ b/src/test/java/org/apache/datasketches/quantilescommon/PartitionBoundariesTest.java @@ -250,7 +250,7 @@ public void checkSketchPartitionLimits2() { //this should pass final long goodPartSizeRequest= sk.getMinPartitionSizeItems(); println("Good partSizeRequest " + goodPartSizeRequest); - final GenericPartitionBoundaries gpb = sk.getPartitionBoundariesFromPartSize(goodPartSizeRequest); + sk.getPartitionBoundariesFromPartSize(goodPartSizeRequest); //this should fail try { final long badPartSizeRequest = goodPartSizeRequest - 1; diff --git a/src/test/java/org/apache/datasketches/req/ReqSketchTest.java b/src/test/java/org/apache/datasketches/req/ReqSketchTest.java index 0499f865a..afd2d0c3e 100644 --- a/src/test/java/org/apache/datasketches/req/ReqSketchTest.java +++ b/src/test/java/org/apache/datasketches/req/ReqSketchTest.java @@ -125,8 +125,6 @@ private static void checkGetRank(final ReqSketch sk, final int min, final int ma final String dfmt = "%10.2f%10.6f" + LS; final String sfmt = "%10s%10s" + LS; if (iDebug > 0) { printf(sfmt, "Value", "Rank"); } - final float va = 0; - final double ranka = 0; for (int i = 0; i < spArr.length; i++) { final float v = spArr[i]; final double trueRank = trueRanks[i]; diff --git a/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java b/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java index 65c6c0a46..b01a996be 100644 --- a/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java +++ b/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java @@ -763,11 +763,9 @@ public void checkConstructorSrcSegCorruptions() { final UpdatableThetaSketch usk1 = UpdatableThetaSketch.builder().setNominalEntries(k).setResizeFactor(rf).build(seg1); for (int i=0; i>> 1); //corrupt theta and seg1.set(JAVA_BYTE, LG_ARR_LONGS_BYTE, (byte) 10); //corrupt lgArrLongs try { - usk2 = DirectQuickSelectSketch.writableWrap(seg1, null, Util.DEFAULT_UPDATE_SEED); + DirectQuickSelectSketch.writableWrap(seg1, null, Util.DEFAULT_UPDATE_SEED); fail("Expected SketchesArgumentException"); } catch (final SketchesArgumentException e) { //pass @@ -803,7 +801,7 @@ public void checkConstructorSrcSegCorruptions() { final byte badFlags = (byte) (COMPACT_FLAG_MASK | READ_ONLY_FLAG_MASK | ORDERED_FLAG_MASK); seg1.set(JAVA_BYTE, FLAGS_BYTE, badFlags); try { - usk2 = DirectQuickSelectSketch.writableWrap(seg1, null, Util.DEFAULT_UPDATE_SEED); + DirectQuickSelectSketch.writableWrap(seg1, null, Util.DEFAULT_UPDATE_SEED); fail("Expected SketchesArgumentException"); } catch (final SketchesArgumentException e) { //pass @@ -812,7 +810,7 @@ public void checkConstructorSrcSegCorruptions() { final byte[] arr2 = Arrays.copyOfRange(arr1, 0, bytes-1); //corrupt length final MemorySegment seg2 = MemorySegment.ofArray(arr2); try { - usk2 = DirectQuickSelectSketch.writableWrap(seg2, null, Util.DEFAULT_UPDATE_SEED); + DirectQuickSelectSketch.writableWrap(seg2, null, Util.DEFAULT_UPDATE_SEED); fail("Expected SketchesArgumentException"); } catch (final SketchesArgumentException e) { //pass diff --git a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesCompactSketchTest.java b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesCompactSketchTest.java index dbf5fc026..bfde5099b 100644 --- a/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesCompactSketchTest.java +++ b/src/test/java/org/apache/datasketches/tuple/arrayofdoubles/ArrayOfDoublesCompactSketchTest.java @@ -126,8 +126,7 @@ public void checkGetValuesAndKeysMethods() { MemorySegment hcskSeg = MemorySegment.ofArray(hcsk.toByteArray()); - DirectArrayOfDoublesCompactSketch dcsk2 = - (DirectArrayOfDoublesCompactSketch)ArrayOfDoublesSketch.wrap(hcskSeg, Util.DEFAULT_UPDATE_SEED); + Assert.assertTrue(ArrayOfDoublesSketch.wrap(hcskSeg, Util.DEFAULT_UPDATE_SEED) instanceof DirectArrayOfDoublesCompactSketch); final double[][] values4 = dqssk.getValues(); final double[] values1d4 = dqssk.getValuesAsOneDimension(); final long[] keys4 = dqssk.getKeys(); From 31e154780c6454f87e0307d5939a8a9507588c1b Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Sun, 1 Nov 2026 13:38:26 -0800 Subject: [PATCH 2/3] Util.convertToLongArray: name the flag bigEndian, and add javadoc The flag was named littleEndian, but true produced Big-Endian longs: bytes {1..8} became 0x0102030405060708. Renaming it to bigEndian makes the name match the behavior, so the code itself is unchanged. The method is package-private and only UtilTest calls it. The javadoc documents the byte order, and that a final group of fewer than 8 bytes is flush right in its long in either byte order. UtilTest.checkConvertToLongArray keeps its expectations and adds checks of full groups against ByteBuffer in both byte orders, using bytes with the high bit set, plus an empty array. Co-Authored-By: Claude Opus 5.5 --- .../org/apache/datasketches/common/Util.java | 20 +++++++++++++++++-- .../apache/datasketches/common/UtilTest.java | 20 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/datasketches/common/Util.java b/src/main/java/org/apache/datasketches/common/Util.java index 23d94a17d..0d867bc44 100644 --- a/src/main/java/org/apache/datasketches/common/Util.java +++ b/src/main/java/org/apache/datasketches/common/Util.java @@ -175,7 +175,23 @@ public static byte[] longToBytes(final long v, final byte[] arr) { /** Byte array conversions *******************/ - static long[] convertToLongArray(final byte[] byteArr, final boolean littleEndian) { + /** + * Converts a byte array to an array of longs, taking the bytes in groups of 8. + * + *

Each group of 8 bytes becomes one long. If bigEndian is false, the first byte of a group + * is the least significant byte of its long (Little-Endian). If true, the first byte of a group is the + * most significant byte (Big-Endian).

+ * + *

If the length of the byte array is not a multiple of 8, the final group has fewer than 8 bytes. + * Those bytes are converted in the same byte order and are flush right in the final long; its unused + * high-order bytes are zero. For example, the final group {9, 10, 11, 12} becomes + * 0x000000000C0B0A09 if Little-Endian and 0x00000000090A0B0C if Big-Endian.

+ * + * @param byteArr the given byte array + * @param bigEndian if true, the bytes of each group are in Big-Endian order, otherwise Little-Endian. + * @return an array of ceiling(byteArr.length / 8) longs. + */ + static long[] convertToLongArray(final byte[] byteArr, final boolean bigEndian) { final int len = byteArr.length; final long[] longArr = new long[(len / 8) + ((len % 8) != 0 ? 1 : 0)]; int off = 0; @@ -184,7 +200,7 @@ static long[] convertToLongArray(final byte[] byteArr, final boolean littleEndia final int end = Math.min(off + 8, len); //exclusive long tgt = 0; for (int j = off; j < end; j++) { - final int shift = littleEndian ? (end - 1 - j) * 8 : (j - off) * 8; + final int shift = bigEndian ? (end - 1 - j) * 8 : (j - off) * 8; tgt |= (byteArr[j] & 0XFFL) << shift; } off += 8; diff --git a/src/test/java/org/apache/datasketches/common/UtilTest.java b/src/test/java/org/apache/datasketches/common/UtilTest.java index f21438194..950ef7565 100644 --- a/src/test/java/org/apache/datasketches/common/UtilTest.java +++ b/src/test/java/org/apache/datasketches/common/UtilTest.java @@ -58,6 +58,9 @@ import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; + import org.testng.Assert; import org.testng.annotations.Test; @@ -431,17 +434,34 @@ public void checkExactLog2OfIntWithArg() { static void checkConvertToLongArray() { final byte[] arr = {1,2,3,4,5,6,7,8,9,10,11,12}; + //Little-Endian; the 4-byte tail is flush right long[] out = convertToLongArray(arr, false); + assertEquals(out.length, 2); String s = zeroPad(Long.toHexString(out[0]), 16); assertEquals(s, "0807060504030201"); s = zeroPad(Long.toHexString(out[1]), 16); assertEquals(s, "000000000c0b0a09"); + //Big-Endian; the 4-byte tail is flush right out = convertToLongArray(arr, true); + assertEquals(out.length, 2); s = zeroPad(Long.toHexString(out[0]), 16); assertEquals(s, "0102030405060708"); s = zeroPad(Long.toHexString(out[1]), 16); assertEquals(s, "00000000090a0b0c"); + + //full groups agree with ByteBuffer in both byte orders + final byte[] arr16 = new byte[16]; + for (int i = 0; i < 16; i++) { arr16[i] = (byte) (0xF0 + i); } //high bit set, to check sign handling + for (final boolean bigEndian : new boolean[] {false, true}) { + final ByteBuffer bb = ByteBuffer.wrap(arr16).order(bigEndian ? ByteOrder.BIG_ENDIAN : ByteOrder.LITTLE_ENDIAN); + out = convertToLongArray(arr16, bigEndian); + assertEquals(out.length, 2); + assertEquals(out[0], bb.getLong(0)); + assertEquals(out[1], bb.getLong(8)); + } + + assertEquals(convertToLongArray(new byte[0], true).length, 0); } @Test From 38e8049483bbf6d2c010d897e0c349352fcf275f Mon Sep 17 00:00:00 2001 From: Lee Rhodes Date: Sun, 1 Nov 2026 14:36:41 -0800 Subject: [PATCH 3/3] KllPreambleUtil.toString: use an explicitly non-null serDe for items The debug toString(MemorySegment, SketchType, boolean, serDe) already required a non-null serDe for an items sketch, but the check was at the top of the method, and CodeQL could not connect it to the later "else //ITEMS_SKETCH" branches. Each items branch now takes its serDe from Objects.requireNonNull(serDe, ITEMS_SERDE_MSG), so the guard is at the dereference. No behavior change. Co-Authored-By: Claude Opus 5.5 --- .../datasketches/kll/KllPreambleUtil.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/datasketches/kll/KllPreambleUtil.java b/src/main/java/org/apache/datasketches/kll/KllPreambleUtil.java index 27a6372bb..dc658626d 100644 --- a/src/main/java/org/apache/datasketches/kll/KllPreambleUtil.java +++ b/src/main/java/org/apache/datasketches/kll/KllPreambleUtil.java @@ -139,6 +139,8 @@ final class KllPreambleUtil { private KllPreambleUtil() {} + private static final String ITEMS_SERDE_MSG = "SerDe parameter must not be null for ITEMS_SKETCH."; + // Preamble byte addresses static final int PREAMBLE_INTS_BYTE_ADR = 0; static final int SER_VER_BYTE_ADR = 1; @@ -224,7 +226,7 @@ static String toString(final MemorySegment seg, final SketchType sketchType, fin static String toString(final MemorySegment seg, final SketchType sketchType, final boolean includeData, final ArrayOfItemsSerDe serDe) { if (sketchType == KLL_ITEMS_SKETCH) { - Objects.requireNonNull(serDe, "SerDe parameter must not be null for ITEMS_SKETCH."); + Objects.requireNonNull(serDe, ITEMS_SERDE_MSG); } final KllMemorySegmentValidate segVal = new KllMemorySegmentValidate(seg, sketchType, serDe); final SketchStructure myStructure = segVal.sketchStructure; @@ -359,10 +361,11 @@ static String toString(final MemorySegment seg, final SketchType sketchType, sb.append(seg.get(JAVA_LONG_UNALIGNED, offsetBytes)).append(LS); offsetBytes += typeBytes; } else { //ITEMS_SKETCH - sb.append(serDe.deserializeFromMemorySegment(seg, offsetBytes, 1)[0]).append(LS); - offsetBytes += serDe.sizeOf(seg, offsetBytes, 1); - sb.append(serDe.deserializeFromMemorySegment(seg, offsetBytes, 1)[0]).append(LS); - offsetBytes += serDe.sizeOf(seg, offsetBytes, 1); + final ArrayOfItemsSerDe itemsSerDe = Objects.requireNonNull(serDe, ITEMS_SERDE_MSG); + sb.append(itemsSerDe.deserializeFromMemorySegment(seg, offsetBytes, 1)[0]).append(LS); + offsetBytes += itemsSerDe.sizeOf(seg, offsetBytes, 1); + sb.append(itemsSerDe.deserializeFromMemorySegment(seg, offsetBytes, 1)[0]).append(LS); + offsetBytes += itemsSerDe.sizeOf(seg, offsetBytes, 1); } sb.append("RETAINED DATA").append(LS); @@ -383,9 +386,10 @@ static String toString(final MemorySegment seg, final SketchType sketchType, offsetBytes += typeBytes; } } else { //ITEMS_SKETCH - final T[] itemsArr = serDe.deserializeFromMemorySegment(seg, offsetBytes, retainedItems); + final ArrayOfItemsSerDe itemsSerDe = Objects.requireNonNull(serDe, ITEMS_SERDE_MSG); + final T[] itemsArr = itemsSerDe.deserializeFromMemorySegment(seg, offsetBytes, retainedItems); for (int i = 0; i < itemsArr.length; i++) { - sb.append(i + ", " + serDe.toString(itemsArr[i])).append(LS); + sb.append(i + ", " + itemsSerDe.toString(itemsArr[i])).append(LS); } } @@ -399,7 +403,8 @@ static String toString(final MemorySegment seg, final SketchType sketchType, } else if (sketchType == KLL_LONGS_SKETCH) { sb.append(seg.get(JAVA_LONG_UNALIGNED, DATA_START_ADR_SINGLE_ITEM)).append(LS); } else { //ITEMS_SKETCH - sb.append(serDe.deserializeFromMemorySegment(seg, DATA_START_ADR_SINGLE_ITEM, 1)[0]).append(LS); + final ArrayOfItemsSerDe itemsSerDe = Objects.requireNonNull(serDe, ITEMS_SERDE_MSG); + sb.append(itemsSerDe.deserializeFromMemorySegment(seg, DATA_START_ADR_SINGLE_ITEM, 1)[0]).append(LS); } } else { //COMPACT_EMPTY