Skip to content

Commit 672f6f0

Browse files
authored
refactor(jdbc): extract numeric text parsing and collapse numeric getters (#218)
## Summary Extracts SQLite's text-to-number coercion from `LibsqlResultSet` into a package-private `SqliteNumericText`, then collapses the duplicated `getInt`/`getFloat` switch ladders onto the existing `toLong`/`toDouble` switches. - New `SqliteNumericText` holds `parseLong(String)`, `parseDouble(String)`, and `blobAsText(byte[])` plus the private scanners `skipSqliteWhitespace`, `isSqliteWhitespace`, `isAsciiDigit`. The scanner bodies are byte-identical to the deleted ones; only `parseSqliteLong`/`parseSqliteDouble` were renamed to `parseLong`/`parseDouble`. - `toLong(Value)` and `toDouble(Value)` remain the single `Value` switches and now delegate the text and BLOB cases. - `getInt` is `(int) toLong(getCurrentValue(columnIndex))`; `getFloat` is `(float) toDouble(getCurrentValue(columnIndex))`. - No parser rule, message, saturation bound, date/time code, LOB class, or POM changed. A follow-up commit adds characterization tests for the saturation bound and all six SQLite whitespace characters. `LibsqlResultSet.java` is now 912 lines (was 1029), under the 950-line acceptance bound. ## Equivalence of the collapsed getters The pre-change switch arms, the `toLong`/`toDouble` arms, and the new casts are compared arm by arm: | `Value` variant | Pre-change `getInt` arm | `(int) toLong(v)` arm | Equal | |---|---|---|---| | `Null` | `0` | `(int) 0L` = `0` | yes | | `Integer` | `(int) i.value()` | `(int) i.value()` | yes | | `Float` | `(int) (long) f.value()` | `(int) (long) f.value()` | yes — truncate to int64, then low-32-bit wrap | | `Text` | `(int) parseSqliteLong(t.value())` | `(int) SqliteNumericText.parseLong(t.value())` | yes — same body | | `Blob` | `(int) parseSqliteLong(blobAsText(b))` | `(int) SqliteNumericText.parseLong(SqliteNumericText.blobAsText(b.bytes()))` | yes — same body and UTF-8 decode | | `Value` variant | Pre-change `getFloat` arm | `(float) toDouble(v)` arm | Equal | |---|---|---|---| | `Null` | `0.0F` | `(float) 0.0` = `0.0F` | yes | | `Integer` | `(float) (double) i.value()` | `(float) (double) i.value()` | yes — rounds through double, not direct long→float | | `Float` | `(float) f.value()` | `(float) f.value()` | yes | | `Text` | `(float) parseSqliteDouble(t.value())` | `(float) SqliteNumericText.parseDouble(t.value())` | yes — same body | | `Blob` | `(float) parseSqliteDouble(blobAsText(b))` | `(float) SqliteNumericText.parseDouble(SqliteNumericText.blobAsText(b.bytes()))` | yes | A scratch differential probe (not committed) ran both ladders over the five variants: `Null`; boundary and wrapping `Integer` values (`Long.MIN/MAX_VALUE`, `Integer.MIN/MAX_VALUE`, `1L << 32`, `3_000_000_000L`); `Float` values including `NaN`, both infinities, `-0.0`, `3.9e9`, `Long.MAX_VALUE` as double, and `Math.nextDown(1.0f)`; 30 text and 30 BLOB inputs covering empty, whitespace-only, sign-only, numeric prefixes, trailing garbage, `3.5e` / `3.5e+` (no exponent digit), `3.5e-2x`, `.5`, and int64 overflow both ways; plus 40,000 random longs and random-bits doubles. Result: **60,092 values, 0 mismatches** (int compared by value, float by raw bits). `getBoolean` still routes through `getInt`, so REAL `0.5` stays false, and `LibsqlResultSetTest` / `LibsqlResultSetGetBooleanTest` pass unedited. ## Follow-up: characterization tests Commit `373b875` adds caller-observable tests to `LibsqlResultSetTest` without touching production code. | Mutant | Test that kills it | Observed failure | |---|---|---| | Halve the saturation bound in `parseLong` | `largeIntegerTextBelowLongMaxIsReturnedUnchanged` | `expected: 6000000000000000000L` / `but was: 9223372036854775807L` | | Drop `'\t'` from the whitespace set | `everySqliteWhitespaceCharacterIsSkippedBeforeTheNumericPrefix(String)[2]` (the tab invocation only) | `expected: 42` / `but was: 0` | New coverage: - Large valid integer `6000000000000000000` (between `Long.MAX_VALUE / 2` and `Long.MAX_VALUE`) read through `getLong` and `getBigDecimal`, unchanged. - Text just past `Long.MAX_VALUE` saturates to `Long.MAX_VALUE`; text just past `Long.MIN_VALUE` saturates to `Long.MIN_VALUE`. - Space, tab, newline, carriage return, vertical tab, and form feed each skipped before a numeric prefix, as one parameterized test. `LibsqlResultSetTest` is 45 tests (was 36): 4 new methods, one parameterized with 6 invocations. The JDBC module is 392 tests (was 383). ## Diff stat ```text .../magnusp/libsql/jdbc/LibsqlResultSet.java | 133 ++------------------- .../magnusp/libsql/jdbc/SqliteNumericText.java | 121 +++++++++++++++++++ .../magnusp/libsql/jdbc/LibsqlResultSetTest.java | 36 ++++++ 3 files changed, 165 insertions(+), 125 deletions(-) ``` ## Lane results | Lane | Result | |---|---| | `./mvnw -pl libsql-jdbc -am test` | green — 392 tests, 0 failures, 0 errors, 0 skipped | | `./mvnw spotless:apply` then `./mvnw spotless:check` | green — BUILD SUCCESS, no reformat needed | | `./mvnw -pl libsql-conformance -am verify` | green — 272 tests, 0 failures, 0 errors, 7 skipped | Fixes #200
1 parent 850f437 commit 672f6f0

3 files changed

Lines changed: 165 additions & 125 deletions

File tree

‎libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSet.java‎

Lines changed: 8 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -202,14 +202,7 @@ public short getShort(int columnIndex) throws SQLException {
202202

203203
@Override
204204
public int getInt(int columnIndex) throws SQLException {
205-
Value val = getCurrentValue(columnIndex);
206-
return switch (val) {
207-
case Value.Null n -> 0;
208-
case Value.Integer i -> (int) i.value();
209-
case Value.Float f -> (int) (long) f.value();
210-
case Value.Text t -> (int) parseSqliteLong(t.value());
211-
case Value.Blob b -> (int) parseSqliteLong(blobAsText(b));
212-
};
205+
return (int) toLong(getCurrentValue(columnIndex));
213206
}
214207

215208
@Override
@@ -219,14 +212,7 @@ public long getLong(int columnIndex) throws SQLException {
219212

220213
@Override
221214
public float getFloat(int columnIndex) throws SQLException {
222-
Value val = getCurrentValue(columnIndex);
223-
return switch (val) {
224-
case Value.Null n -> 0.0F;
225-
case Value.Integer i -> (float) (double) i.value();
226-
case Value.Float f -> (float) f.value();
227-
case Value.Text t -> (float) parseSqliteDouble(t.value());
228-
case Value.Blob b -> (float) parseSqliteDouble(blobAsText(b));
229-
};
215+
return (float) toDouble(getCurrentValue(columnIndex));
230216
}
231217

232218
@Override
@@ -239,8 +225,9 @@ private static long toLong(Value val) {
239225
case Value.Null n -> 0L;
240226
case Value.Integer i -> i.value();
241227
case Value.Float f -> (long) f.value();
242-
case Value.Text t -> parseSqliteLong(t.value());
243-
case Value.Blob b -> parseSqliteLong(blobAsText(b));
228+
case Value.Text t -> SqliteNumericText.parseLong(t.value());
229+
case Value.Blob b ->
230+
SqliteNumericText.parseLong(SqliteNumericText.blobAsText(b.bytes()));
244231
};
245232
}
246233

@@ -249,116 +236,12 @@ private static double toDouble(Value val) {
249236
case Value.Null n -> 0.0;
250237
case Value.Integer i -> (double) i.value();
251238
case Value.Float f -> f.value();
252-
case Value.Text t -> parseSqliteDouble(t.value());
253-
case Value.Blob b -> parseSqliteDouble(blobAsText(b));
239+
case Value.Text t -> SqliteNumericText.parseDouble(t.value());
240+
case Value.Blob b ->
241+
SqliteNumericText.parseDouble(SqliteNumericText.blobAsText(b.bytes()));
254242
};
255243
}
256244

257-
private static String blobAsText(Value.Blob b) {
258-
return new String(b.bytes(), java.nio.charset.StandardCharsets.UTF_8);
259-
}
260-
261-
private static long parseSqliteLong(String text) {
262-
int length = text.length();
263-
int index = skipSqliteWhitespace(text, 0);
264-
boolean negative = false;
265-
if (index < length && (text.charAt(index) == '+' || text.charAt(index) == '-')) {
266-
negative = text.charAt(index) == '-';
267-
index++;
268-
}
269-
270-
int digitsStart = index;
271-
long value = 0L;
272-
boolean overflow = false;
273-
while (index < length) {
274-
char c = text.charAt(index);
275-
if (c < '0' || c > '9') {
276-
break;
277-
}
278-
if (!overflow) {
279-
int digit = c - '0';
280-
if (value > (Long.MAX_VALUE - digit) / 10) {
281-
overflow = true;
282-
} else {
283-
value = value * 10 + digit;
284-
}
285-
}
286-
index++;
287-
}
288-
289-
if (index == digitsStart) {
290-
return 0L;
291-
}
292-
if (overflow) {
293-
return negative ? Long.MIN_VALUE : Long.MAX_VALUE;
294-
}
295-
return negative ? -value : value;
296-
}
297-
298-
private static double parseSqliteDouble(String text) {
299-
int length = text.length();
300-
int index = skipSqliteWhitespace(text, 0);
301-
int start = index;
302-
if (index < length && (text.charAt(index) == '+' || text.charAt(index) == '-')) {
303-
index++;
304-
}
305-
306-
int digits = 0;
307-
while (index < length && isAsciiDigit(text.charAt(index))) {
308-
index++;
309-
digits++;
310-
}
311-
if (index < length && text.charAt(index) == '.') {
312-
index++;
313-
while (index < length && isAsciiDigit(text.charAt(index))) {
314-
index++;
315-
digits++;
316-
}
317-
}
318-
319-
if (digits == 0) {
320-
return 0.0;
321-
}
322-
323-
int end = index;
324-
if (index < length && (text.charAt(index) == 'e' || text.charAt(index) == 'E')) {
325-
int exponentIndex = index + 1;
326-
if (exponentIndex < length
327-
&& (text.charAt(exponentIndex) == '+' || text.charAt(exponentIndex) == '-')) {
328-
exponentIndex++;
329-
}
330-
int exponentDigits = 0;
331-
while (exponentIndex < length && isAsciiDigit(text.charAt(exponentIndex))) {
332-
exponentIndex++;
333-
exponentDigits++;
334-
}
335-
if (exponentDigits > 0) {
336-
end = exponentIndex;
337-
}
338-
}
339-
340-
try {
341-
return Double.parseDouble(text.substring(start, end));
342-
} catch (NumberFormatException e) {
343-
return 0.0;
344-
}
345-
}
346-
347-
private static int skipSqliteWhitespace(String text, int index) {
348-
while (index < text.length() && isSqliteWhitespace(text.charAt(index))) {
349-
index++;
350-
}
351-
return index;
352-
}
353-
354-
private static boolean isSqliteWhitespace(char c) {
355-
return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\u000b' || c == '\f';
356-
}
357-
358-
private static boolean isAsciiDigit(char c) {
359-
return c >= '0' && c <= '9';
360-
}
361-
362245
@Override
363246
@Deprecated
364247
@SuppressWarnings("deprecation")
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package com.github.magnusp.libsql.jdbc;
2+
3+
import java.nio.charset.StandardCharsets;
4+
5+
/**
6+
* Parses SQLite's text-to-number coercion, matching the conversion sqlite-jdbc 3.53.4.0 applies
7+
* for text and BLOB columns. Extracted verbatim from {@link LibsqlResultSet}.
8+
*
9+
* <p>Parsing consumes a numeric prefix only, so trailing garbage is ignored. An exponent counts
10+
* only when at least one digit follows it. Unparseable text yields {@code 0} or {@code 0.0}, and
11+
* integer overflow saturates to {@link Long#MIN_VALUE} or {@link Long#MAX_VALUE}.
12+
*/
13+
final class SqliteNumericText {
14+
15+
private SqliteNumericText() {}
16+
17+
static long parseLong(String text) {
18+
int length = text.length();
19+
int index = skipSqliteWhitespace(text, 0);
20+
boolean negative = false;
21+
if (index < length && (text.charAt(index) == '+' || text.charAt(index) == '-')) {
22+
negative = text.charAt(index) == '-';
23+
index++;
24+
}
25+
26+
int digitsStart = index;
27+
long value = 0L;
28+
boolean overflow = false;
29+
while (index < length) {
30+
char c = text.charAt(index);
31+
if (c < '0' || c > '9') {
32+
break;
33+
}
34+
if (!overflow) {
35+
int digit = c - '0';
36+
if (value > (Long.MAX_VALUE - digit) / 10) {
37+
overflow = true;
38+
} else {
39+
value = value * 10 + digit;
40+
}
41+
}
42+
index++;
43+
}
44+
45+
if (index == digitsStart) {
46+
return 0L;
47+
}
48+
if (overflow) {
49+
return negative ? Long.MIN_VALUE : Long.MAX_VALUE;
50+
}
51+
return negative ? -value : value;
52+
}
53+
54+
static double parseDouble(String text) {
55+
int length = text.length();
56+
int index = skipSqliteWhitespace(text, 0);
57+
int start = index;
58+
if (index < length && (text.charAt(index) == '+' || text.charAt(index) == '-')) {
59+
index++;
60+
}
61+
62+
int digits = 0;
63+
while (index < length && isAsciiDigit(text.charAt(index))) {
64+
index++;
65+
digits++;
66+
}
67+
if (index < length && text.charAt(index) == '.') {
68+
index++;
69+
while (index < length && isAsciiDigit(text.charAt(index))) {
70+
index++;
71+
digits++;
72+
}
73+
}
74+
75+
if (digits == 0) {
76+
return 0.0;
77+
}
78+
79+
int end = index;
80+
if (index < length && (text.charAt(index) == 'e' || text.charAt(index) == 'E')) {
81+
int exponentIndex = index + 1;
82+
if (exponentIndex < length
83+
&& (text.charAt(exponentIndex) == '+' || text.charAt(exponentIndex) == '-')) {
84+
exponentIndex++;
85+
}
86+
int exponentDigits = 0;
87+
while (exponentIndex < length && isAsciiDigit(text.charAt(exponentIndex))) {
88+
exponentIndex++;
89+
exponentDigits++;
90+
}
91+
if (exponentDigits > 0) {
92+
end = exponentIndex;
93+
}
94+
}
95+
96+
try {
97+
return Double.parseDouble(text.substring(start, end));
98+
} catch (NumberFormatException e) {
99+
return 0.0;
100+
}
101+
}
102+
103+
static String blobAsText(byte[] bytes) {
104+
return new String(bytes, StandardCharsets.UTF_8);
105+
}
106+
107+
private static int skipSqliteWhitespace(String text, int index) {
108+
while (index < text.length() && isSqliteWhitespace(text.charAt(index))) {
109+
index++;
110+
}
111+
return index;
112+
}
113+
114+
private static boolean isSqliteWhitespace(char c) {
115+
return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\u000b' || c == '\f';
116+
}
117+
118+
private static boolean isAsciiDigit(char c) {
119+
return c >= '0' && c <= '9';
120+
}
121+
}

‎libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55
import com.github.magnusp.libsql.hrana.codec.StmtResult;
66
import com.github.magnusp.libsql.hrana.codec.Value;
77
import org.junit.jupiter.api.Test;
8+
import org.junit.jupiter.params.ParameterizedTest;
9+
import org.junit.jupiter.params.provider.ValueSource;
810

911
import java.io.IOException;
1012
import java.io.InputStream;
1113
import java.io.Reader;
14+
import java.math.BigDecimal;
1215
import java.nio.charset.StandardCharsets;
1316
import java.sql.Array;
1417
import java.sql.Blob;
@@ -138,6 +141,39 @@ void textWithLeadingWhitespaceAndTrailingGarbageUsesNumericPrefix() throws SQLEx
138141
assertNumeric(rs, 1, 42, 42L, 42.0f, 42.0);
139142
}
140143

