From bda4c332476ff6fcf4e7e6a5f3281dd3611ba882 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:10:12 -0400 Subject: [PATCH 1/3] Fix Util.numDigits returning 0 for 0 and 1, and 18 for 1E18 numDigits(0) and numDigits(1) both returned 0, and numDigits(1000000000000000000L) returned 18 instead of 19. Util.java:805 nudged multiples of ten up by one and then took ceil(log(n) / log(10)). For n = 0 and n = 1 that is ceil(log(1) / log(10)), which is ceil(0.0), which is 0. For n = 1E18 the nudged value 1000000000000000001 has no exact double, so it rounds back to 1E18 and the ratio comes out at 17.999999999999996. Replaced the floating point form with an exact integer loop, which has no representation limit anywhere in the long range. Zero and any negative now return 1, matching LongsAsOrderableStrings.digits in the test tree, which guards maxValue <= 0 the same way. Added UtilTest.checkNumDigits. --- .../java/org/apache/datasketches/common/Util.java | 10 ++++++---- .../java/org/apache/datasketches/common/UtilTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/datasketches/common/Util.java b/src/main/java/org/apache/datasketches/common/Util.java index afb080501..7690a3056 100644 --- a/src/main/java/org/apache/datasketches/common/Util.java +++ b/src/main/java/org/apache/datasketches/common/Util.java @@ -798,13 +798,15 @@ public static int bitAt(final long number, final int bitPos) { /** Decimal Digits ***************************/ /** - * Computes the number of decimal digits of the number n + * Computes the number of decimal digits of the number n. + * Zero has one digit, and any negative n also returns one. * @param n the given number * @return the number of decimal digits of the number n */ - public static int numDigits(long n) { - if ((n % 10) == 0) { n++; } - return (int) ceil(log(n) / log(10)); + public static int numDigits(final long n) { + int digits = 1; + for (long v = n; v >= 10; v /= 10) { digits++; } + return digits; } /** Generic relational tests *****************/ diff --git a/src/test/java/org/apache/datasketches/common/UtilTest.java b/src/test/java/org/apache/datasketches/common/UtilTest.java index f9f0b09ae..2a2648126 100644 --- a/src/test/java/org/apache/datasketches/common/UtilTest.java +++ b/src/test/java/org/apache/datasketches/common/UtilTest.java @@ -46,6 +46,7 @@ import static org.apache.datasketches.common.Util.longToBytes; import static org.apache.datasketches.common.Util.milliSecToString; import static org.apache.datasketches.common.Util.nanoSecToString; +import static org.apache.datasketches.common.Util.numDigits; import static org.apache.datasketches.common.Util.numberOfLeadingOnes; import static org.apache.datasketches.common.Util.numberOfTrailingOnes; import static org.apache.datasketches.common.Util.powerSeriesNextDouble; @@ -241,6 +242,16 @@ public void checkCharacterPad() { assertEquals(out,"zzzzzzzzzzzzPad 30, prepend z:"); } + @Test + public void checkNumDigits() { + for (long n = 0; n < 1000; n++) { + assertEquals(numDigits(n), Long.toString(n).length()); + } + assertEquals(numDigits(999999999999999999L), 18); + assertEquals(numDigits(1000000000000000000L), 19); + assertEquals(numDigits(Long.MAX_VALUE), 19); + } + @Test public void checkProbabilityFn1() { checkProbability(.5, "Good"); From 6040f84c911aecdf5912aaf938d6db7d239d0848 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:44:48 -0400 Subject: [PATCH 2/3] Make numDigits count characters including the minus sign Applied the review. numDigits now returns the minimum number of characters required to print n as a decimal, so a negative number adds one for the minus sign character. Zero still returns 1. The javadoc now states the assumed context, printing alignment of plainly expressed decimal numbers with an optional minus sign and no commas, underscores or other special characters. The parameter stays final, so the loop divides a local copy of n. checkNumDigits no longer asserts hardcoded digit counts. It compares numDigits(n) against String.valueOf(n).length() over -1000 to 1000, every power of ten and every power of ten minus one in both signs, Long.MAX_VALUE and Long.MIN_VALUE. --- .../org/apache/datasketches/common/Util.java | 20 ++++++++++++------- .../apache/datasketches/common/UtilTest.java | 19 +++++++++++++----- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/datasketches/common/Util.java b/src/main/java/org/apache/datasketches/common/Util.java index 7690a3056..187380fbf 100644 --- a/src/main/java/org/apache/datasketches/common/Util.java +++ b/src/main/java/org/apache/datasketches/common/Util.java @@ -798,15 +798,21 @@ public static int bitAt(final long number, final int bitPos) { /** Decimal Digits ***************************/ /** - * Computes the number of decimal digits of the number n. - * Zero has one digit, and any negative n also returns one. - * @param n the given number - * @return the number of decimal digits of the number n + * Computes the minimum number of characters required to print the number n as a decimal. + * Negative numbers add one for the minus sign character. + * No other non-digit characters are assumed. + * @param n the given number, which may be negative. + * @return the number of characters required to print the number n as a decimal */ public static int numDigits(final long n) { - int digits = 1; - for (long v = n; v >= 10; v /= 10) { digits++; } - return digits; + if (n == 0) { return 1; } //handles the zero special case + int count = (n < 0) ? 1 : 0; //handles the minus sign + long v = n; + while (v != 0) { + v /= 10; + count++; + } + return count; } /** Generic relational tests *****************/ diff --git a/src/test/java/org/apache/datasketches/common/UtilTest.java b/src/test/java/org/apache/datasketches/common/UtilTest.java index 2a2648126..c9c3fd9a0 100644 --- a/src/test/java/org/apache/datasketches/common/UtilTest.java +++ b/src/test/java/org/apache/datasketches/common/UtilTest.java @@ -244,12 +244,21 @@ public void checkCharacterPad() { @Test public void checkNumDigits() { - for (long n = 0; n < 1000; n++) { - assertEquals(numDigits(n), Long.toString(n).length()); + for (long n = -1000; n <= 1000; n++) { + checkN(n); } - assertEquals(numDigits(999999999999999999L), 18); - assertEquals(numDigits(1000000000000000000L), 19); - assertEquals(numDigits(Long.MAX_VALUE), 19); + for (long n = 1; (n < Long.MAX_VALUE) && (n > 0); n *= 10) { + checkN(n); + checkN(n - 1); + checkN(-n); + checkN(-n + 1); + } + checkN(Long.MAX_VALUE); + checkN(Long.MIN_VALUE); + } + + private static void checkN(final long n) { + assertEquals(numDigits(n), String.valueOf(n).length()); } @Test From 6498a90f1c4ca5e9aedf42d3b58565f1a9372216 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:46:40 -0400 Subject: [PATCH 3/3] Drop the always true loop guard in checkNumDigits CodeQL flagged the power of ten loop in UtilTest.checkNumDigits as a useless comparison test. The guard was (n < Long.MAX_VALUE) && (n > 0), and n > 0 is always true there unless the multiply overflows, which a static analyser does not model. The loop now counts the exponent from 0 to 18, every power of ten that fits in a signed long, and holds the multiply back on the last pass so nothing overflows. It visits the same 19 values, 1 through 1E18, and still asserts numDigits(n) against String.valueOf(n).length(). The dense sweep from -1000 to 1000 and the two extreme values are unchanged. --- src/test/java/org/apache/datasketches/common/UtilTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/apache/datasketches/common/UtilTest.java b/src/test/java/org/apache/datasketches/common/UtilTest.java index c9c3fd9a0..13a937f36 100644 --- a/src/test/java/org/apache/datasketches/common/UtilTest.java +++ b/src/test/java/org/apache/datasketches/common/UtilTest.java @@ -247,11 +247,14 @@ public void checkNumDigits() { for (long n = -1000; n <= 1000; n++) { checkN(n); } - for (long n = 1; (n < Long.MAX_VALUE) && (n > 0); n *= 10) { + final int maxExp = 18; //1E18 is the largest power of ten that fits in a signed long + long n = 1; + for (int exp = 0; exp <= maxExp; exp++) { checkN(n); checkN(n - 1); checkN(-n); checkN(-n + 1); + if (exp < maxExp) { n *= 10; } //the multiply past 1E18 would overflow } checkN(Long.MAX_VALUE); checkN(Long.MIN_VALUE);