Repository navigation
refactor(jdbc): extract numeric text parsing and collapse numeric getters - #218
Merged
Merged
Conversation
Characterization tests for the four unpinned behaviors in the JDBC refactor Wave 0: - attached-schema table metadata through pragma_database_list - date/time text tails (offset, Z, bare time) and BLOB rejection - duplicate savepoint names and releasing the outermost savepoint of an auto-commit-started transaction - executeBatch racing statement close Tests only; no production code, pom or dependency change.
Move the two read-only Blob and Clob implementations out of LibsqlResultSet into package-private top-level classes. Constructors become package-private so getBlob and getClob can still build them. Pure move: messages, checks, stream encodings, and free() semantics are unchanged. Also drops the four imports that only the moved code used (OutputStream, Writer, StandardCharsets, Arrays).
Owner
Author
|
Root verdict: PASS at 373b875. The extraction moved all five parser methods verbatim and both getter collapses are arm-for-arm equal, including REAL to int through int64 and integer to float through double. Independent checks found the moved parsers had no saturation test and covered one of six whitespace characters, so a fix-forward added both on this branch. 392 unit tests and 272 conformance tests green, and four mutants now die. Countersign: root, 373b875. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts SQLite's text-to-number coercion from
LibsqlResultSetinto a package-privateSqliteNumericText, then collapses the duplicatedgetInt/getFloatswitch ladders onto the existingtoLong/toDoubleswitches.SqliteNumericTextholdsparseLong(String),parseDouble(String), andblobAsText(byte[])plus the private scannersskipSqliteWhitespace,isSqliteWhitespace,isAsciiDigit. The scanner bodies are byte-identical to the deleted ones; onlyparseSqliteLong/parseSqliteDoublewere renamed toparseLong/parseDouble.toLong(Value)andtoDouble(Value)remain the singleValueswitches and now delegate the text and BLOB cases.getIntis(int) toLong(getCurrentValue(columnIndex));getFloatis(float) toDouble(getCurrentValue(columnIndex)).LibsqlResultSet.javais now 912 lines (was 1029), under the 950-line acceptance bound.Equivalence of the collapsed getters
The pre-change switch arms, the
toLong/toDoublearms, and the new casts are compared arm by arm:ValuevariantgetIntarm(int) toLong(v)armNull0(int) 0L=0Integer(int) i.value()(int) i.value()Float(int) (long) f.value()(int) (long) f.value()Text(int) parseSqliteLong(t.value())(int) SqliteNumericText.parseLong(t.value())Blob(int) parseSqliteLong(blobAsText(b))(int) SqliteNumericText.parseLong(SqliteNumericText.blobAsText(b.bytes()))ValuevariantgetFloatarm(float) toDouble(v)armNull0.0F(float) 0.0=0.0FInteger(float) (double) i.value()(float) (double) i.value()Float(float) f.value()(float) f.value()Text(float) parseSqliteDouble(t.value())(float) SqliteNumericText.parseDouble(t.value())Blob(float) parseSqliteDouble(blobAsText(b))(float) SqliteNumericText.parseDouble(SqliteNumericText.blobAsText(b.bytes()))A scratch differential probe (not committed) ran both ladders over the five variants:
Null; boundary and wrappingIntegervalues (Long.MIN/MAX_VALUE,Integer.MIN/MAX_VALUE,1L << 32,3_000_000_000L);Floatvalues includingNaN, both infinities,-0.0,3.9e9,Long.MAX_VALUEas double, andMath.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).
getBooleanstill routes throughgetInt, so REAL0.5stays false, andLibsqlResultSetTest/LibsqlResultSetGetBooleanTestpass unedited.Follow-up: characterization tests
Commit
373b875adds caller-observable tests toLibsqlResultSetTestwithout touching production code.parseLonglargeIntegerTextBelowLongMaxIsReturnedUnchangedexpected: 6000000000000000000L/but was: 9223372036854775807L'\t'from the whitespace seteverySqliteWhitespaceCharacterIsSkippedBeforeTheNumericPrefix(String)[2](the tab invocation only)expected: 42/but was: 0New coverage:
6000000000000000000(betweenLong.MAX_VALUE / 2andLong.MAX_VALUE) read throughgetLongandgetBigDecimal, unchanged.Long.MAX_VALUEsaturates toLong.MAX_VALUE; text just pastLong.MIN_VALUEsaturates toLong.MIN_VALUE.LibsqlResultSetTestis 45 tests (was 36): 4 new methods, one parameterized with 6 invocations. The JDBC module is 392 tests (was 383).Diff stat
Lane results
./mvnw -pl libsql-jdbc -am test./mvnw spotless:applythen./mvnw spotless:check./mvnw -pl libsql-conformance -am verifyFixes #200