144+
@ParameterizedTest
145+
@ValueSource(strings = {" ", "\t", "\n", "\r", "\u000b", "\f"})
146+
void everySqliteWhitespaceCharacterIsSkippedBeforeTheNumericPrefix(String whitespace) throws SQLException {
147+
LibsqlResultSet rs = resultSet(new Value.Text(whitespace + "42xyz"));
148+
assertThat(rs.next()).isTrue();
149+
assertNumeric(rs, 1, 42, 42L, 42.0f, 42.0);
150+
}
151+
152+
@Test
153+
void largeIntegerTextBelowLongMaxIsReturnedUnchanged() throws SQLException {
154+
LibsqlResultSet rs = resultSet(new Value.Text("6000000000000000000"));
155+
assertThat(rs.next()).isTrue();
156+
157+
assertThat(rs.getLong(1)).isEqualTo(6_000_000_000_000_000_000L);
158+
assertThat(rs.getBigDecimal(1)).isEqualByComparingTo(new BigDecimal("6000000000000000000"));
159+
}
160+
161+
@Test
162+
void textJustPastLongMaxSaturatesToLongMax() throws SQLException {
163+
LibsqlResultSet rs = resultSet(new Value.Text("9223372036854775808"));
164+
assertThat(rs.next()).isTrue();
165+
166+
assertThat(rs.getLong(1)).isEqualTo(Long.MAX_VALUE);
167+
}
168+
169+
@Test
170+
void textJustPastLongMinSaturatesToLongMin() throws SQLException {
171+
LibsqlResultSet rs = resultSet(new Value.Text("-9223372036854775809"));
172+
assertThat(rs.next()).isTrue();
173+
174+
assertThat(rs.getLong(1)).isEqualTo(Long.MIN_VALUE);
175+
}
176+
141177
@Test
142178
void numericBlobIsInterpretedAsText() throws SQLException {
143179
LibsqlResultSet rs = resultSet(new Value.Blob("123".getBytes(StandardCharsets.UTF_8)));

0 commit comments

Comments
 (0)